Project

General

Profile

Bug #4483

osmo-bts-trx: global-buffer-overflow in gsm0503_mcs1_dl_interleave after enabling "egprs only" in osmo-pcu (osmo-gsm-tester)

Added by pespin 7 months ago. Updated 5 months ago.

Status:
Resolved
Priority:
Normal
Assignee:
Category:
osmo-bts-trx
Target version:
-
Start date:
04/06/2020
Due date:
% Done:

100%

Spec Reference:

Description

First time I tested osmo-gsm-tester.git Change-Id Idd0b5bf8769d693480268c0a0b89dbfd63779e48, which sets osmo-pcu VTY cfg to use "egprs only" when EGPRS is enabled by the scenario, I got a crash (test ping.py) in osmo-bts-trx while the modem was PS attaching to the network:

20200406183348272 DTRX <000b> trx_if.c:122 phy0.0: Clock indication: fn=50053
20200406183348272 DL1C <0006> scheduler_trx.c:1787 TRX Clock Ind: elapsed_us=1000036, elapsed_fn=217, error_us=-1419
20200406183348272 DL1C <0006> scheduler_trx.c:1805 GSM clock jitter: -2452us (elapsed_fn=0)
20200406183348964 DPCU <0009> pcu_sock.c:391 Sending RACH indication: qta=-2, ra=113, fn=50198
20200406183349270 DTRX <000b> trx_if.c:122 phy0.0: Clock indication: fn=50269
20200406183349270 DL1C <0006> scheduler_trx.c:1787 TRX Clock Ind: elapsed_us= 998125, elapsed_fn=216, error_us=+1285
20200406183349270 DL1C <0006> scheduler_trx.c:1805 GSM clock jitter: -3803us (elapsed_fn=0)
=================================================================
==12388==ERROR: AddressSanitizer: global-buffer-overflow on address 0x7fa20b9ab8d0 at pc 0x7fa20b982894 bp 0x7ffdfea8b9c0 sp 0x7ffdfea8b9b8
READ of size 1 at 0x7fa20b9ab8d0 thread T0
    #0 0x7fa20b982893 in gsm0503_mcs1_dl_interleave /home/osmocom-build/jenkins/workspace/osmo-gsm-tester_build-osmo-bts/libosmocore/src/coding/gsm0503_interleaving.c:165
    #1 0x7fa20b994f1a in egprs_type3_map /home/osmocom-build/jenkins/workspace/osmo-gsm-tester_build-osmo-bts/libosmocore/src/coding/gsm0503_coding.c:1179
    #2 0x7fa20b994f1a in gsm0503_pdtch_egprs_encode /home/osmocom-build/jenkins/workspace/osmo-gsm-tester_build-osmo-bts/libosmocore/src/coding/gsm0503_coding.c:1392
    #3 0x558d8ad7ebe3 in tx_pdtch_fn /home/osmocom-build/jenkins/workspace/osmo-gsm-tester_build-osmo-bts/osmo-bts/src/osmo-bts-trx/scheduler_trx.c:280
    #4 0x558d8ada0119 in _sched_dl_burst /home/osmocom-build/jenkins/workspace/osmo-gsm-tester_build-osmo-bts/osmo-bts/src/common/scheduler.c:1188
    #5 0x558d8ad7c4f7 in trx_sched_fn /home/osmocom-build/jenkins/workspace/osmo-gsm-tester_build-osmo-bts/osmo-bts/src/osmo-bts-trx/scheduler_trx.c:1568
    #6 0x558d8ad7cb3a in trx_fn_timer_cb /home/osmocom-build/jenkins/workspace/osmo-gsm-tester_build-osmo-bts/osmo-bts/src/osmo-bts-trx/scheduler_trx.c:1672
    #7 0x7fa209fd2bc6 in osmo_fd_disp_fds /home/osmocom-build/jenkins/workspace/osmo-gsm-tester_build-osmo-bts/libosmocore/src/select.c:227
    #8 0x7fa209fd2bc6 in _osmo_select_main /home/osmocom-build/jenkins/workspace/osmo-gsm-tester_build-osmo-bts/libosmocore/src/select.c:265
    #9 0x7fa209fd5bfa in osmo_select_main /home/osmocom-build/jenkins/workspace/osmo-gsm-tester_build-osmo-bts/libosmocore/src/select.c:274
    #10 0x558d8ae2242b in bts_main /home/osmocom-build/jenkins/workspace/osmo-gsm-tester_build-osmo-bts/osmo-bts/src/common/main.c:354
    #11 0x7fa208a732e0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202e0)
    #12 0x558d8ad6aa39 in _start (/home/jenkins/workspace/osmo-gsm-tester_manual-run/trial-263/inst/osmo-bts/bin/osmo-bts-trx+0x131a39)

