2022-05-08 00:21:41

by Ildar Kamaletdinov

[permalink] [raw]
Subject: [PATCH BlueZ 0/4] Fix few more bugs found by SVACE

This patch set includes few fixes for bugs that was found by
Linux Verification Center (linuxtesting.org) with the SVACE static
analysis tool.

Ildar Kamaletdinov (4):
tools: Fix memory leak in hciconfig
tools: Fix memory leaks in btgatt-server/client
tools: Fix handle leak in rfcomm
device: Fix uninitialized value usage

src/device.c | 3 ++-
tools/btgatt-client.c | 5 ++++-
tools/btgatt-server.c | 5 ++++-
tools/hciconfig.c | 5 ++++-
tools/rfcomm.c | 4 ++++
5 files changed, 18 insertions(+), 4 deletions(-)

--
2.35.3



2022-05-08 07:43:29

by Ildar Kamaletdinov

[permalink] [raw]
Subject: [PATCH BlueZ 1/4] tools: Fix memory leak in hciconfig

printf() was using function that return dynamic allocated memory as
a parameter.

Found by Linux Verification Center (linuxtesting.org) with the SVACE
static analysis tool.
---
tools/hciconfig.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/hciconfig.c b/tools/hciconfig.c
index e4d521583..2619285d5 100644
--- a/tools/hciconfig.c
+++ b/tools/hciconfig.c
@@ -80,7 +80,10 @@ static void print_pkt_type(struct hci_dev_info *di)

static void print_link_policy(struct hci_dev_info *di)
{
- printf("\tLink policy: %s\n", hci_lptostr(di->link_policy));
+ char *str;
+ str = hci_lptostr(di->link_policy);
+ printf("\tLink policy: %s\n", str);
+ bt_free(str);
}

static void print_link_mode(struct hci_dev_info *di)
--
2.35.3


2022-05-09 04:45:19

by Ildar Kamaletdinov

[permalink] [raw]
Subject: [PATCH BlueZ 2/4] tools: Fix memory leaks in btgatt-server/client

According to man buffer allocated by getline() should be freed by
the user program even if getline() failed.

Found by Linux Verification Center (linuxtesting.org) with the SVACE
static analysis tool.
---
tools/btgatt-client.c | 5 ++++-
tools/btgatt-server.c | 5 ++++-
2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/tools/btgatt-client.c b/tools/btgatt-client.c
index 8c9365aa2..9447062fb 100644
--- a/tools/btgatt-client.c
+++ b/tools/btgatt-client.c
@@ -1355,12 +1355,15 @@ static void prompt_read_cb(int fd, uint32_t events, void *user_data)
return;
}

- if ((read = getline(&line, &len, stdin)) == -1)
+ if ((read = getline(&line, &len, stdin)) == -1) {
+ free(line);
return;
+ }

if (read <= 1) {
cmd_help(cli, NULL);
print_prompt();
+ free(line);
return;
}

diff --git a/tools/btgatt-server.c b/tools/btgatt-server.c
index 4a5d2b720..90a6c9b0a 100644
--- a/tools/btgatt-server.c
+++ b/tools/btgatt-server.c
@@ -1080,12 +1080,15 @@ static void prompt_read_cb(int fd, uint32_t events, void *user_data)
}

read = getline(&line, &len, stdin);
- if (read < 0)
+ if (read < 0) {
+ free(line);
return;
+ }

if (read <= 1) {
cmd_help(server, NULL);
print_prompt();
+ free(line);
return;
}

--
2.35.3


2022-05-09 05:32:50

by Ildar Kamaletdinov

[permalink] [raw]
Subject: [PATCH BlueZ 4/4] device: Fix uninitialized value usage

Definitely `dbus_bool_t b;` must be initialized before comparing it
with current value.

Found by Linux Verification Center (linuxtesting.org) with the SVACE
static analysis tool.
---
src/device.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/device.c b/src/device.c
index 6da5c380b..7114e1b3e 100644
--- a/src/device.c
+++ b/src/device.c
@@ -1568,6 +1568,8 @@ static void dev_property_set_wake_allowed(const GDBusPropertyTable *property,
return;
}

