2020-10-27 00:03:18

by Stotland, Inga

Subject: [PATCH BlueZ] tools/mesh-cfgclient: Fix errors found by static analysis

This fixes a NULL pointer dereference error in subscription_cmd().

Also re-order calling sequence for l_free() & l_queue_remove()
in msg_recvd(): even though technically it is not a bug to pass
a value of a freed pointer to l_queue_remove(), it's a poor form
and confuses the analyzer.
---
tools/mesh/cfgcli.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/tools/mesh/cfgcli.c b/tools/mesh/cfgcli.c
index 1c20db85a..d8eee4edc 100644
--- a/tools/mesh/cfgcli.c
+++ b/tools/mesh/cfgcli.c
@@ -410,8 +410,8 @@ static bool msg_recvd(uint16_t src, uint16_t idx, uint8_t *data,
req = get_req_by_rsp(src, opcode);
if (req) {
cmd = req->cmd;
- free_request(req);
l_queue_remove(requests, req);
+ free_request(req);
} else
cmd = NULL;

@@ -1470,15 +1470,14 @@ static void subscription_cmd(int argc, char *argv[], uint32_t opcode)

grp = l_queue_find(groups, match_group_addr, L_UINT_TO_PTR(sub_addr));

- if (!grp && opcode != OP_CONFIG_MODEL_SUB_DELETE) {
- grp = add_group(sub_addr);
-
- if (!grp && IS_VIRTUAL(sub_addr)) {
- print_virtual_not_found(sub_addr);
- return bt_shell_noninteractive_quit(EXIT_FAILURE);
- }
+ if (!grp && IS_VIRTUAL(sub_addr)) {
+ print_virtual_not_found(sub_addr);
+ return bt_shell_noninteractive_quit(EXIT_FAILURE);
}

+ if (!grp && opcode != OP_CONFIG_MODEL_SUB_DELETE)
+ grp = add_group(sub_addr);
+
if (IS_VIRTUAL(sub_addr)) {
if (opcode == OP_CONFIG_MODEL_SUB_ADD)
opcode = OP_CONFIG_MODEL_SUB_VIRT_ADD;
--
2.26.2

2020-10-27 00:04:11

by bluez.test.bot

Subject: RE: [BlueZ] tools/mesh-cfgclient: Fix errors found by static analysis

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

---Test result---

##############################
Test: CheckPatch - PASS

##############################
Test: CheckGitLint - PASS

##############################
Test: CheckBuild - PASS

##############################
Test: MakeCheck - PASS



---
Regards,
Linux Bluetooth

2020-10-29 15:23:28

by Gix, Brian

Subject: Re: [PATCH BlueZ] tools/mesh-cfgclient: Fix errors found by static analysis

Applied

On Mon, 2020-10-26 at 14:08 -0700, Inga Stotland wrote:
> This fixes a NULL pointer dereference error in subscription_cmd().
>
> Also re-order calling sequence for l_free() & l_queue_remove()
> in msg_recvd(): even though technically it is not a bug to pass
> a value of a freed pointer to l_queue_remove(), it's a poor form
> and confuses the analyzer.
> ---
> tools/mesh/cfgcli.c | 15 +++++++--------
> 1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/tools/mesh/cfgcli.c b/tools/mesh/cfgcli.c
> index 1c20db85a..d8eee4edc 100644
> --- a/tools/mesh/cfgcli.c
> +++ b/tools/mesh/cfgcli.c
> @@ -410,8 +410,8 @@ static bool msg_recvd(uint16_t src, uint16_t idx, uint8_t *data,
> req = get_req_by_rsp(src, opcode);
> if (req) {
> cmd = req->cmd;
> - free_request(req);
> l_queue_remove(requests, req);
> + free_request(req);
> } else
> cmd = NULL;
>
> @@ -1470,15 +1470,14 @@ static void subscription_cmd(int argc, char *argv[], uint32_t opcode)
>
> grp = l_queue_find(groups, match_group_addr, L_UINT_TO_PTR(sub_addr));
>
> - if (!grp && opcode != OP_CONFIG_MODEL_SUB_DELETE) {
> - grp = add_group(sub_addr);
> -
> - if (!grp && IS_VIRTUAL(sub_addr)) {
> - print_virtual_not_found(sub_addr);
> - return bt_shell_noninteractive_quit(EXIT_FAILURE);
> - }
> + if (!grp && IS_VIRTUAL(sub_addr)) {
> + print_virtual_not_found(sub_addr);
> + return bt_shell_noninteractive_quit(EXIT_FAILURE);
> }
>
> + if (!grp && opcode != OP_CONFIG_MODEL_SUB_DELETE)
> + grp = add_group(sub_addr);
> +
> if (IS_VIRTUAL(sub_addr)) {
> if (opcode == OP_CONFIG_MODEL_SUB_ADD)
> opcode = OP_CONFIG_MODEL_SUB_VIRT_ADD;