On Tue, Jun 20, 2023 at 01:07:37PM +0300, Dmitry Antipov wrote: > When compiling with gcc 13.1 and CONFIG_FORTIFY_SOURCE=y, > I've noticed the following: > > In function ‘fortify_memcpy_chk’, > inlined from ‘mwifiex_construct_tdls_action_frame’ at drivers/net/wireless/marvell/mwifiex/tdls.c:765:3, > inlined from ‘mwifiex_send_tdls_action_frame’ at drivers/net/wireless/marvell/mwifiex/tdls.c:856:6: > ./include/linux/fortify-string.h:529:25: warning: call to ‘__read_overflow2_field’ > declared with attribute warning: detected read beyond size of field (2nd parameter); > maybe use struct_group()? [-Wattribute-warning] > 529 | __read_overflow2_field(q_size_field, size); > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > The compiler actually complains on: > > memmove(pos + ETH_ALEN, &mgmt->u.action.category, > sizeof(mgmt->u.action.u.tdls_discover_resp)); > > and it happens because the fortification logic interprets this > as an attempt to overread 1-byte 'u.action.category' member of > 'struct ieee80211_mgmt'. To silence this warning, it's enough > to pass an address of 'u.action' itself instead of an address > of its first member. > > Signed-off-by: Dmitry Antipov <dmantipov@xxxxxxxxx> > --- > drivers/net/wireless/marvell/mwifiex/tdls.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/wireless/marvell/mwifiex/tdls.c b/drivers/net/wireless/marvell/mwifiex/tdls.c > index 97bb87c3676b..5a2941965757 100644 > --- a/drivers/net/wireless/marvell/mwifiex/tdls.c > +++ b/drivers/net/wireless/marvell/mwifiex/tdls.c > @@ -762,7 +762,7 @@ mwifiex_construct_tdls_action_frame(struct mwifiex_private *priv, > mgmt->u.action.u.tdls_discover_resp.capability = > cpu_to_le16(capab); > /* move back for addr4 */ > - memmove(pos + ETH_ALEN, &mgmt->u.action.category, > + memmove(pos + ETH_ALEN, &mgmt->u.action, > sizeof(mgmt->u.action.u.tdls_discover_resp)); This invocation seems a bit suspect, as it uses a 'sizeof' of a field that doesn't match the actual pointer (it's off by 1 byte), but that's not your fault. I suppose it's no wonder we had so many problems with TDLS support on mwifiex... Anyway, the refactor looks fine: Reviewed-by: Brian Norris <briannorris@xxxxxxxxxxxx> > /* init address 4 */ > eth_broadcast_addr(pos); > -- > 2.41.0 >