+ dbus_message_iter_get_basic(value, &b);
+
/* Emit busy or success depending on current value. */
if (b == device->pending_wake_allowed) {
if (device->wake_allowed == device->pending_wake_allowed)
@@ -1580,7 +1582,6 @@ static void dev_property_set_wake_allowed(const GDBusPropertyTable *property,
return;
}

- dbus_message_iter_get_basic(value, &b);
device_set_wake_override(device, b);
device_set_wake_allowed(device, b, id);
}
--
2.35.3


2022-05-09 08:00:29

by bluez.test.bot

[permalink] [raw]
Subject: RE: Fix few more bugs found by SVACE

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=639408

---Test result---

Test Summary:
CheckPatch FAIL 5.63 seconds
GitLint PASS 3.84 seconds
Prep - Setup ELL PASS 42.34 seconds
Build - Prep PASS 0.68 seconds
Build - Configure PASS 8.45 seconds
Build - Make PASS 1317.42 seconds
Make Check PASS 11.74 seconds
Make Check w/Valgrind PASS 438.85 seconds
Make Distcheck PASS 226.73 seconds
Build w/ext ELL - Configure PASS 8.47 seconds
Build w/ext ELL - Make PASS 1317.12 seconds
Incremental Build with patchesPASS 5316.66 seconds

Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script with rule in .checkpatch.conf
Output:
[BlueZ,1/4] tools: Fix memory leak in hciconfig
WARNING:LINE_SPACING: Missing a blank line after declarations
#68: FILE: tools/hciconfig.c:84:
+ char *str;
+ str = hci_lptostr(di->link_policy);

/github/workspace/src/12842044.patch total: 0 errors, 1 warnings, 11 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/12842044.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,2/4] tools: Fix memory leaks in btgatt-server/client
ERROR:ASSIGN_IN_IF: do not use assignment in if condition
#68: FILE: tools/btgatt-client.c:1358:
+ if ((read = getline(&line, &len, stdin)) == -1) {

/github/workspace/src/12842046.patch total: 1 errors, 0 warnings, 32 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/12842046.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.




---
Regards,
Linux Bluetooth

2022-05-09 09:35:28

by Ildar Kamaletdinov

[permalink] [raw]
Subject: [PATCH BlueZ 3/4] tools: Fix handle leak in rfcomm

Some branches of execution can make handle (socket) leakage.

Found by Linux Verification Center (linuxtesting.org) with the SVACE
static analysis tool.
---
tools/rfcomm.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/tools/rfcomm.c b/tools/rfcomm.c
index cd520aa44..e013ff588 100644
--- a/tools/rfcomm.c
+++ b/tools/rfcomm.c
@@ -298,6 +298,7 @@ static void cmd_connect(int ctl, int dev, bdaddr_t *bdaddr, int argc, char **arg

if (setsockopt(sk, SOL_SOCKET, SO_LINGER, &l, sizeof(l)) < 0) {
perror("Can't set linger option");
+ close(sk);
return;
}
}
@@ -466,6 +467,7 @@ static void cmd_listen(int ctl, int dev, bdaddr_t *bdaddr, int argc, char **argv
if (getsockname(nsk, (struct sockaddr *)&laddr, &alen) < 0) {
perror("Can't get RFCOMM socket name");
close(nsk);
+ close(sk);
return;
}

@@ -475,6 +477,7 @@ static void cmd_listen(int ctl, int dev, bdaddr_t *bdaddr, int argc, char **argv
if (setsockopt(nsk, SOL_SOCKET, SO_LINGER, &l, sizeof(l)) < 0) {
perror("Can't set linger option");
close(nsk);
+ close(sk);
return;
}
}
@@ -490,6 +493,7 @@ static void cmd_listen(int ctl, int dev, bdaddr_t *bdaddr, int argc, char **argv
dev = ioctl(nsk, RFCOMMCREATEDEV, &req);
if (dev < 0) {
perror("Can't create RFCOMM TTY");
+ close(nsk);
close(sk);
return;
}
--
2.35.3