Smatch results on openbsc / static code analysis

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/OpenBSC@lists.osmocom.org/.

Harald Welte laforge at gnumonks.org
Sat Jul 16 11:49:21 UTC 2011


Hi!

I've started to play a bit with Smatch (http://smatch.sourceforge.net/)
and fixed a number of bugs in libosmocore.

When applying it to openbsc, I get:

  CC     ipaccess.o
/home/laforge/projects/git/openbsc/openbsc/src/libabis/input/ipaccess.c +455 ipaccess_drop(28) info: loop could be replaced with if statement.
/home/laforge/projects/git/openbsc/openbsc/src/libabis/input/ipaccess.c +451 ipaccess_drop(24) info: ignoring unreachable code.

The point herer is: we loop over a list, but we return from the first
iteration of the loop.  Zecke?

  CC     abis_nm.o
/home/laforge/projects/git/openbsc/openbsc/src/libbsc/abis_nm.c +810 sw_load_segment(38) warn: unsigned 'len' is never less than zero.

'len' has to be signed, I fixed that one.

  CC     paging.o
/home/laforge/projects/git/openbsc/openbsc/src/libbsc/paging.c +134 can_send_pag_req(25) info: ignoring unreachable code.

We have a goto statement in each possible caes (including defualt) above
it. So the return 0 will never be hit.  That's ok and not a bug.  But I
think the code is too convoluted this way.  I think we should have one
function that just returns (sdcch/tch) based on the rsl_type and
net->pag_any_tch, and then a second function that has a simple if/else.

I'm not against goto - but I think this time it really can be avoided
easily.

  CC     bsc_vty.o
/home/laforge/projects/git/openbsc/openbsc/src/libbsc/bsc_vty.c +1062 show_e1ts(25) warn: variable dereferenced before check 'line'
/home/laforge/projects/git/openbsc/openbsc/src/libbsc/bsc_vty.c +1075 show_e1ts(38) warn: buffer overflow 'line->ts' 32 <= 32
/home/laforge/projects/git/openbsc/openbsc/src/libbsc/bsc_vty.c +1080 show_e1ts(43) error: potential null derefence 'line'.

fixed two of them, the third is bogus

  CC     db.o
/home/laforge/projects/git/openbsc/openbsc/src/libmsc/db.c +254 db_fini(6) info: redundant null check on db_dirname calling free()
/home/laforge/projects/git/openbsc/openbsc/src/libmsc/db.c +256 db_fini(8) info: redundant null check on db_basename calling free()
/home/laforge/projects/git/openbsc/openbsc/src/libmsc/db.c +280 db_create_subscriber(20) warn: variable dereferenced before check 'subscr'
/home/laforge/projects/git/openbsc/openbsc/src/libmsc/db.c +1062 sms_from_result(36) warn: 256 is more than 255 (max 'sms->user_data_len' can be) so this is always false.

fixed the first 3, the last remains as a safeguard

  CC     gsm_04_08.o
/home/laforge/projects/git/openbsc/openbsc/src/libmsc/gsm_04_08.c +550 mm_rx_loc_upd_req(46) error: we previously assumed 'conn->loc_operation' could be null.
/home/laforge/projects/git/openbsc/openbsc/src/libmsc/gsm_04_08.c +1891 gsm48_cc_rx_setup(68) error: we previously assumed 'trans->subscr' could be null.
/home/laforge/projects/git/openbsc/openbsc/src/libmsc/gsm_04_08.c +2193 gsm48_cc_rx_connect(40) error: we previously assumed 'trans->subscr' could be null.

The first is bogus,  the others need to be investigated

  CC     gsm_04_11.o
/home/laforge/projects/git/openbsc/openbsc/src/libmsc/gsm_04_11.c +599 gsm340_rx_tpdu(46) error: sms_alphabet is never equal to 4294967295 (wrong type 0 - 255).

I fixed that one!

  CC     ussd.o
/home/laforge/projects/git/openbsc/openbsc/src/libmsc/ussd.c +54 handle_rcv_ussd(9) error: req.text[0] is never equal to 255 (wrong type -128 - 127).
  CC     bsc_ussd.o
/home/laforge/projects/git/openbsc/openbsc/src/osmo-bsc_nat/bsc_ussd.c +385 bsc_check_ussd(62) error: req.text[0] is never equal to 255 (wrong type -128 - 127).

