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

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

[Inga Stotland]

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

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

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 557 bytes --]

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

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

[Gix, Brian]

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;