0x7fa20b9ab8d0 is located 48 bytes to the left of global variable 'gsm0503_pdtch_edge_hl_hn_sbit' defined in 'gsm0503_tables.c:60:14' (0x7fa20b9ab900) of size 24
0x7fa20b9ab8d0 is located 0 bytes to the right of global variable 'gsm0503_usf2six' defined in 'gsm0503_tables.c:66:14' (0x7fa20b9ab8a0) of size 48
SUMMARY: AddressSanitizer: global-buffer-overflow /home/osmocom-build/jenkins/workspace/osmo-gsm-tester_build-osmo-bts/libosmocore/src/coding/gsm0503_interleaving.c:165 in gsm0503_mcs1_dl_interleave
Shadow bytes around the buggy address:
  0x0ff4c172d6c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0ff4c172d6d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0ff4c172d6e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 04 f9 f9
  0x0ff4c172d6f0: f9 f9 f9 f9 00 00 00 00 00 00 00 00 00 00 00 00
  0x0ff4c172d700: f9 f9 f9 f9 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0ff4c172d710: f9 f9 f9 f9 00 00 00 00 00 00[f9]f9 f9 f9 f9 f9
  0x0ff4c172d720: 00 00 00 f9 f9 f9 f9 f9 00 00 00 00 f9 f9 f9 f9
  0x0ff4c172d730: 00 00 00 f9 f9 f9 f9 f9 00 00 00 00 f9 f9 f9 f9
  0x0ff4c172d740: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0ff4c172d750: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0ff4c172d760: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Heap right redzone:      fb
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack partial redzone:   f4
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==12388==ABORTING

Find attached the run directory archived with all logs and pcaps.

trial-263-run.tgz trial-263-run.tgz 188 KB pespin, 04/06/2020 04:41 PM

History

#1 Updated by pespin 7 months ago

  • Description updated (diff)

#2 Updated by pespin 7 months ago

So we have this array:

include/osmocom/coding/gsm0503_tables.h:
18:extern const ubit_t gsm0503_usf2six[8][6];

src/coding/gsm0503_tables.c:
const ubit_t gsm0503_usf2six[8][6] = {
    { 0,0,0, 0,0,0 },
    { 1,0,0, 1,0,1 },
    { 0,1,0, 1,1,0 },
    { 1,1,0, 0,1,1 },
    { 0,0,1, 0,1,1 },
    { 1,0,1, 1,1,0 },
    { 0,1,1, 1,0,1 },
    { 1,1,1, 0,0,0 },
};

Then, in egprs_type3_map() the function gsm0503_mcs1_dl_interleave is called this way (usf=7, which means last subarray of 6 entries is passed):

gsm0503_mcs1_dl_interleave(gsm0503_usf2six[usf], hc, dc, iB);

The function does:

usf=7?
up = gsm0503_usf2six[usf]
ubit_t hc[EGPRS_DATA_C_MAX];
ubit_t dc[EGPRS_DATA_DC_MAX];
ubit_t iB[456];

/*! Interleave MCS1 DL burst bits according to TS 05.03 5.1.5.1.5
 *  \param[in] up 12 input soft coded bits (usf)
 *  \param[in] hc 68 input soft coded bits (header)
 *  \param[in] dc 372 input soft bits (data)
 *  \param[out] iB 456 interleaved soft output bits */
void gsm0503_mcs1_dl_interleave(const ubit_t *up, const ubit_t *hc,
    const ubit_t *dc, ubit_t *iB)
{
    int k;
    ubit_t c[452];
    ubit_t cp[456];

    for (k = 0; k < 12; k++)      <----- HERE; offset 12 is accessed, but array is 6 items long!!!!
        c[k] = up[k];
    for (k = 12; k < 80; k++)
        c[k] = hc[k - 12];
    for (k = 80; k < 452; k++)
        c[k] = dc[k - 80];

    for (k = 0; k < 25; k++)
        cp[k] = c[k];
    for (k = 26; k < 82; k++)
        cp[k] = c[k - 1];
    for (k = 83; k < 139; k++)
        cp[k] = c[k - 2];
    for (k = 140; k < 424; k++)
        cp[k] = c[k - 3];
    for (k = 425; k < 456; k++)
        cp[k] = c[k - 4];

    cp[25] = 0;
    cp[82] = 0;
    cp[139] = 0;
    cp[424] = 0;

    gsm0503_xcch_interleave(cp, iB);
}

So clearly there's something wrong there (offset 12 vs len=6). The buffer overflow only happens if USF=7 is passed, but in any case it looks like potentially wrong processing is done.

#3 Updated by pespin 7 months ago

I think gsm0503_usf2twelve_ubit array should be used there instead of gsm0503_usf2six, trying to confirm now.

#4 Updated by pespin 7 months ago

  • Status changed from In Progress to Feedback
  • % Done changed from 0 to 90

Should be fixed by:
remote: https://gerrit.osmocom.org/c/libosmocore/+/17740 gsm0503_coding: Fix misleading comment UL vs DL
remote: https://gerrit.osmocom.org/c/libosmocore/+/17741 gsm0503_coding: Fix USF encoding in MCS1-4
remote: https://gerrit.osmocom.org/c/libosmocore/+/17742 gsm0503_tables: Document USF encoding tables

#5 Updated by pespin 7 months ago

Added some unit testing as requested:
https://gerrit.osmocom.org/c/libosmocore/+/17750 tests/coding: Test decoding of DL EGPRS data packet

#6 Updated by pespin 5 months ago

  • Status changed from Feedback to Resolved
  • % Done changed from 90 to 100

Fixed & merged, closing.

Also available in: Atom PDF

Add picture from clipboard (Maximum size: 48.8 MB)