Linux kernel staging patches
 help / color / Atom feed
* [PATCH] staging/rtl8192u: Remove all strcpy() uses in favor of strscpy()
@ 2021-07-18 11:32 Len Baker
  2021-07-19  8:49 ` David Laight
  0 siblings, 1 reply; 3+ messages in thread
From: Len Baker @ 2021-07-18 11:32 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Len Baker, Lee Jones, Romain Perier, Allen Pais, Dmitrii Wolf,
	Iain Craig, linux-staging, linux-kernel

strcpy() performs no bounds checking on the destination buffer. This
could result in linear overflows beyond the end of the buffer, leading
to all kinds of misbehaviors. The safe replacement is strscpy().

Signed-off-by: Len Baker <len.baker@gmx.com>
---
 drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
index ab885353f668..1a193f900779 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
@@ -2226,7 +2226,8 @@ static void ieee80211_start_ibss_wq(struct work_struct *work)
 	mutex_lock(&ieee->wx_mutex);

 	if (ieee->current_network.ssid_len == 0) {
-		strcpy(ieee->current_network.ssid, IEEE80211_DEFAULT_TX_ESSID);
+		strscpy(ieee->current_network.ssid, IEEE80211_DEFAULT_TX_ESSID,
+			sizeof(ieee->current_network.ssid));
 		ieee->current_network.ssid_len = strlen(IEEE80211_DEFAULT_TX_ESSID);
 		ieee->ssid_set = 1;
 	}
--
2.25.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* RE: [PATCH] staging/rtl8192u: Remove all strcpy() uses in favor of strscpy()
  2021-07-18 11:32 [PATCH] staging/rtl8192u: Remove all strcpy() uses in favor of strscpy() Len Baker
@ 2021-07-19  8:49 ` David Laight
  2021-07-23 18:16   ` Len Baker
  0 siblings, 1 reply; 3+ messages in thread
From: David Laight @ 2021-07-19  8:49 UTC (permalink / raw)
  To: 'Len Baker', Greg Kroah-Hartman
  Cc: Lee Jones, Romain Perier, Allen Pais, Dmitrii Wolf, Iain Craig,
	linux-staging, linux-kernel

From: Len Baker
> Sent: 18 July 2021 12:32
> 
> strcpy() performs no bounds checking on the destination buffer. This
> could result in linear overflows beyond the end of the buffer, leading
> to all kinds of misbehaviors. The safe replacement is strscpy().
> 
> Signed-off-by: Len Baker <len.baker@gmx.com>
> ---
>  drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
> b/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
> index ab885353f668..1a193f900779 100644
> --- a/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
> +++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
> @@ -2226,7 +2226,8 @@ static void ieee80211_start_ibss_wq(struct work_struct *work)
>  	mutex_lock(&ieee->wx_mutex);
> 
>  	if (ieee->current_network.ssid_len == 0) {
> -		strcpy(ieee->current_network.ssid, IEEE80211_DEFAULT_TX_ESSID);
> +		strscpy(ieee->current_network.ssid, IEEE80211_DEFAULT_TX_ESSID,
> +			sizeof(ieee->current_network.ssid));

I'm pretty sure that recent compiler releases know enough
about strcpy() to error overflows for strcpy() from quoted
strings into char[].

If these checks are enabled for kernel builds then they are
actually safer than the run-time check above
(which can be mistyped).

The compiler can (it may not) convert the strcpy() into a memcpy()
using the compile-time length of the quoted string.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] staging/rtl8192u: Remove all strcpy() uses in favor of strscpy()
  2021-07-19  8:49 ` David Laight
@ 2021-07-23 18:16   ` Len Baker
  0 siblings, 0 replies; 3+ messages in thread
From: Len Baker @ 2021-07-23 18:16 UTC (permalink / raw)
  To: David Laight, Greg Kroah-Hartman
  Cc: Len Baker, Lee Jones, Romain Perier, Allen Pais, Dmitrii Wolf,
	Iain Craig, linux-staging, linux-kernel

Hi,

On Mon, Jul 19, 2021 at 08:49:54AM +0000, David Laight wrote:
> From: Len Baker
> > Sent: 18 July 2021 12:32
> >
> > strcpy() performs no bounds checking on the destination buffer. This
> > could result in linear overflows beyond the end of the buffer, leading
> > to all kinds of misbehaviors. The safe replacement is strscpy().
> >
> > Signed-off-by: Len Baker <len.baker@gmx.com>
> > ---
> >  drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
> > b/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
> > index ab885353f668..1a193f900779 100644
> > --- a/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
> > +++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
> > @@ -2226,7 +2226,8 @@ static void ieee80211_start_ibss_wq(struct work_struct *work)
> >  	mutex_lock(&ieee->wx_mutex);
> >
> >  	if (ieee->current_network.ssid_len == 0) {
> > -		strcpy(ieee->current_network.ssid, IEEE80211_DEFAULT_TX_ESSID);
> > +		strscpy(ieee->current_network.ssid, IEEE80211_DEFAULT_TX_ESSID,
> > +			sizeof(ieee->current_network.ssid));
>
> I'm pretty sure that recent compiler releases know enough
> about strcpy() to error overflows for strcpy() from quoted
> strings into char[].
>
> If these checks are enabled for kernel builds then they are
> actually safer than the run-time check above
> (which can be mistyped).
>
> The compiler can (it may not) convert the strcpy() into a memcpy()
> using the compile-time length of the quoted string.

I agree, but if we want to remove this function entirely from the kernel [1]
we need to replace all the strcpy() uses as a previous step. And the safe
replacement is strscpy() [2].

[1] https://github.com/KSPP/linux/issues/88
[2] https://www.kernel.org/doc/html/latest/process/deprecated.html#strcpy

> 	David
>
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)
>

Thanks for the feedback,
Len

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-18 11:32 [PATCH] staging/rtl8192u: Remove all strcpy() uses in favor of strscpy() Len Baker
2021-07-19  8:49 ` David Laight
2021-07-23 18:16   ` Len Baker

Linux kernel staging patches

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-staging/0 linux-staging/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-staging linux-staging/ https://lore.kernel.org/linux-staging \
		linux-staging@lists.linux.dev
	public-inbox-index linux-staging

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/dev.linux.lists.linux-staging


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git