2019-06-06 12:46:57

by Jakub Witowski

[permalink] [raw]
Subject: [PATCH BlueZ 0/1] Fix segmentation fault after adding second network key

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


2019-06-06 12:48:14

by Jakub Witowski

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

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

2019-06-06 20:16:37

by Stotland, Inga

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

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


Attachments:
smime.p7s (3.19 kB)