[PATCH BlueZ 0/1] Fix segmentation fault after adding second network key

[PATCH BlueZ 0/1] Fix segmentation fault after adding second network key

[Jakub Witowski]

Segmentation fault occured when we want to add second network key via
NETWORK KEY ADD opcode (0x8040). It was caused by passing 'subnet' pointer (which was NULL) 
to the start_network_beacon() function.

Jakub Witowski (1):
  mesh: Fix segmentation fault after adding second netkey via
    NET_KEY_ADD opcode

 mesh/net.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

-- 
2.20.1

[PATCH BlueZ 1/1] mesh: Fix segmentation fault after adding second netkey via NET_KEY_ADD opcode

[Jakub Witowski]

Segmentation fault was caused by passing subnet pointer to the
start_network_beacon() which was NULL
---
 mesh/net.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/mesh/net.c b/mesh/net.c
index c7aff9ab4..87a861bf0 100644
--- a/mesh/net.c
+++ b/mesh/net.c
@@ -1017,6 +1017,8 @@ int mesh_net_add_key(struct mesh_net *net, uint16_t idx, const uint8_t *value)
 			return MESH_STATUS_SUCCESS;
 		else
 			return MESH_STATUS_IDX_ALREADY_STORED;
+	} else {
+		subnet = subnet_new(net, idx);
 	}
 
 	status = add_key(net, idx, value);
@@ -2490,7 +2492,7 @@ static void net_rx(void *net_ptr, void *user_data)
 	int8_t rssi = 0;
 
 	key_id = net_key_decrypt(net->iv_index, data->data, data->len,
-								&out, &out_size);
+			&out, &out_size);
 
 	if (!key_id)
 		return;
-- 
2.20.1

Re: [PATCH BlueZ 1/1] mesh: Fix segmentation fault after adding second netkey via NET_KEY_ADD opcode

[Stotland, Inga]

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

Hi Jakub,

On Thu, 2019-06-06 at 13:59 +0200, Jakub Witowski wrote:
> Segmentation fault was caused by passing subnet pointer to the
> start_network_beacon() which was NULL
> ---
>  mesh/net.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/mesh/net.c b/mesh/net.c
> index c7aff9ab4..87a861bf0 100644
> --- a/mesh/net.c
> +++ b/mesh/net.c
> @@ -1017,6 +1017,8 @@ int mesh_net_add_key(struct mesh_net *net,
> uint16_t idx, const uint8_t *value)
>  			return MESH_STATUS_SUCCESS;
>  		else
>  			return MESH_STATUS_IDX_ALREADY_STORED;
> +	} else {
> +		subnet = subnet_new(net, idx);
>  	}

Good catch: this is a regression.
Could you please make a slighty different change that will simplify the
code overall:

static add_key() returns only two types of error codes:
MESH_STATUS_SUCCESS & MESH_STATUS_INSUFF_RESOURCES. To simplify the
code, it makes sense to change the prototype so that instead of status
code it returns a pointer to the new subnet (NULL in case of failure).
Then the return value is examined and in case it's NULL,
MESH_STATUS_INSUFF_RESOURCES is returned in mesh_net_add_key() and
"false" is returned in mesh_net_set_key()

>  
>  	status = add_key(net, idx, value);
> @@ -2490,7 +2492,7 @@ static void net_rx(void *net_ptr, void
> *user_data)
>  	int8_t rssi = 0;
>  
>  	key_id = net_key_decrypt(net->iv_index, data->data, data->len,
> -								&out,
> &out_size);
> +			&out, &out_size);

Let's not change the alignment. From the bluez/doc/coding-style.txt:
"The referred style for line wrapping is to indent as far as possible
to the right without hitting the 80 columns limit."

>  
>  	if (!key_id)
>  		return;

Best regards,

Inga


[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3265 bytes --]