RE: meshctl: set-pub fails

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Steve,

I believe you have identified a legitimate bug.

However, this is the only part of your fix that I think is valid:
> -	segN = SEG_MAX(len);
> +	segN = SEG_MAX(len + sizeof(uint32_t));

len at this point is the unencrypted length of the outgoing payload, and so it does indeed need the size of the MIC taken into consideration before determining network Segmentation.  As the Set Publication message is now 12 octets long, including the opcode, this is one byte too many for an unsegmented message, so segN should in fact be 1 (a two segment message).

However, the other adjustments are incorrect. The length of "data" in the mesh pkt is 30 octets because over the ADV bearer, the Maximimum payload is 29 octets.  The extra 1 octet at the front is the Mesh (GAT) Proxy segmentation (not to be confused with Mesh Transport segmentation).  So the maximum size of the mesh_pkt data is 30. Because this is just a Proxy transport, we still have to obey the ADV size limitation.

Fixing the segN length in net_access_layer_send() should be enough to fix your problem, and you should see *two* segments going out due to this message, and you should receive a SEG ACK in response showing 2 segments ACKed.



> -----Original Message-----
> From: linux-bluetooth-owner@xxxxxxxxxxxxxxx [mailto:linux-bluetooth-
> owner@xxxxxxxxxxxxxxx] On Behalf Of Steve Brown
> Sent: Wednesday, October 11, 2017 7:45 AM
> To: linux-bluetooth@xxxxxxxxxxxxxxx
> Subject: Re: meshctl: set-pub fails
> 
> On Tue, 2017-10-10 at 16:30 -0600, Steve Brown wrote:
> > On Tue, 2017-10-10 at 10:04 -0600, Steve Brown wrote:
> > > discover-unprovisioned, provision, add-appkey & bind work as
> > > expected.
> > > The meshctl's onoff client evokes expected behavior in zephyr's
> > > onoff server.
> > >
> > > The command being passed
> > >
> > > set-pub 0100 c000 1 5 1000
> > >
> > > The length of the set-pub packet exceeded the size of the data array
> > > in struct mesh_pkt in net.c by 1. This stepped on the length field
> > > which followed. The transmit mostly failed. The zephyr server
> > > received nothing.
> > >
> > > I extended the data array to 35. I now get consistent output.
> > > However,
> > > the zephyr server is unable to decrypt with devkey. Net decrypt
> > > works, but app decrypt fails.
> > >
> > > I looked at how config-client.c:cmd_set_pub handled the app key
> > > index.
> > > According to 4.3.2.16 (pg 158) of the Mesh Profile Spec, the app key
> > > index should be in the top 12 bits of octet 4 & 5. I shifted it up
> > > by 4, but get the same results.
> > >
> > > I'm pretty sure the problem is on the meshctl side. I've used the
> > > Silabs Android app to successfully configure the zephyr server. It
> > > successfully sends a set-pub which the zephyr server correctly
> > > handles.
> > >
> > > Maybe this is a regression. Has set-pub worked in the past?
> > >
> >
> > There is no problem with the placement of the app key index. The
> > figures in the spec are variously big and little endian. I misread the
> > figure.
> >
> > Steve
> 
> Sorry to be continually replying to my own posts.
> 
> There were 2 problems:
> 
> 1. mesh_pkt's data area was too small. The final message was 31 bytes.
> I added 5 bytes and an assert. I'm not sure what the max might be.
> 
> 2. The SEG_MAX test was on the wrong length. It should have been on
> sar->len, not len. The message should have been segmented, but was not.
> 
> With this patch, the set-pub below returns success.
> 
> set-pub 0100 c000 1 0 1000
> 
> Could somebody familiar with the code comment?
> 
> diff --git a/mesh/net.c b/mesh/net.c
> index 2d75c4f7d..89d22c99b 100644
> --- a/mesh/net.c
> +++ b/mesh/net.c
> @@ -110,7 +110,7 @@ struct mesh_virt_addr {  };
> 
>  struct mesh_pkt {
> -	uint8_t		data[30];
> +	uint8_t		data[35];
>  	uint8_t		len;
>  };
> 
> @@ -1308,6 +1308,9 @@ static void send_seg(struct mesh_sar_msg *sar,
> uint8_t seg)
>  	}
> 
>  	pkt->len += (sar->ctl ? 8 : 4);
> +
> +	g_assert(!(pkt->len > sizeof(pkt->data)));
> +
>  	mesh_crypto_packet_encode(data, pkt->len,
>  			part->enc_key,
>  			sar->iv_index,
> @@ -2098,7 +2101,7 @@ bool net_access_layer_send(uint8_t ttl, uint16_t
> src, uint32_t dst,
>  	if (!result)
>  		return false;
> 
> -	segN = SEG_MAX(len);
> +	segN = SEG_MAX(len + sizeof(uint32_t));
> 
>  	/* Only one ACK required SAR message per destination at a time */
>  	if (segN && IS_UNICAST(dst)) {
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo
> info at  http://vger.kernel.org/majordomo-info.html
��.n��������+%������w��{.n�����{����^n�r������&��z�ޗ�zf���h���~����������_��+v���)ߣ�

[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux