Dear Ivan,
could you please have a look at the coverity issues in the gsm_rlcmac.cpp routines?
Uninitialized scalar variable: gsm_rlcmac.cpp:5321 ar.direction not initialized gsm_rlcmac.cpp:5039 ar.direction not initialized gsm_rlcmac.cpp:5155 ar.direction not initialized gsm_rlcmac.cpp:4872 ar.direction not initialized
Just initialize it in csnStreamInit?
Out-of-bounds read: gsm_rlcmac.cpp:5502 " Overrunning array "data->RLC_DATA" of 20 bytes at byte offset 22 using index "i" (which evaluates to 22)."
gsm_rlcmac.cpp:5440 " Overrunning array "data->RLC_DATA" of 20 bytes at byte offset 22 using index "i" (which evaluates to 22)."
Maybe just add an assert that dataNumOctets <= 20?
Hi Holger,
2013/11/11 Holger Hans Peter Freyther hfreyther@sysmocom.de:
Uninitialized scalar variable: gsm_rlcmac.cpp:5321 ar.direction not initialized gsm_rlcmac.cpp:5039 ar.direction not initialized gsm_rlcmac.cpp:5155 ar.direction not initialized gsm_rlcmac.cpp:4872 ar.direction not initialized
Just initialize it in csnStreamInit?
Yes.
Out-of-bounds read: gsm_rlcmac.cpp:5502 " Overrunning array "data->RLC_DATA" of 20 bytes at byte offset 22 using index "i" (which evaluates to 22)."
gsm_rlcmac.cpp:5440 " Overrunning array "data->RLC_DATA" of 20 bytes at byte offset 22 using index "i" (which evaluates to 22)."
Maybe just add an assert that dataNumOctets <= 20?
Yes, it makes sense.
I will prepare patch for this issues soon.
2013/11/12 Ivan Kluchnikov Ivan.Kluchnikov@fairwaves.ru:
Hi Holger,
2013/11/11 Holger Hans Peter Freyther hfreyther@sysmocom.de:
Uninitialized scalar variable: gsm_rlcmac.cpp:5321 ar.direction not initialized gsm_rlcmac.cpp:5039 ar.direction not initialized gsm_rlcmac.cpp:5155 ar.direction not initialized gsm_rlcmac.cpp:4872 ar.direction not initialized
Just initialize it in csnStreamInit?
Yes.
Out-of-bounds read: gsm_rlcmac.cpp:5502 " Overrunning array "data->RLC_DATA" of 20 bytes at byte offset 22 using index "i" (which evaluates to 22)."
gsm_rlcmac.cpp:5440 " Overrunning array "data->RLC_DATA" of 20 bytes at byte offset 22 using index "i" (which evaluates to 22)."
Maybe just add an assert that dataNumOctets <= 20?
Yes, it makes sense.
-- Regards, Ivan Kluchnikov. http://fairwaves.ru
Hi Holger,
I merged my patch to the master branch. Please, check it.
2013/12/25 Holger Hans Peter Freyther holger@freyther.de:
On Tue, Nov 12, 2013 at 05:44:43PM +0400, Ivan Kluchnikov wrote:
Dear Ivan,
I will prepare patch for this issues soon.
it would be nice if we could finally close this issue. Do you have a patch ready for this?
holger
On Mon, Dec 30, 2013 at 02:47:53PM +0400, Ivan Kluchnikov wrote:
Hi Holger,
I merged my patch to the master branch. Please, check it.
Daniel has merged this to sysmocom/master and jenkins points out that the result of RLCMactest changed. Could you please look at the result and check if that is correct or wrong?
diff --git a/tests/rlcmac/RLCMACTest.ok b/tests/rlcmac/RLCMACTest.ok index 99117ff..22744ed 100644 --- a/tests/rlcmac/RLCMACTest.ok +++ b/tests/rlcmac/RLCMACTest.ok @@ -53,6 +53,6 @@ vector1 = 4016713dc09427ca2ae57ef90906aafc001f80222b +++++++++Finish DECODE++++++++++ =========Start ENCODE============= +++++++++Finish ENCODE+++++++++++ -vector1 = 4016713dc09427ca2ae57ef90906aafc001f80222b -vector2 = 4016713dc09427ca2ae57ef90906aafc001f80222b -vector1 == vector2 : TRUE +vector1 = 4016713dc09427d001da10906aafc001f80222b +vector2 = 4016713dc0942691001da10906aafc0b2b2b2b2b +vector1 == vector2 : FALSE
Hi,
On Sat, 2014-01-11 at 20:22, Holger Hans Peter Freyther wrote:
On Mon, Dec 30, 2013 at 02:47:53PM +0400, Ivan Kluchnikov wrote:
Hi Holger,
I merged my patch to the master branch. Please, check it.
Daniel has merged this to sysmocom/master and jenkins points out that the result of RLCMactest changed. Could you please look at the result and check if that is correct or wrong?
we just found the issue. In one instance the direction was set before csnStreamInit which of course doesn't work anymore. See my patch in a different mail. I already pushed it to sysmocom/master and master as it fixes the test failure.
Regards, Daniel
Good catch, Daniel,
Thank you for fixing this bug!
2014/1/15 Daniel Willmann dwillmann@sysmocom.de:
Hi,
On Sat, 2014-01-11 at 20:22, Holger Hans Peter Freyther wrote:
On Mon, Dec 30, 2013 at 02:47:53PM +0400, Ivan Kluchnikov wrote:
Hi Holger,
I merged my patch to the master branch. Please, check it.
Daniel has merged this to sysmocom/master and jenkins points out that the result of RLCMactest changed. Could you please look at the result and check if that is correct or wrong?
we just found the issue. In one instance the direction was set before csnStreamInit which of course doesn't work anymore. See my patch in a different mail. I already pushed it to sysmocom/master and master as it fixes the test failure.
Regards, Daniel
--
- Daniel Willmann dwillmann@sysmocom.de http://www.sysmocom.de/
=======================================================================
- sysmocom - systems for mobile communications GmbH
- Schivelbeiner Str. 5
- 10439 Berlin, Germany
- Sitz / Registered office: Berlin, HRB 134158 B
- Geschaeftsfuehrer / Managing Directors: Holger Freyther, Harald Welte
osmocom-net-gprs@lists.osmocom.org
-
Daniel Willmann -
Holger Hans Peter Freyther -
Holger Hans Peter Freyther -
Ivan Kluchnikov