This is due to 'struct ussd_request.text' being 'char', I changed it to
uint8_t.

  CC     bs11_config.o
/home/laforge/projects/git/openbsc/openbsc/src/utils/bs11_config.c +223 linkstate_name(5) error: buffer overflow 'bs11_link_state' 3 <= 3
/home/laforge/projects/git/openbsc/openbsc/src/utils/bs11_config.c +240 mbccu_load_name(5) error: buffer overflow 'mbccu_load' 6 <= 6
/home/laforge/projects/git/openbsc/openbsc/src/utils/bs11_config.c +905 main(34) info: ignoring unreachable code.

fixed.

  CC     ipaccess-firmware.o
/home/laforge/projects/git/openbsc/openbsc/src/ipaccess/ipaccess-firmware.c +64 ipaccess_analyze_file(26) warn: buffer overflow 'firmware_header->more_magic' 2 <= 2
/home/laforge/projects/git/openbsc/openbsc/src/ipaccess/ipaccess-firmware.c +64 ipaccess_analyze_file(26) warn: buffer overflow 'firmware_header->more_magic' 2 <= 3

zecke?

  CC     ipaccess-proxy.o
/home/laforge/projects/git/openbsc/openbsc/src/ipaccess/ipaccess-proxy.c +173 store_idtags(14) error: buffer overflow 'ipbc->id_tags' 255 <= 255
/home/laforge/projects/git/openbsc/openbsc/src/ipaccess/ipaccess-proxy.c +173 store_idtags(14) error: buffer overflow 'ipbc->id_tags' 255 <= 255
/home/laforge/projects/git/openbsc/openbsc/src/ipaccess/ipaccess-proxy.c +175 store_idtags(16) error: buffer overflow 'ipbc->id_tags' 255 <= 255
/home/laforge/projects/git/openbsc/openbsc/src/ipaccess/ipaccess-proxy.c +178 store_idtags(19) error: buffer overflow 'ipbc->id_tags' 255 <= 255
/home/laforge/projects/git/openbsc/openbsc/src/ipaccess/ipaccess-proxy.c +500 ipaccess_rcvmsg(66) error: buffer overflow 'ipbc->rsl_conn' 4 <= 4
/home/laforge/projects/git/openbsc/openbsc/src/ipaccess/ipaccess-proxy.c +504 ipaccess_rcvmsg(70) error: buffer overflow 'ipbc->bsc_rsl_conn' 4 <= 4

fixed

  CC     gprs_bssgp_util.o
/home/laforge/projects/git/openbsc/openbsc/src/libgb/gprs_bssgp_util.c +114 bssgp_tx_status(17) warn: variable dereferenced before check 'orig_msg'

fixed.

  CC     gb_proxy_main.o
/home/laforge/projects/git/openbsc/openbsc/src/gprs/gb_proxy_main.c +284 main(81) info: ignoring unreachable code.

bogus, sa it's jus an exit(0)

  CC     gprs_gmm.o
/home/laforge/projects/git/openbsc/openbsc/src/gprs/gprs_gmm.c +757 gsm48_rx_gmm_att_req(133) warn: variable dereferenced before check 'ctx'

fixed

  CC     gprs_sndcp.o
/home/laforge/projects/git/openbsc/openbsc/src/gprs/gprs_sndcp.c +478 sndcp_unitdata_req(37) info: ignoring unreachable code.

comment in the code says it is not reached

  CC     sgsn_main.o
/home/laforge/projects/git/openbsc/openbsc/src/gprs/sgsn_main.c +284 main(83) info: ignoring unreachable code.

comment in the code says it is not reached

  CC     sgsn_libgtp.o
/home/laforge/projects/git/openbsc/openbsc/src/gprs/sgsn_libgtp.c +504 sgsn_rx_sndcp_ud_ind(32) info: ignoring unreachable code.

fixed

  CC     bsc_nat.o
/home/laforge/projects/git/openbsc/openbsc/src/osmo-bsc_nat/bsc_nat.c +1553 get_next_free_bsc_id(20) info: ignoring unreachable code.

zecke?

-- 
- Harald Welte <laforge at gnumonks.org>           http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
                                                  (ETSI EN 300 175-7 Ch. A6)




More information about the OpenBSC mailing list