2021-08-26 15:49:06

by Wenlong Zhang

[permalink] [raw]
Subject: [PATCH] staging: rtl8723bs: prevent ->ssid overflow in rtw_wx_set_scan()

Commit 74b6b20df8cf ("staging: rtl8188eu: prevent ->ssid overflow in rtw_wx_set_scan()")
fixed up the staging driver rtl8188eu by adding another check
to prevent writing beyond the end of the ->ssid[] array.

Resolve this by properly fixing up the rtl8723bs driver's version of
rtw_wx_set_scan()

Reported-by: Wenlong Zhang(iLifetruth) <[email protected]>
Fixes: 74b6b20df8cf ("staging: rtl8188eu: prevent ->ssid overflow in rtw_wx_set_scan()")

Signed-off-by: Wenlong Zhang <[email protected]>

---
drivers/staging/rtl8723bs/os_dep/ioctl_linux.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c b/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c
index 902ac8169948..6fc1020cea11 100644
--- a/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c
+++ b/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c
@@ -1351,9 +1351,9 @@ static int rtw_wx_set_scan(struct net_device *dev, struct iw_request_info *a,

sec_len = *(pos++); len -= 1;

- if (sec_len > 0 && sec_len <= len) {
+ if (sec_len > 0 && sec_len <= len && sec_len <= 32) {
ssid[ssid_index].SsidLength = sec_len;
- memcpy(ssid[ssid_index].Ssid, pos, ssid[ssid_index].SsidLength);
+ memcpy(ssid[ssid_index].Ssid, pos, sec_len);
/* DBG_871X("%s COMBO_SCAN with specific ssid:%s, %d\n", __func__ */
/* , ssid[ssid_index].Ssid, ssid[ssid_index].SsidLength); */
ssid_index++;
--
2.15.0


2021-08-26 17:02:42

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH] staging: rtl8723bs: prevent ->ssid overflow in rtw_wx_set_scan()

On Thu, Aug 26, 2021 at 11:46:22PM +0800, Wenlong Zhang wrote:
> Commit 74b6b20df8cf ("staging: rtl8188eu: prevent ->ssid overflow in rtw_wx_set_scan()")
> fixed up the staging driver rtl8188eu by adding another check
> to prevent writing beyond the end of the ->ssid[] array.
>
> Resolve this by properly fixing up the rtl8723bs driver's version of
> rtw_wx_set_scan()
>
> Reported-by: Wenlong Zhang(iLifetruth) <[email protected]>
> Fixes: 74b6b20df8cf ("staging: rtl8188eu: prevent ->ssid overflow in rtw_wx_set_scan()")
>
> Signed-off-by: Wenlong Zhang <[email protected]>
>

The patch looks good but somehow it doesn't apply to today's linux-next.

regards,
dan carpenter

2021-08-26 17:21:34

by Fabio Aiuto

[permalink] [raw]
Subject: Re: [PATCH] staging: rtl8723bs: prevent ->ssid overflow in rtw_wx_set_scan()

Hello Welong,

On Thu, Aug 26, 2021 at 11:46:22PM +0800, Wenlong Zhang wrote:
> Commit 74b6b20df8cf ("staging: rtl8188eu: prevent ->ssid overflow in rtw_wx_set_scan()")
> fixed up the staging driver rtl8188eu by adding another check
> to prevent writing beyond the end of the ->ssid[] array.
>
> Resolve this by properly fixing up the rtl8723bs driver's version of
> rtw_wx_set_scan()
>
> Reported-by: Wenlong Zhang(iLifetruth) <[email protected]>
> Fixes: 74b6b20df8cf ("staging: rtl8188eu: prevent ->ssid overflow in rtw_wx_set_scan()")
>
> Signed-off-by: Wenlong Zhang <[email protected]>
>
> ---
> drivers/staging/rtl8723bs/os_dep/ioctl_linux.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c b/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c
> index 902ac8169948..6fc1020cea11 100644
> --- a/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c
> +++ b/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c
> @@ -1351,9 +1351,9 @@ static int rtw_wx_set_scan(struct net_device *dev, struct iw_request_info *a,
>
> sec_len = *(pos++); len -= 1;
>
> - if (sec_len > 0 && sec_len <= len) {
> + if (sec_len > 0 && sec_len <= len && sec_len <= 32) {
> ssid[ssid_index].SsidLength = sec_len;
> - memcpy(ssid[ssid_index].Ssid, pos, ssid[ssid_index].SsidLength);
> + memcpy(ssid[ssid_index].Ssid, pos, sec_len);
> /* DBG_871X("%s COMBO_SCAN with specific ssid:%s, %d\n", __func__ */
> /* , ssid[ssid_index].Ssid, ssid[ssid_index].SsidLength); */
> ssid_index++;
> --
> 2.15.0
>

today the patch which removes wext handlers has been accepted
in staging-testing so maybe rtw_wx_set_scan is going to disappear.

thank you,

fabio

2021-08-26 17:28:40

by Fabio Aiuto

[permalink] [raw]
Subject: Re: [PATCH] staging: rtl8723bs: prevent ->ssid overflow in rtw_wx_set_scan()

Hello Welong,

(Added commit reference)

On Thu, Aug 26, 2021 at 11:46:22PM +0800, Wenlong Zhang wrote:
> Commit 74b6b20df8cf ("staging: rtl8188eu: prevent ->ssid overflow in rtw_wx_set_scan()")
> fixed up the staging driver rtl8188eu by adding another check
> to prevent writing beyond the end of the ->ssid[] array.
>
> Resolve this by properly fixing up the rtl8723bs driver's version of
> rtw_wx_set_scan()
>
> Reported-by: Wenlong Zhang(iLifetruth) <[email protected]>
> Fixes: 74b6b20df8cf ("staging: rtl8188eu: prevent ->ssid overflow in rtw_wx_set_scan()")
>
> Signed-off-by: Wenlong Zhang <[email protected]>
>
> ---
> drivers/staging/rtl8723bs/os_dep/ioctl_linux.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c b/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c
> index 902ac8169948..6fc1020cea11 100644
> --- a/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c
> +++ b/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c
> @@ -1351,9 +1351,9 @@ static int rtw_wx_set_scan(struct net_device *dev, struct iw_request_info *a,
>
> sec_len = *(pos++); len -= 1;
>
> - if (sec_len > 0 && sec_len <= len) {
> + if (sec_len > 0 && sec_len <= len && sec_len <= 32) {
> ssid[ssid_index].SsidLength = sec_len;
> - memcpy(ssid[ssid_index].Ssid, pos, ssid[ssid_index].SsidLength);
> + memcpy(ssid[ssid_index].Ssid, pos, sec_len);
> /* DBG_871X("%s COMBO_SCAN with specific ssid:%s, %d\n", __func__ */
> /* , ssid[ssid_index].Ssid, ssid[ssid_index].SsidLength); */
> ssid_index++;
> --
> 2.15.0
>

today the patch which removes wext handlers has been accepted
(commit 174ac41a7aafb31041cba3fe54ccd89b9daeef5d)
in staging-testing so maybe rtw_wx_set_scan is going to disappear.

thank you,

fabio

2021-08-26 18:19:37

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH] staging: rtl8723bs: prevent ->ssid overflow in rtw_wx_set_scan()

On Thu, Aug 26, 2021 at 07:26:19PM +0200, Fabio Aiuto wrote:
> today the patch which removes wext handlers has been accepted
> (commit 174ac41a7aafb31041cba3fe54ccd89b9daeef5d)
> in staging-testing so maybe rtw_wx_set_scan is going to disappear.
>

From a process perspective, in staging we don't track things that might
happen in the future. We look at each patch in the order that they
arrive and either apply them or reject them.

And from a practical perspective this patch might be something that
people want to backport so it would be nice to apply it.

regards,
dan carpenter

2021-08-27 06:47:41

by Fabio Aiuto

[permalink] [raw]
Subject: Re: [PATCH] staging: rtl8723bs: prevent ->ssid overflow in rtw_wx_set_scan()

On Thu, Aug 26, 2021 at 09:16:48PM +0300, Dan Carpenter wrote:
> On Thu, Aug 26, 2021 at 07:26:19PM +0200, Fabio Aiuto wrote:
> > today the patch which removes wext handlers has been accepted
> > (commit 174ac41a7aafb31041cba3fe54ccd89b9daeef5d)
> > in staging-testing so maybe rtw_wx_set_scan is going to disappear.
> >
>
> From a process perspective, in staging we don't track things that might
> happen in the future. We look at each patch in the order that they
> arrive and either apply them or reject them.

Wenlong's patch will be applied on top of staging-testing first,
and it won't apply due to wext-removal patch.

>
> And from a practical perspective this patch might be something that
> people want to backport so it would be nice to apply it.
>
> regards,
> dan carpenter

thank you,

fabio

2021-08-27 06:53:35

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] staging: rtl8723bs: prevent ->ssid overflow in rtw_wx_set_scan()

On Thu, Aug 26, 2021 at 11:46:22PM +0800, Wenlong Zhang wrote:
> Commit 74b6b20df8cf ("staging: rtl8188eu: prevent ->ssid overflow in rtw_wx_set_scan()")
> fixed up the staging driver rtl8188eu by adding another check
> to prevent writing beyond the end of the ->ssid[] array.
>
> Resolve this by properly fixing up the rtl8723bs driver's version of
> rtw_wx_set_scan()

Odd trailing whitespace :(

>
> Reported-by: Wenlong Zhang(iLifetruth) <[email protected]>
> Fixes: 74b6b20df8cf ("staging: rtl8188eu: prevent ->ssid overflow in rtw_wx_set_scan()")
>
> Signed-off-by: Wenlong Zhang <[email protected]>

No need for a blank line here.

Also, you don't need a reported-by: if you are the author and sign off
on the patch, that would just be implied :)

Can you fix this up and resend a v2?

thanks,

greg k-h

2021-08-27 09:03:16

by Wenlong Zhang

[permalink] [raw]
Subject: [PATCH v2] staging: rtl8723bs: prevent ->ssid overflow in rtw_wx_set_scan()

Commit 74b6b20df8cf ("staging: rtl8188eu: prevent ->ssid overflow in rtw_wx_set_scan()")
fixed up the staging driver rtl8188eu by adding another check
to prevent writing beyond the end of the ->ssid[] array.

Resolve this by properly fixing up the rtl8723bs driver's version of
rtw_wx_set_scan()

Fixes: 74b6b20df8cf ("staging: rtl8188eu: prevent ->ssid overflow in rtw_wx_set_scan()")
Signed-off-by: Wenlong Zhang <[email protected]>
---
drivers/staging/rtl8723bs/os_dep/ioctl_linux.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c b/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c
index 902ac8169948..6fc1020cea11 100644
--- a/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c
+++ b/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c
@@ -1351,9 +1351,9 @@ static int rtw_wx_set_scan(struct net_device *dev, struct iw_request_info *a,

sec_len = *(pos++); len -= 1;

- if (sec_len > 0 && sec_len <= len) {
+ if (sec_len > 0 && sec_len <= len && sec_len <= 32) {
ssid[ssid_index].SsidLength = sec_len;
- memcpy(ssid[ssid_index].Ssid, pos, ssid[ssid_index].SsidLength);
+ memcpy(ssid[ssid_index].Ssid, pos, sec_len);
/* DBG_871X("%s COMBO_SCAN with specific ssid:%s, %d\n", __func__ */
/* , ssid[ssid_index].Ssid, ssid[ssid_index].SsidLength); */
ssid_index++;
--
2.15.0

2021-08-27 09:49:26

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v2] staging: rtl8723bs: prevent ->ssid overflow in rtw_wx_set_scan()

On Fri, Aug 27, 2021 at 05:00:27PM +0800, Wenlong Zhang wrote:
> Commit 74b6b20df8cf ("staging: rtl8188eu: prevent ->ssid overflow in rtw_wx_set_scan()")
> fixed up the staging driver rtl8188eu by adding another check
> to prevent writing beyond the end of the ->ssid[] array.
>
> Resolve this by properly fixing up the rtl8723bs driver's version of
> rtw_wx_set_scan()
>
> Fixes: 74b6b20df8cf ("staging: rtl8188eu: prevent ->ssid overflow in rtw_wx_set_scan()")
> Signed-off-by: Wenlong Zhang <[email protected]>
> ---
> drivers/staging/rtl8723bs/os_dep/ioctl_linux.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c b/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c
> index 902ac8169948..6fc1020cea11 100644
> --- a/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c
> +++ b/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c
> @@ -1351,9 +1351,9 @@ static int rtw_wx_set_scan(struct net_device *dev, struct iw_request_info *a,
>
> sec_len = *(pos++); len -= 1;
>
> - if (sec_len > 0 && sec_len <= len) {
> + if (sec_len > 0 && sec_len <= len && sec_len <= 32) {
> ssid[ssid_index].SsidLength = sec_len;
> - memcpy(ssid[ssid_index].Ssid, pos, ssid[ssid_index].SsidLength);
> + memcpy(ssid[ssid_index].Ssid, pos, sec_len);
> /* DBG_871X("%s COMBO_SCAN with specific ssid:%s, %d\n", __func__ */
> /* , ssid[ssid_index].Ssid, ssid[ssid_index].SsidLength); */
> ssid_index++;
> --
> 2.15.0
>
>

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him
a patch that has triggered this response. He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created. Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- This looks like a new version of a previously submitted patch, but you
did not list below the --- line any changes from the previous version.
Please read the section entitled "The canonical patch format" in the
kernel file, Documentation/SubmittingPatches for what needs to be done
here to properly describe this.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot

2021-08-27 12:53:39

by Wenlong Zhang

[permalink] [raw]
Subject: [PATCH v3] staging: rtl8723bs: prevent ->ssid overflow in rtw_wx_set_scan()

Commit 74b6b20df8cf ("staging: rtl8188eu: prevent ->ssid overflow in rtw_wx_set_scan()")
fixed up the staging driver rtl8188eu by adding another check
to prevent writing beyond the end of the ->ssid[] array.

Resolve this by properly fixing up the rtl8723bs driver's version of
rtw_wx_set_scan()

Fixes: 74b6b20df8cf ("staging: rtl8188eu: prevent ->ssid overflow in rtw_wx_set_scan()")
Signed-off-by: Wenlong Zhang <[email protected]>
---
v3:
- Added the changelogs for this patch.
v2:
- Fixed the description of this patch.
Thanks Greg KH for the review and guidance
---
drivers/staging/rtl8723bs/os_dep/ioctl_linux.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c b/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c
index 902ac8169948..6fc1020cea11 100644
--- a/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c
+++ b/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c
@@ -1351,9 +1351,9 @@ static int rtw_wx_set_scan(struct net_device *dev, struct iw_request_info *a,

sec_len = *(pos++); len -= 1;

- if (sec_len > 0 && sec_len <= len) {
+ if (sec_len > 0 && sec_len <= len && sec_len <= 32) {
ssid[ssid_index].SsidLength = sec_len;
- memcpy(ssid[ssid_index].Ssid, pos, ssid[ssid_index].SsidLength);
+ memcpy(ssid[ssid_index].Ssid, pos, sec_len);
/* DBG_871X("%s COMBO_SCAN with specific ssid:%s, %d\n", __func__ */
/* , ssid[ssid_index].Ssid, ssid[ssid_index].SsidLength); */
ssid_index++;
--
2.15.0

2021-08-27 13:10:21

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH v3] staging: rtl8723bs: prevent ->ssid overflow in rtw_wx_set_scan()

On Fri, Aug 27, 2021 at 08:50:59PM +0800, Wenlong Zhang wrote:
> Commit 74b6b20df8cf ("staging: rtl8188eu: prevent ->ssid overflow in rtw_wx_set_scan()")
> fixed up the staging driver rtl8188eu by adding another check
> to prevent writing beyond the end of the ->ssid[] array.
>
> Resolve this by properly fixing up the rtl8723bs driver's version of
> rtw_wx_set_scan()
>
> Fixes: 74b6b20df8cf ("staging: rtl8188eu: prevent ->ssid overflow in rtw_wx_set_scan()")
> Signed-off-by: Wenlong Zhang <[email protected]>
> ---

This still doesn't apply at all. I do not think you are writing the
patch against linux-next.

regards,
dan carpenter

2021-08-27 13:15:32

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v3] staging: rtl8723bs: prevent ->ssid overflow in rtw_wx_set_scan()

On Fri, Aug 27, 2021 at 08:50:59PM +0800, Wenlong Zhang wrote:
> Commit 74b6b20df8cf ("staging: rtl8188eu: prevent ->ssid overflow in rtw_wx_set_scan()")
> fixed up the staging driver rtl8188eu by adding another check
> to prevent writing beyond the end of the ->ssid[] array.
>
> Resolve this by properly fixing up the rtl8723bs driver's version of
> rtw_wx_set_scan()
>
> Fixes: 74b6b20df8cf ("staging: rtl8188eu: prevent ->ssid overflow in rtw_wx_set_scan()")
> Signed-off-by: Wenlong Zhang <[email protected]>
> ---
> v3:
> - Added the changelogs for this patch.
> v2:
> - Fixed the description of this patch.
> Thanks Greg KH for the review and guidance

This doesn't apply against any tree that I can see. Neither my
staging-next or the 5.14-rc6 kernel release.

What did you make it against? Please rebase and resend it against at
the very least, 5.14-rc6.

thanks,

greg k-h