* [PATCH v2][next] wireless: wext-spy: Fix out-of-bounds warning
@ 2021-04-22 20:00 Gustavo A. R. Silva
2021-04-22 20:04 ` Johannes Berg
0 siblings, 1 reply; 4+ messages in thread
From: Gustavo A. R. Silva @ 2021-04-22 20:00 UTC (permalink / raw)
To: Johannes Berg, David S. Miller, Jakub Kicinski
Cc: linux-wireless, netdev, linux-kernel, Gustavo A. R. Silva,
linux-hardening, Kees Cook
Fix the following out-of-bounds warning:
net/wireless/wext-spy.c:178:2: warning: 'memcpy' offset [25, 28] from the object at 'threshold' is out of the bounds of referenced subobject 'low' with type 'struct iw_quality' at offset 20 [-Warray-bounds]
The problem is that the original code is trying to copy data into a
couple of struct members adjacent to each other in a single call to
memcpy(). This causes a legitimate compiler warning because memcpy()
overruns the length of &threshold.low and &spydata->spy_thr_low. As
these are just a couple of struct members, fix this by using direct
assignments, instead of memcpy().
This helps with the ongoing efforts to globally enable -Warray-bounds
and get us closer to being able to tighten the FORTIFY_SOURCE routines
on memcpy().
Link: https://github.com/KSPP/linux/issues/109
Reported-by: kernel test robot <lkp@intel.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
---
Changes in v2:
- Use direct struct assignments instead of memcpy().
- Fix one more instance of this same issue in function
iw_handler_get_thrspy().
- Update changelog text.
- Add Kees' RB tag.
net/wireless/wext-spy.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/net/wireless/wext-spy.c b/net/wireless/wext-spy.c
index 33bef22e44e9..b379a0371653 100644
--- a/net/wireless/wext-spy.c
+++ b/net/wireless/wext-spy.c
@@ -120,8 +120,8 @@ int iw_handler_set_thrspy(struct net_device * dev,
return -EOPNOTSUPP;
/* Just do it */
- memcpy(&(spydata->spy_thr_low), &(threshold->low),
- 2 * sizeof(struct iw_quality));
+ spydata->spy_thr_low = threshold->low;
+ spydata->spy_thr_high = threshold->high;
/* Clear flag */
memset(spydata->spy_thr_under, '\0', sizeof(spydata->spy_thr_under));
@@ -147,8 +147,8 @@ int iw_handler_get_thrspy(struct net_device * dev,
return -EOPNOTSUPP;
/* Just do it */
- memcpy(&(threshold->low), &(spydata->spy_thr_low),
- 2 * sizeof(struct iw_quality));
+ threshold->low = spydata->spy_thr_low;
+ threshold->high = spydata->spy_thr_high;
return 0;
}
@@ -173,10 +173,10 @@ static void iw_send_thrspy_event(struct net_device * dev,
memcpy(threshold.addr.sa_data, address, ETH_ALEN);
threshold.addr.sa_family = ARPHRD_ETHER;
/* Copy stats */
- memcpy(&(threshold.qual), wstats, sizeof(struct iw_quality));
+ threshold.qual = *wstats;
/* Copy also thresholds */
- memcpy(&(threshold.low), &(spydata->spy_thr_low),
- 2 * sizeof(struct iw_quality));
+ threshold.low = spydata->spy_thr_low;
+ threshold.high = spydata->spy_thr_high;
/* Send event to user space */
wireless_send_event(dev, SIOCGIWTHRSPY, &wrqu, (char *) &threshold);
--
2.27.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2][next] wireless: wext-spy: Fix out-of-bounds warning
2021-04-22 20:00 [PATCH v2][next] wireless: wext-spy: Fix out-of-bounds warning Gustavo A. R. Silva
@ 2021-04-22 20:04 ` Johannes Berg
2021-04-22 20:10 ` Gustavo A. R. Silva
2021-04-22 20:48 ` Kees Cook
0 siblings, 2 replies; 4+ messages in thread
From: Johannes Berg @ 2021-04-22 20:04 UTC (permalink / raw)
To: Gustavo A. R. Silva, David S. Miller, Jakub Kicinski
Cc: linux-wireless, netdev, linux-kernel, linux-hardening, Kees Cook
On Thu, 2021-04-22 at 15:00 -0500, Gustavo A. R. Silva wrote:
>
> Changes in v2:
> - Use direct struct assignments instead of memcpy().
> - Fix one more instance of this same issue in function
> iw_handler_get_thrspy().
> - Update changelog text.
Thanks.
> - Add Kees' RB tag.
He probably won't mind in this case, but you did some pretty substantial
changes to the patch, so I really wouldn't recommend keeping it there.
johannes
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2][next] wireless: wext-spy: Fix out-of-bounds warning
2021-04-22 20:04 ` Johannes Berg
@ 2021-04-22 20:10 ` Gustavo A. R. Silva
2021-04-22 20:48 ` Kees Cook
1 sibling, 0 replies; 4+ messages in thread
From: Gustavo A. R. Silva @ 2021-04-22 20:10 UTC (permalink / raw)
To: Kees Cook, Johannes Berg, Gustavo A. R. Silva, David S. Miller,
Jakub Kicinski
Cc: linux-wireless, netdev, linux-kernel, linux-hardening
On 4/22/21 15:04, Johannes Berg wrote:
> On Thu, 2021-04-22 at 15:00 -0500, Gustavo A. R. Silva wrote:
>>
>> Changes in v2:
>> - Use direct struct assignments instead of memcpy().
>> - Fix one more instance of this same issue in function
>> iw_handler_get_thrspy().
>> - Update changelog text.
>
> Thanks.
>
>> - Add Kees' RB tag.
>
> He probably won't mind in this case, but you did some pretty substantial
> changes to the patch, so I really wouldn't recommend keeping it there.
Right.
Kees, could you please confirm you RB tag in this new version?
Thanks
--
Gustavo
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2][next] wireless: wext-spy: Fix out-of-bounds warning
2021-04-22 20:04 ` Johannes Berg
2021-04-22 20:10 ` Gustavo A. R. Silva
@ 2021-04-22 20:48 ` Kees Cook
1 sibling, 0 replies; 4+ messages in thread
From: Kees Cook @ 2021-04-22 20:48 UTC (permalink / raw)
To: Johannes Berg
Cc: Gustavo A. R. Silva, David S. Miller, Jakub Kicinski,
linux-wireless, netdev, linux-kernel, linux-hardening
On Thu, Apr 22, 2021 at 10:04:29PM +0200, Johannes Berg wrote:
> On Thu, 2021-04-22 at 15:00 -0500, Gustavo A. R. Silva wrote:
> >
> > Changes in v2:
> > - Use direct struct assignments instead of memcpy().
> > - Fix one more instance of this same issue in function
> > iw_handler_get_thrspy().
> > - Update changelog text.
>
> Thanks.
>
> > - Add Kees' RB tag.
>
> He probably won't mind in this case, but you did some pretty substantial
> changes to the patch, so I really wouldn't recommend keeping it there.
Thanks for double-checking! Yeah, I'm fine with it; Gustavo and I had
talked in the past about similar solutions in other places, so he
forwarded the intent from those conversations. (Not that you had any
visibility into that!) But, yes, still:
Reviewed-by: Kees Cook <keescook@chromium.org>
Thanks!
--
Kees Cook
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-04-22 20:48 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-22 20:00 [PATCH v2][next] wireless: wext-spy: Fix out-of-bounds warning Gustavo A. R. Silva
2021-04-22 20:04 ` Johannes Berg
2021-04-22 20:10 ` Gustavo A. R. Silva
2021-04-22 20:48 ` Kees Cook
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.