Message ID | 20210422200032.GA168995@embeddedor (mailing list archive) |
---|---|
State | Accepted |
Delegated to: | Johannes Berg |
Headers | show |
Series | [v2,next] wireless: wext-spy: Fix out-of-bounds warning | expand |
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
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
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!
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);