* [PATCH] wireless: airo: potential buffer overflow in sprintf()
@ 2018-10-24 8:33 ` Dan Carpenter
0 siblings, 0 replies; 12+ messages in thread
From: Dan Carpenter @ 2018-10-24 8:33 UTC (permalink / raw)
To: Kalle Valo, Johannes Berg; +Cc: linux-wireless, kernel-janitors, Amir Goldstein
It looks like we wanted to print a maximum of BSSList_rid.ssidLen bytes
of the ssid, but we accidentally use "%*s" (width) instead of "%.*s"
(precision) so if the ssid doesn't have a NUL terminator this could lead
to an overflow.
Fixes: e174961ca1a0 ("net: convert print_mac to %pM")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
Static analsysis. Not tested.
drivers/net/wireless/cisco/airo.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/cisco/airo.c b/drivers/net/wireless/cisco/airo.c
index 04dd7a936593..5512c7f73fce 100644
--- a/drivers/net/wireless/cisco/airo.c
+++ b/drivers/net/wireless/cisco/airo.c
@@ -5462,7 +5462,7 @@ static int proc_BSSList_open( struct inode *inode, struct file *file ) {
we have to add a spin lock... */
rc = readBSSListRid(ai, doLoseSync, &BSSList_rid);
while(rc == 0 && BSSList_rid.index != cpu_to_le16(0xffff)) {
- ptr += sprintf(ptr, "%pM %*s rssi = %d",
+ ptr += sprintf(ptr, "%pM %.*s rssi = %d",
BSSList_rid.bssid,
(int)BSSList_rid.ssidLen,
BSSList_rid.ssid,
--
2.11.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH] wireless: airo: potential buffer overflow in sprintf()
@ 2018-10-24 8:33 ` Dan Carpenter
0 siblings, 0 replies; 12+ messages in thread
From: Dan Carpenter @ 2018-10-24 8:33 UTC (permalink / raw)
To: Kalle Valo, Johannes Berg; +Cc: linux-wireless, kernel-janitors, Amir Goldstein
It looks like we wanted to print a maximum of BSSList_rid.ssidLen bytes
of the ssid, but we accidentally use "%*s" (width) instead of "%.*s"
(precision) so if the ssid doesn't have a NUL terminator this could lead
to an overflow.
Fixes: e174961ca1a0 ("net: convert print_mac to %pM")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
Static analsysis. Not tested.
drivers/net/wireless/cisco/airo.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/cisco/airo.c b/drivers/net/wireless/cisco/airo.c
index 04dd7a936593..5512c7f73fce 100644
--- a/drivers/net/wireless/cisco/airo.c
+++ b/drivers/net/wireless/cisco/airo.c
@@ -5462,7 +5462,7 @@ static int proc_BSSList_open( struct inode *inode, struct file *file ) {
we have to add a spin lock... */
rc = readBSSListRid(ai, doLoseSync, &BSSList_rid);
while(rc = 0 && BSSList_rid.index != cpu_to_le16(0xffff)) {
- ptr += sprintf(ptr, "%pM %*s rssi = %d",
+ ptr += sprintf(ptr, "%pM %.*s rssi = %d",
BSSList_rid.bssid,
(int)BSSList_rid.ssidLen,
BSSList_rid.ssid,
--
2.11.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH] wireless: airo: potential buffer overflow in sprintf()
2018-10-24 8:33 ` Dan Carpenter
@ 2018-10-24 8:56 ` Kalle Valo
-1 siblings, 0 replies; 12+ messages in thread
From: Kalle Valo @ 2018-10-24 8:56 UTC (permalink / raw)
To: Dan Carpenter
Cc: Johannes Berg, linux-wireless, kernel-janitors, Amir Goldstein
Dan Carpenter <dan.carpenter@oracle.com> writes:
> It looks like we wanted to print a maximum of BSSList_rid.ssidLen bytes
> of the ssid, but we accidentally use "%*s" (width) instead of "%.*s"
> (precision) so if the ssid doesn't have a NUL terminator this could lead
> to an overflow.
>
> Fixes: e174961ca1a0 ("net: convert print_mac to %pM")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> Static analsysis. Not tested.
IMHO this part (after "---" line) is important information and should be
part of commit log. I can fix that.
I'll also remove "wireless:" from the title.
--
Kalle Valo
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] wireless: airo: potential buffer overflow in sprintf()
@ 2018-10-24 8:56 ` Kalle Valo
0 siblings, 0 replies; 12+ messages in thread
From: Kalle Valo @ 2018-10-24 8:56 UTC (permalink / raw)
To: Dan Carpenter
Cc: Johannes Berg, linux-wireless, kernel-janitors, Amir Goldstein
Dan Carpenter <dan.carpenter@oracle.com> writes:
> It looks like we wanted to print a maximum of BSSList_rid.ssidLen bytes
> of the ssid, but we accidentally use "%*s" (width) instead of "%.*s"
> (precision) so if the ssid doesn't have a NUL terminator this could lead
> to an overflow.
>
> Fixes: e174961ca1a0 ("net: convert print_mac to %pM")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> Static analsysis. Not tested.
IMHO this part (after "---" line) is important information and should be
part of commit log. I can fix that.
I'll also remove "wireless:" from the title.
--
Kalle Valo
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] wireless: airo: potential buffer overflow in sprintf()
2018-10-24 8:56 ` Kalle Valo
@ 2018-10-24 9:07 ` Dan Carpenter
-1 siblings, 0 replies; 12+ messages in thread
From: Dan Carpenter @ 2018-10-24 9:07 UTC (permalink / raw)
To: Kalle Valo; +Cc: Johannes Berg, linux-wireless, kernel-janitors, Amir Goldstein
On Wed, Oct 24, 2018 at 11:56:53AM +0300, Kalle Valo wrote:
> Dan Carpenter <dan.carpenter@oracle.com> writes:
>
> > It looks like we wanted to print a maximum of BSSList_rid.ssidLen bytes
> > of the ssid, but we accidentally use "%*s" (width) instead of "%.*s"
> > (precision) so if the ssid doesn't have a NUL terminator this could lead
> > to an overflow.
> >
> > Fixes: e174961ca1a0 ("net: convert print_mac to %pM")
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > ---
> > Static analsysis. Not tested.
>
> IMHO this part (after "---" line) is important information and should be
> part of commit log. I can fix that.
>
In my experience most maintainers disagree (with varying degrees of
intensity).
regards,
dan carpenter
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] wireless: airo: potential buffer overflow in sprintf()
@ 2018-10-24 9:07 ` Dan Carpenter
0 siblings, 0 replies; 12+ messages in thread
From: Dan Carpenter @ 2018-10-24 9:07 UTC (permalink / raw)
To: Kalle Valo; +Cc: Johannes Berg, linux-wireless, kernel-janitors, Amir Goldstein
On Wed, Oct 24, 2018 at 11:56:53AM +0300, Kalle Valo wrote:
> Dan Carpenter <dan.carpenter@oracle.com> writes:
>
> > It looks like we wanted to print a maximum of BSSList_rid.ssidLen bytes
> > of the ssid, but we accidentally use "%*s" (width) instead of "%.*s"
> > (precision) so if the ssid doesn't have a NUL terminator this could lead
> > to an overflow.
> >
> > Fixes: e174961ca1a0 ("net: convert print_mac to %pM")
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > ---
> > Static analsysis. Not tested.
>
> IMHO this part (after "---" line) is important information and should be
> part of commit log. I can fix that.
>
In my experience most maintainers disagree (with varying degrees of
intensity).
regards,
dan carpenter
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] wireless: airo: potential buffer overflow in sprintf()
2018-10-24 9:07 ` Dan Carpenter
@ 2018-10-24 9:23 ` Kalle Valo
-1 siblings, 0 replies; 12+ messages in thread
From: Kalle Valo @ 2018-10-24 9:23 UTC (permalink / raw)
To: Dan Carpenter
Cc: Johannes Berg, linux-wireless, kernel-janitors, Amir Goldstein
Dan Carpenter <dan.carpenter@oracle.com> writes:
> On Wed, Oct 24, 2018 at 11:56:53AM +0300, Kalle Valo wrote:
>> Dan Carpenter <dan.carpenter@oracle.com> writes:
>>
>> > It looks like we wanted to print a maximum of BSSList_rid.ssidLen bytes
>> > of the ssid, but we accidentally use "%*s" (width) instead of "%.*s"
>> > (precision) so if the ssid doesn't have a NUL terminator this could lead
>> > to an overflow.
>> >
>> > Fixes: e174961ca1a0 ("net: convert print_mac to %pM")
>> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>> > ---
>> > Static analsysis. Not tested.
>>
>> IMHO this part (after "---" line) is important information and should be
>> part of commit log. I can fix that.
>>
>
> In my experience most maintainers disagree (with varying degrees of
> intensity).
Heh, why would adding four words explaining the background of the patch
to a commit log would be a bad thing? :) Well, I guess I just view
things differently.
--
Kalle Valo
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] wireless: airo: potential buffer overflow in sprintf()
@ 2018-10-24 9:23 ` Kalle Valo
0 siblings, 0 replies; 12+ messages in thread
From: Kalle Valo @ 2018-10-24 9:23 UTC (permalink / raw)
To: Dan Carpenter
Cc: Johannes Berg, linux-wireless, kernel-janitors, Amir Goldstein
Dan Carpenter <dan.carpenter@oracle.com> writes:
> On Wed, Oct 24, 2018 at 11:56:53AM +0300, Kalle Valo wrote:
>> Dan Carpenter <dan.carpenter@oracle.com> writes:
>>
>> > It looks like we wanted to print a maximum of BSSList_rid.ssidLen bytes
>> > of the ssid, but we accidentally use "%*s" (width) instead of "%.*s"
>> > (precision) so if the ssid doesn't have a NUL terminator this could lead
>> > to an overflow.
>> >
>> > Fixes: e174961ca1a0 ("net: convert print_mac to %pM")
>> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>> > ---
>> > Static analsysis. Not tested.
>>
>> IMHO this part (after "---" line) is important information and should be
>> part of commit log. I can fix that.
>>
>
> In my experience most maintainers disagree (with varying degrees of
> intensity).
Heh, why would adding four words explaining the background of the patch
to a commit log would be a bad thing? :) Well, I guess I just view
things differently.
--
Kalle Valo
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] wireless: airo: potential buffer overflow in sprintf()
2018-10-24 9:23 ` Kalle Valo
@ 2018-10-24 9:26 ` Amir Goldstein
-1 siblings, 0 replies; 12+ messages in thread
From: Amir Goldstein @ 2018-10-24 9:26 UTC (permalink / raw)
To: kvalo; +Cc: Dan Carpenter, johannes.berg, linux-wireless, kernel-janitors
On Wed, Oct 24, 2018 at 12:23 PM Kalle Valo <kvalo@codeaurora.org> wrote:
>
> Dan Carpenter <dan.carpenter@oracle.com> writes:
>
> > On Wed, Oct 24, 2018 at 11:56:53AM +0300, Kalle Valo wrote:
> >> Dan Carpenter <dan.carpenter@oracle.com> writes:
> >>
> >> > It looks like we wanted to print a maximum of BSSList_rid.ssidLen bytes
> >> > of the ssid, but we accidentally use "%*s" (width) instead of "%.*s"
> >> > (precision) so if the ssid doesn't have a NUL terminator this could lead
> >> > to an overflow.
> >> >
> >> > Fixes: e174961ca1a0 ("net: convert print_mac to %pM")
> >> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> >> > ---
> >> > Static analsysis. Not tested.
> >>
> >> IMHO this part (after "---" line) is important information and should be
> >> part of commit log. I can fix that.
> >>
> >
> > In my experience most maintainers disagree (with varying degrees of
> > intensity).
>
> Heh, why would adding four words explaining the background of the patch
> to a commit log would be a bad thing? :) Well, I guess I just view
> things differently.
>
By the time a maintainer applies a patch and requests to merge it upstream
the patch should be tested. Right?
So how would a comment "Not tested" make any sense in an upstream
merged patch?
Thanks,
Amir.
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] wireless: airo: potential buffer overflow in sprintf()
@ 2018-10-24 9:26 ` Amir Goldstein
0 siblings, 0 replies; 12+ messages in thread
From: Amir Goldstein @ 2018-10-24 9:26 UTC (permalink / raw)
To: kvalo; +Cc: Dan Carpenter, johannes.berg, linux-wireless, kernel-janitors
On Wed, Oct 24, 2018 at 12:23 PM Kalle Valo <kvalo@codeaurora.org> wrote:
>
> Dan Carpenter <dan.carpenter@oracle.com> writes:
>
> > On Wed, Oct 24, 2018 at 11:56:53AM +0300, Kalle Valo wrote:
> >> Dan Carpenter <dan.carpenter@oracle.com> writes:
> >>
> >> > It looks like we wanted to print a maximum of BSSList_rid.ssidLen bytes
> >> > of the ssid, but we accidentally use "%*s" (width) instead of "%.*s"
> >> > (precision) so if the ssid doesn't have a NUL terminator this could lead
> >> > to an overflow.
> >> >
> >> > Fixes: e174961ca1a0 ("net: convert print_mac to %pM")
> >> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> >> > ---
> >> > Static analsysis. Not tested.
> >>
> >> IMHO this part (after "---" line) is important information and should be
> >> part of commit log. I can fix that.
> >>
> >
> > In my experience most maintainers disagree (with varying degrees of
> > intensity).
>
> Heh, why would adding four words explaining the background of the patch
> to a commit log would be a bad thing? :) Well, I guess I just view
> things differently.
>
By the time a maintainer applies a patch and requests to merge it upstream
the patch should be tested. Right?
So how would a comment "Not tested" make any sense in an upstream
merged patch?
Thanks,
Amir.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] wireless: airo: potential buffer overflow in sprintf()
2018-10-24 8:33 ` Dan Carpenter
@ 2018-11-06 17:03 ` Kalle Valo
-1 siblings, 0 replies; 12+ messages in thread
From: Kalle Valo @ 2018-11-06 17:03 UTC (permalink / raw)
To: Dan Carpenter
Cc: Johannes Berg, linux-wireless, kernel-janitors, Amir Goldstein
Dan Carpenter <dan.carpenter@oracle.com> wrote:
> It looks like we wanted to print a maximum of BSSList_rid.ssidLen bytes
> of the ssid, but we accidentally use "%*s" (width) instead of "%.*s"
> (precision) so if the ssid doesn't have a NUL terminator this could lead
> to an overflow.
>
> Static analysis. Not tested.
>
> Fixes: e174961ca1a0 ("net: convert print_mac to %pM")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Patch applied to wireless-drivers-next.git, thanks.
3d39e1bb1c88 wireless: airo: potential buffer overflow in sprintf()
--
https://patchwork.kernel.org/patch/10654389/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2018-11-06 17:03 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-24 8:33 [PATCH] wireless: airo: potential buffer overflow in sprintf() Dan Carpenter
2018-10-24 8:33 ` Dan Carpenter
2018-10-24 8:56 ` Kalle Valo
2018-10-24 8:56 ` Kalle Valo
2018-10-24 9:07 ` Dan Carpenter
2018-10-24 9:07 ` Dan Carpenter
2018-10-24 9:23 ` Kalle Valo
2018-10-24 9:23 ` Kalle Valo
2018-10-24 9:26 ` Amir Goldstein
2018-10-24 9:26 ` Amir Goldstein
2018-11-06 17:03 ` Kalle Valo
2018-11-06 17:03 ` Kalle Valo
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.