Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1556526 > unrolled thread
| Started by | Arnd Bergmann <arnd@arndb.de> |
|---|---|
| First post | 2017-01-11 15:40 +0100 |
| Last post | 2017-01-12 10:50 +0100 |
| Articles | 9 — 2 participants |
Back to article view | Back to linux.kernel
[PATCH] wext: handle NULL exta data in iwe_stream_add_point better Arnd Bergmann <arnd@arndb.de> - 2017-01-11 15:40 +0100
Re: [PATCH] wext: handle NULL exta data in iwe_stream_add_point better Johannes Berg <johannes@sipsolutions.net> - 2017-01-11 15:40 +0100
Re: [PATCH] wext: handle NULL exta data in iwe_stream_add_point better Arnd Bergmann <arnd@arndb.de> - 2017-01-11 16:10 +0100
Re: [PATCH] wext: handle NULL exta data in iwe_stream_add_point better Johannes Berg <johannes@sipsolutions.net> - 2017-01-11 16:10 +0100
Re: [PATCH] wext: handle NULL exta data in iwe_stream_add_point better Arnd Bergmann <arnd@arndb.de> - 2017-01-11 21:50 +0100
Re: [PATCH] wext: handle NULL exta data in iwe_stream_add_point better Johannes Berg <johannes@sipsolutions.net> - 2017-01-12 09:40 +0100
Re: [PATCH] wext: handle NULL exta data in iwe_stream_add_point better Johannes Berg <johannes@sipsolutions.net> - 2017-01-12 10:20 +0100
Re: [PATCH] wext: handle NULL exta data in iwe_stream_add_point better Johannes Berg <johannes@sipsolutions.net> - 2017-01-12 10:20 +0100
Re: [PATCH] wext: handle NULL exta data in iwe_stream_add_point better Arnd Bergmann <arnd@arndb.de> - 2017-01-12 10:50 +0100
| From | Arnd Bergmann <arnd@arndb.de> |
|---|---|
| Date | 2017-01-11 15:40 +0100 |
| Subject | [PATCH] wext: handle NULL exta data in iwe_stream_add_point better |
| Message-ID | <sYsed-82P-1@gated-at.bofh.it> |
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
[toc] | [next] | [standalone]
| From | Johannes Berg <johannes@sipsolutions.net> |
|---|---|
| Date | 2017-01-11 15:40 +0100 |
| Subject | Re: [PATCH] wext: handle NULL exta data in iwe_stream_add_point better |
| Message-ID | <sYsed-82P-11@gated-at.bofh.it> |
| In reply to | #1556526 |
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
[toc] | [prev] | [next] | [standalone]
| From | Arnd Bergmann <arnd@arndb.de> |
|---|---|
| Date | 2017-01-11 16:10 +0100 |
| Message-ID | <sYsHf-8rV-23@gated-at.bofh.it> |
| In reply to | #1556528 |
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
[toc] | [prev] | [next] | [standalone]
| From | Johannes Berg <johannes@sipsolutions.net> |
|---|---|
| Date | 2017-01-11 16:10 +0100 |
| Subject | Re: [PATCH] wext: handle NULL exta data in iwe_stream_add_point better |
| Message-ID | <sYsHg-8rV-55@gated-at.bofh.it> |
| In reply to | #1556569 |
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
[toc] | [prev] | [next] | [standalone]
| From | Arnd Bergmann <arnd@arndb.de> |
|---|---|
| Date | 2017-01-11 21:50 +0100 |
| Message-ID | <sYy0h-37v-21@gated-at.bofh.it> |
| In reply to | #1556584 |
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
[toc] | [prev] | [next] | [standalone]
| From | Johannes Berg <johannes@sipsolutions.net> |
|---|---|
| Date | 2017-01-12 09:40 +0100 |
| Subject | Re: [PATCH] wext: handle NULL exta data in iwe_stream_add_point better |
| Message-ID | <sYJ5p-1Jo-45@gated-at.bofh.it> |
| In reply to | #1556914 |
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
[toc] | [prev] | [next] | [standalone]
| From | Johannes Berg <johannes@sipsolutions.net> |
|---|---|
| Date | 2017-01-12 10:20 +0100 |
| Subject | Re: [PATCH] wext: handle NULL exta data in iwe_stream_add_point better |
| Message-ID | <sYJI5-2bd-5@gated-at.bofh.it> |
| In reply to | #1557196 |
> 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
[toc] | [prev] | [next] | [standalone]
| From | Johannes Berg <johannes@sipsolutions.net> |
|---|---|
| Date | 2017-01-12 10:20 +0100 |
| Subject | Re: [PATCH] wext: handle NULL exta data in iwe_stream_add_point better |
| Message-ID | <sYJI5-2bd-21@gated-at.bofh.it> |
| In reply to | #1556914 |
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
[toc] | [prev] | [next] | [standalone]
| From | Arnd Bergmann <arnd@arndb.de> |
|---|---|
| Date | 2017-01-12 10:50 +0100 |
| Message-ID | <sYKb7-2kU-9@gated-at.bofh.it> |
| In reply to | #1557223 |
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
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web