* [PATCH] wext: handle NULL exta data in iwe_stream_add_point better
@ 2017-01-11 14:35 Arnd Bergmann
2017-01-11 14:38 ` Johannes Berg
0 siblings, 1 reply; 9+ messages in thread
From: Arnd Bergmann @ 2017-01-11 14:35 UTC (permalink / raw)
To: Johannes Berg
Cc: linux-wireless, Arnd Bergmann, David S. Miller, Johannes Berg,
netdev, linux-kernel
gcc-7 complains that wl3501_cs passes NULL into a function that
then uses the argument as the input for memcpy:
drivers/net/wireless/wl3501_cs.c: In function 'wl3501_get_scan':
include/net/iw_handler.h:559:3: error: argument 2 null where non-null expected [-Werror=nonnull]
memcpy(stream + point_len, extra, iwe->u.data.length);
This works fine here because iwe->u.data.length is guaranteed to be
NULL, and the memcpy doesn't actually have an effect.
Making the length check explicit avoids the warning and should have
no other effect here.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
include/net/iw_handler.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/net/iw_handler.h b/include/net/iw_handler.h
index e0f4109e64c6..1a41043688bc 100644
--- a/include/net/iw_handler.h
+++ b/include/net/iw_handler.h
@@ -556,7 +556,8 @@ iwe_stream_add_point(struct iw_request_info *info, char *stream, char *ends,
memcpy(stream + lcp_len,
((char *) &iwe->u) + IW_EV_POINT_OFF,
IW_EV_POINT_PK_LEN - IW_EV_LCP_PK_LEN);
- memcpy(stream + point_len, extra, iwe->u.data.length);
+ if (iwe->u.data.length)
+ memcpy(stream + point_len, extra, iwe->u.data.length);
stream += event_len;
}
return stream;
--
2.9.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] wext: handle NULL exta data in iwe_stream_add_point better
2017-01-11 14:35 [PATCH] wext: handle NULL exta data in iwe_stream_add_point better Arnd Bergmann
@ 2017-01-11 14:38 ` Johannes Berg
2017-01-11 15:00 ` Arnd Bergmann
0 siblings, 1 reply; 9+ messages in thread
From: Johannes Berg @ 2017-01-11 14:38 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: linux-wireless, David S. Miller, netdev, linux-kernel
On Wed, 2017-01-11 at 15:35 +0100, Arnd Bergmann wrote:
> gcc-7 complains that wl3501_cs passes NULL into a function that
> then uses the argument as the input for memcpy:
>
> drivers/net/wireless/wl3501_cs.c: In function 'wl3501_get_scan':
> include/net/iw_handler.h:559:3: error: argument 2 null where non-null
> expected [-Werror=nonnull]
> memcpy(stream + point_len, extra, iwe->u.data.length);
I love wext ;-)
> This works fine here because iwe->u.data.length is guaranteed to be
> NULL, and the memcpy doesn't actually have an effect.
I think you mean 0, not NULL, but I can fix that when I apply it.
johannes
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] wext: handle NULL exta data in iwe_stream_add_point better
2017-01-11 14:38 ` Johannes Berg
@ 2017-01-11 15:00 ` Arnd Bergmann
2017-01-11 15:06 ` Johannes Berg
0 siblings, 1 reply; 9+ messages in thread
From: Arnd Bergmann @ 2017-01-11 15:00 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, David S. Miller, Networking, linux-kernel
On Wed, Jan 11, 2017 at 3:38 PM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Wed, 2017-01-11 at 15:35 +0100, Arnd Bergmann wrote:
>> This works fine here because iwe->u.data.length is guaranteed to be
>> NULL, and the memcpy doesn't actually have an effect.
>
> I think you mean 0, not NULL, but I can fix that when I apply it.
Right, thanks!
Arnd
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] wext: handle NULL exta data in iwe_stream_add_point better
2017-01-11 15:00 ` Arnd Bergmann
@ 2017-01-11 15:06 ` Johannes Berg
2017-01-11 20:39 ` Arnd Bergmann
0 siblings, 1 reply; 9+ messages in thread
From: Johannes Berg @ 2017-01-11 15:06 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: linux-wireless, David S. Miller, Networking, linux-kernel
On Wed, 2017-01-11 at 16:00 +0100, Arnd Bergmann wrote:
> On Wed, Jan 11, 2017 at 3:38 PM, Johannes Berg
> <johannes@sipsolutions.net> wrote:
> > On Wed, 2017-01-11 at 15:35 +0100, Arnd Bergmann wrote:
> > > This works fine here because iwe->u.data.length is guaranteed to
> > > be
> > > NULL, and the memcpy doesn't actually have an effect.
> >
> > I think you mean 0, not NULL, but I can fix that when I apply it.
>
> Right, thanks!
Applied. Also fixed the typo in the subject :)
johannes
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] wext: handle NULL exta data in iwe_stream_add_point better
2017-01-11 15:06 ` Johannes Berg
@ 2017-01-11 20:39 ` Arnd Bergmann
2017-01-12 8:37 ` Johannes Berg
2017-01-12 9:16 ` Johannes Berg
0 siblings, 2 replies; 9+ messages in thread
From: Arnd Bergmann @ 2017-01-11 20:39 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, David S. Miller, Networking, linux-kernel
On Wednesday, January 11, 2017 4:06:17 PM CET Johannes Berg wrote:
>
> Applied. Also fixed the typo in the subject :)
Thanks! Unfortunately I now got another warning for the same function,
and though I would have expected the patch to fix it, that did not work:
In file included from /git/arm-soc/drivers/net/wireless/intersil/prism54/islpci_dev.h:27:0,
from /git/arm-soc/drivers/net/wireless/intersil/prism54/isl_ioctl.h:24,
from /git/arm-soc/drivers/net/wireless/intersil/prism54/isl_ioctl.c:32:
/git/arm-soc/drivers/net/wireless/intersil/prism54/isl_ioctl.c: In function 'prism54_get_scan':
/git/arm-soc/include/net/iw_handler.h:560:4: error: argument 2 null where non-null expected [-Werror=nonnull]
memcpy(stream + point_len, extra, iwe->u.data.length);
The change below kills that warning too, but it gets even uglier there:
diff --git a/include/net/iw_handler.h b/include/net/iw_handler.h
index 1a41043688bc..c2aa73e5e6bb 100644
--- a/include/net/iw_handler.h
+++ b/include/net/iw_handler.h
@@ -556,7 +556,7 @@ iwe_stream_add_point(struct iw_request_info *info, char *stream, char *ends,
memcpy(stream + lcp_len,
((char *) &iwe->u) + IW_EV_POINT_OFF,
IW_EV_POINT_PK_LEN - IW_EV_LCP_PK_LEN);
- if (iwe->u.data.length)
+ if (iwe->u.data.length && extra)
memcpy(stream + point_len, extra, iwe->u.data.length);
stream += event_len;
}
Let me know if you want a proper follow-up patch, or if you can amend your
commit, or you have a better idea for resolving that warning.
Arnd
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] wext: handle NULL exta data in iwe_stream_add_point better
2017-01-11 20:39 ` Arnd Bergmann
@ 2017-01-12 8:37 ` Johannes Berg
2017-01-12 9:13 ` Johannes Berg
2017-01-12 9:16 ` Johannes Berg
1 sibling, 1 reply; 9+ messages in thread
From: Johannes Berg @ 2017-01-12 8:37 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: linux-wireless, David S. Miller, Networking, linux-kernel
On Wed, 2017-01-11 at 21:39 +0100, Arnd Bergmann wrote:
> On Wednesday, January 11, 2017 4:06:17 PM CET Johannes Berg wrote:
> >
> > Applied. Also fixed the typo in the subject :)
>
> Thanks! Unfortunately I now got another warning for the same
> function, and though I would have expected the patch to fix it, that
> did not work:
I've come to expect better of you (i.e. testing your own patches) ;-)
Come to think of it, I'm thinking I should drop this patch and the
driver should just use iwe_stream_add_event() instead? It'll be
somewhat tricky to get the length correct though.
Alternatively, perhaps we should just uninline all the crap and then
the compiler can't bother us :)
johannes
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] wext: handle NULL exta data in iwe_stream_add_point better
2017-01-12 8:37 ` Johannes Berg
@ 2017-01-12 9:13 ` Johannes Berg
0 siblings, 0 replies; 9+ messages in thread
From: Johannes Berg @ 2017-01-12 9:13 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: linux-wireless, David S. Miller, Networking, linux-kernel
> Come to think of it, I'm thinking I should drop this patch and the
> driver should just use iwe_stream_add_event() instead? It'll be
> somewhat tricky to get the length correct though.
No, turns out that's basically impossible with all the compat etc.
stuff here.
johannes
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] wext: handle NULL exta data in iwe_stream_add_point better
2017-01-11 20:39 ` Arnd Bergmann
2017-01-12 8:37 ` Johannes Berg
@ 2017-01-12 9:16 ` Johannes Berg
2017-01-12 9:44 ` Arnd Bergmann
1 sibling, 1 reply; 9+ messages in thread
From: Johannes Berg @ 2017-01-12 9:16 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: linux-wireless, David S. Miller, Networking, linux-kernel
On Wed, 2017-01-11 at 21:39 +0100, Arnd Bergmann wrote:
> On Wednesday, January 11, 2017 4:06:17 PM CET Johannes Berg wrote:
> >
> > Applied. Also fixed the typo in the subject :)
>
> Thanks! Unfortunately I now got another warning for the same
> function, and though I would have expected the patch to fix it, that
> did not work:
>
> In file included from /git/arm-
> soc/drivers/net/wireless/intersil/prism54/islpci_dev.h:27:0,
> from /git/arm-
> soc/drivers/net/wireless/intersil/prism54/isl_ioctl.h:24,
> from /git/arm-
> soc/drivers/net/wireless/intersil/prism54/isl_ioctl.c:32:
> /git/arm-soc/drivers/net/wireless/intersil/prism54/isl_ioctl.c: In
> function 'prism54_get_scan':
> /git/arm-soc/include/net/iw_handler.h:560:4: error: argument 2 null
> where non-null expected [-Werror=nonnull]
> memcpy(stream + point_len, extra, iwe->u.data.length);
And I realized only now that this was a different place ...
I've just added the check you suggested - spent way too much time
already on this old crap :)
johannes
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] wext: handle NULL exta data in iwe_stream_add_point better
2017-01-12 9:16 ` Johannes Berg
@ 2017-01-12 9:44 ` Arnd Bergmann
0 siblings, 0 replies; 9+ messages in thread
From: Arnd Bergmann @ 2017-01-12 9:44 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, David S. Miller, Networking, linux-kernel
On Thursday, January 12, 2017 10:16:00 AM CET Johannes Berg wrote:
> And I realized only now that this was a different place ...
Right, it was a few hundred randconfigs later after I had confirmed
that the first patch fixed all the configurations that were broken
at first.
> I've just added the check you suggested - spent way too much time
> already on this old crap
Ok, thanks! Let's hope it doesn't come back once more.
I'm still trying to categorize the newly added warnings in gcc-7,
there a number of very useful warnings that got added, but some of
them are rather noisy and find both a number of real bugs and
false positives. The NULL check had only a few findings that all
seemed worth fixing.
Arnd
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-01-12 9:44 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-11 14:35 [PATCH] wext: handle NULL exta data in iwe_stream_add_point better Arnd Bergmann
2017-01-11 14:38 ` Johannes Berg
2017-01-11 15:00 ` Arnd Bergmann
2017-01-11 15:06 ` Johannes Berg
2017-01-11 20:39 ` Arnd Bergmann
2017-01-12 8:37 ` Johannes Berg
2017-01-12 9:13 ` Johannes Berg
2017-01-12 9:16 ` Johannes Berg
2017-01-12 9:44 ` Arnd Bergmann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).