[BlueZ,1/7] monitor: Fix out-of-bound read in print_le_states

Commit Message

Ildar Kamaletdinov April 1, 2022, 7:46 a.m. UTC

Accessing le_states_desc_table array with value 15 can cause
out-of-bound read because current size of array is 14.

Currently this cannot lead to any problems becase we do no have such
state in le_states_comb_table but this could be changed in future and
raise described problem.

Found by Linux Verification Center (linuxtesting.org) with the SVACE
static analysis tool.
---
 monitor/packet.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

[e-mail] April 1, 2022, 8:45 a.m. UTC | #1

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=628109

---Test result---

Test Summary:
CheckPatch                    FAIL      9.98 seconds
GitLint                       PASS      6.96 seconds
Prep - Setup ELL              PASS      42.33 seconds
Build - Prep                  PASS      0.68 seconds
Build - Configure             PASS      8.45 seconds
Build - Make                  FAIL      36.46 seconds
Make Check                    FAIL      10.48 seconds
Make Check w/Valgrind         FAIL      35.64 seconds
Make Distcheck                PASS      223.16 seconds
Build w/ext ELL - Configure   PASS      8.57 seconds
Build w/ext ELL - Make        FAIL      36.93 seconds
Incremental Build with patchesFAIL      46.08 seconds

Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script with rule in .checkpatch.conf
Output:
[BlueZ,3/7] tools: Fix signed interger overflow in btsnoop.c
WARNING:TYPO_SPELLING: 'interger' may be misspelled - perhaps 'integer'?
#35: 
Subject: [PATCH BlueZ 3/7] tools: Fix signed interger overflow in btsnoop.c
                                             ^^^^^^^^

/github/workspace/src/12797974.patch total: 0 errors, 1 warnings, 8 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

/github/workspace/src/12797974.patch has style problems, please review.

NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.

[BlueZ,4/7] tools: Prevent infinity loops in bluemoon.c
WARNING:TYPO_SPELLING: 'standart' may be misspelled - perhaps 'standard'?
#50: 
According to C99 standart SIZE_MAX could be as small as 65535.
                 ^^^^^^^^

/github/workspace/src/12797975.patch total: 0 errors, 1 warnings, 25 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

/github/workspace/src/12797975.patch has style problems, please review.

NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.

[BlueZ,6/7] device: Limit width of fields in sscanf
WARNING:LONG_LINE: line length of 82 exceeds 80 columns
#90: FILE: src/device.c:3887:
+	if (sscanf(value, GATT_INCLUDE_UUID_STR ":%04hx:%04hx:%36s", &start, &end,

/github/workspace/src/12797978.patch total: 0 errors, 1 warnings, 37 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

/github/workspace/src/12797978.patch has style problems, please review.

NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.

[BlueZ,7/7] gatt: Fix double free and freed memory dereference
WARNING:TYPO_SPELLING: 'occure' may be misspelled - perhaps 'occurred'?
#50: 
notification it is possible to to occure double free and dereference of
                                  ^^^^^^

WARNING:REPEATED_WORD: Possible repeated word: 'to'
#50: 
notification it is possible to to occure double free and dereference of

/github/workspace/src/12797968.patch total: 0 errors, 2 warnings, 10 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

/github/workspace/src/12797968.patch has style problems, please review.

NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.


##############################
Test: Build - Make - FAIL
Desc: Build the BlueZ source tree
Output:
monitor/packet.c: In function ‘print_le_states’:
monitor/packet.c:2836:17: error: comparison of integer expressions of different signedness: ‘int’ and ‘long unsigned int’ [-Werror=sign-compare]
 2836 |   for (n = 0; n < ARRAY_SIZE(le_states_desc_table); n++) {
      |                 ^
cc1: all warnings being treated as errors
make[1]: *** [Makefile:7277: monitor/packet.o] Error 1
make: *** [Makefile:4310: all] Error 2


##############################
Test: Make Check - FAIL
Desc: Run 'make check'
Output:
monitor/packet.c: In function ‘print_le_states’:
monitor/packet.c:2836:17: error: comparison of integer expressions of different signedness: ‘int’ and ‘long unsigned int’ [-Werror=sign-compare]
 2836 |   for (n = 0; n < ARRAY_SIZE(le_states_desc_table); n++) {
      |                 ^
cc1: all warnings being treated as errors
make[1]: *** [Makefile:7277: monitor/packet.o] Error 1
make: *** [Makefile:11283: check] Error 2


##############################
Test: Make Check w/Valgrind - FAIL
Desc: Run 'make check' with Valgrind
Output:
monitor/packet.c: In function ‘print_le_states’:
monitor/packet.c:2836:17: error: comparison of integer expressions of different signedness: ‘int’ and ‘long unsigned int’ [-Werror=sign-compare]
 2836 |   for (n = 0; n < ARRAY_SIZE(le_states_desc_table); n++) {
      |                 ^
cc1: all warnings being treated as errors
make[1]: *** [Makefile:7277: monitor/packet.o] Error 1
make: *** [Makefile:4310: all] Error 2


##############################
Test: Build w/ext ELL - Make - FAIL
Desc: Build BlueZ source with '--enable-external-ell' configuration
Output:
monitor/packet.c: In function ‘print_le_states’:
monitor/packet.c:2836:17: error: comparison of integer expressions of different signedness: ‘int’ and ‘long unsigned int’ [-Werror=sign-compare]
 2836 |   for (n = 0; n < ARRAY_SIZE(le_states_desc_table); n++) {
      |                 ^
cc1: all warnings being treated as errors
make[1]: *** [Makefile:7277: monitor/packet.o] Error 1
make: *** [Makefile:4310: all] Error 2


##############################
Test: Incremental Build with patches - FAIL
Desc: Incremental build per patch in the series
Output:
monitor/packet.c: In function ‘print_le_states’:
monitor/packet.c:2836:17: error: comparison of integer expressions of different signedness: ‘int’ and ‘long unsigned int’ [-Werror=sign-compare]
 2836 |   for (n = 0; n < ARRAY_SIZE(le_states_desc_table); n++) {
      |                 ^
cc1: all warnings being treated as errors
make[1]: *** [Makefile:7277: monitor/packet.o] Error 1
make: *** [Makefile:4310: all] Error 2




---
Regards,
Linux Bluetooth

Patch

diff --git a/monitor/packet.c b/monitor/packet.c
index b7431b57d..c61d6bd4b 100644
--- a/monitor/packet.c
+++ b/monitor/packet.c
@@ -2833,7 +2833,7 @@  static void print_le_states(const uint8_t *states_array)
 		if (!(states & val))
 			continue;
 
-		for (n = 0; n < 16; n++) {
+		for (n = 0; n < ARRAY_SIZE(le_states_desc_table); n++) {
 			if (le_states_comb_table[i].states & (1 << n))
 				str[num++] = le_states_desc_table[n].str;
 		}