[PATCH] hostapd: Fix rnr ie length when no need to report bss

[Allen Ye (葉芷勳)]

On Sat, 2023-10-28 at 19:44 +0300, Jouni Malinen wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  On Fri, Aug 18, 2023 at 03:16:39PM +0800, Allen Ye wrote:
> > Fix rnr ie length when no need to report bss. If we don't have
> content in
> > TBTT then don't need to change the length of the ie (*size_offset).
> 
> > diff --git a/src/ap/ieee802_11.c b/src/ap/ieee802_11.c
> > @@ -7484,8 +7484,10 @@ static u8 * hostapd_eid_rnr_iface(struct
> hostapd_data *hapd,
> 
> >  start = i;
> > -*tbtt_count_pos = RNR_TBTT_INFO_COUNT(tbtt_count - 1);
> > -*size_offset = (eid - size_offset) - 1;
> > +if (tbtt_count != 0) {
> > +*tbtt_count_pos = RNR_TBTT_INFO_COUNT(tbtt_count - 1);
> > +*size_offset = (eid - size_offset) - 1;
> > +}
> 
> That tbtt_count - 1 part looks obviously wrong for tbtt_count == 0,
> but
> does this actually fix the full issue here? The eid pointer has
> already
> been advanced by four bytes at this point, i.e., the fixed fields of
> the
> Neighbor AP Information field has already been written here (with the
> first octet not explicitly written, but it would likely be
> initialized
> to 0). Those four bytes would remain here and the next iteration of
> the
> while loop might add more entries after this with another set of
> those
> four bytes. And that would seem to result in invalid information.
> 
> In addition to making those two operations conditional on tbtt_count
> >
> 0, should this also restore eid and len back to their previous values
> (i.e., something like eid -= RNR_TBTT_HEADER_LEN; len -=
> RNR_TBTT_HEADER_LEN;)?
> 
> -- 
> Jouni Malinen                                            PGP id
> EFC895FA

Hi Jouni,

On my opinion, your concern here will not happen. There are two
situations that hostapd need another Neighbor AP Information field.
The first is the current length of Neighbor AP Information + next
RNR_TBTT_INFO_LEN > 255 and the other is the current tbtt_count >=
RNR_TBTT_INFO_COUNT_MAX.
That will break the for loop and go through next while iterator.
In these two situations, the next tbtt information must be inserted in
Neighbor AP Information field, because it has passed the valid checks
in for loop of previous while iteration.
Therefore, I think we don't need to restore eid and len back even if
there are more than one while iterations.

Thanks,
Allen