[PATCH] i2c: Replace all non-returning strlcpy with strscpy

[PATCH] i2c: Replace all non-returning strlcpy with strscpy

[Azeem Shaikh]

strlcpy() reads the entire source buffer first.
This read may exceed the destination size limit.
This is both inefficient and can lead to linear read
overflows if a source string is not NUL-terminated [1].
In an effort to remove strlcpy() completely [2], replace
strlcpy() here with strscpy().
No return values were used, so direct replacement is safe.

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

Signed-off-by: Azeem Shaikh <azeemshaikh38@gmail.com>
---
 drivers/leds/flash/leds-as3645a.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/leds/flash/leds-as3645a.c b/drivers/leds/flash/leds-as3645a.c
index bb2249771acb..7dc574b18f5f 100644
--- a/drivers/leds/flash/leds-as3645a.c
+++ b/drivers/leds/flash/leds-as3645a.c
@@ -651,8 +651,8 @@ static int as3645a_v4l2_setup(struct as3645a *flash)
 		},
 	};
 
-	strlcpy(cfg.dev_name, led->dev->kobj.name, sizeof(cfg.dev_name));
-	strlcpy(cfgind.dev_name, flash->iled_cdev.dev->kobj.name,
+	strscpy(cfg.dev_name, led->dev->kobj.name, sizeof(cfg.dev_name));
+	strscpy(cfgind.dev_name, flash->iled_cdev.dev->kobj.name,
 		sizeof(cfgind.dev_name));
 
 	flash->vf = v4l2_flash_init(

Re: [PATCH] i2c: Replace all non-returning strlcpy with strscpy

[Lee Jones]

On Tue, 23 May 2023, Azeem Shaikh wrote:

> strlcpy() reads the entire source buffer first.
> This read may exceed the destination size limit.
> This is both inefficient and can lead to linear read
> overflows if a source string is not NUL-terminated [1].
> In an effort to remove strlcpy() completely [2], replace
> strlcpy() here with strscpy().
> No return values were used, so direct replacement is safe.
> 
> [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy
> [2] https://github.com/KSPP/linux/issues/89
> 
> Signed-off-by: Azeem Shaikh <azeemshaikh38@gmail.com>
> ---
>  drivers/leds/flash/leds-as3645a.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Please resubmit, taking the time to check the subject line please.

> diff --git a/drivers/leds/flash/leds-as3645a.c b/drivers/leds/flash/leds-as3645a.c
> index bb2249771acb..7dc574b18f5f 100644
> --- a/drivers/leds/flash/leds-as3645a.c
> +++ b/drivers/leds/flash/leds-as3645a.c
> @@ -651,8 +651,8 @@ static int as3645a_v4l2_setup(struct as3645a *flash)
>  		},
>  	};
>  
> -	strlcpy(cfg.dev_name, led->dev->kobj.name, sizeof(cfg.dev_name));
> -	strlcpy(cfgind.dev_name, flash->iled_cdev.dev->kobj.name,
> +	strscpy(cfg.dev_name, led->dev->kobj.name, sizeof(cfg.dev_name));
> +	strscpy(cfgind.dev_name, flash->iled_cdev.dev->kobj.name,
>  		sizeof(cfgind.dev_name));
>  
>  	flash->vf = v4l2_flash_init(
> 

-- 
Lee Jones [李琼斯]

Re: [PATCH] i2c: Replace all non-returning strlcpy with strscpy

[Sakari Ailus]

Hi Lee, Azeem,

On Tue, May 23, 2023 at 10:05:40AM +0100, Lee Jones wrote:
> On Tue, 23 May 2023, Azeem Shaikh wrote:
> 
> > strlcpy() reads the entire source buffer first.
> > This read may exceed the destination size limit.
> > This is both inefficient and can lead to linear read
> > overflows if a source string is not NUL-terminated [1].
> > In an effort to remove strlcpy() completely [2], replace
> > strlcpy() here with strscpy().
> > No return values were used, so direct replacement is safe.
> > 
> > [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy
> > [2] https://github.com/KSPP/linux/issues/89
> > 
> > Signed-off-by: Azeem Shaikh <azeemshaikh38@gmail.com>
> > ---
> >  drivers/leds/flash/leds-as3645a.c |    4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> Please resubmit, taking the time to check the subject line please.

I'd say also shorter description will suffice. Nowadays people understand
the motivation replacing strlcpy() by strscpy() without too much
elaboration. Lines may be up to 74 characters long, too, and period isn't
automatically followed by a newline.

The patch itself seems fine.

I also prefer my @linux.intel.com address, as in MAINTAINERS for this
driver.

-- 
Kind regards,

Sakari Ailus

Re: [PATCH] i2c: Replace all non-returning strlcpy with strscpy

[Azeem Shaikh]

Thanks for the quick response Lee and Sakari.

On Tue, May 23, 2023 at 5:13 AM Sakari Ailus
<sakari.ailus@linux.intel.com> wrote:
>
> Hi Lee, Azeem,
>
> On Tue, May 23, 2023 at 10:05:40AM +0100, Lee Jones wrote:
> > On Tue, 23 May 2023, Azeem Shaikh wrote:
> >
> > > strlcpy() reads the entire source buffer first.
> > > This read may exceed the destination size limit.
> > > This is both inefficient and can lead to linear read
> > > overflows if a source string is not NUL-terminated [1].
> > > In an effort to remove strlcpy() completely [2], replace
> > > strlcpy() here with strscpy().
> > > No return values were used, so direct replacement is safe.
> > >
> > > [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy
> > > [2] https://github.com/KSPP/linux/issues/89
> > >
> > > Signed-off-by: Azeem Shaikh <azeemshaikh38@gmail.com>
> > > ---
> > >  drivers/leds/flash/leds-as3645a.c |    4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > Please resubmit, taking the time to check the subject line please.
>
> I'd say also shorter description will suffice. Nowadays people understand
> the motivation replacing strlcpy() by strscpy() without too much
> elaboration. Lines may be up to 74 characters long, too, and period isn't
> automatically followed by a newline.
>

Let me know if this commit log looks good to you both and I'll send over a v2.

Subject: [PATCH] leds: as3645a: Replace all non-returning strlcpy with strscpy

Part of a tree-wide effort to remove deprecated strlcpy()[1] and replace
it with strscpy()[2]. No return values were used, so direct replacement
is safe.

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

> I also prefer my @linux.intel.com address, as in MAINTAINERS for this
> driver.

Fyi that the email address mentioned for this driver is not the
@linux.intel.com -
https://github.com/torvalds/linux/blob/44c026a73be8038f03dbdeef028b642880cf1511/MAINTAINERS#L3070.
I'm happy to send the v2 patch to sakari.ailus@linux.intel.com if you
prefer that instead.

Re: [PATCH] i2c: Replace all non-returning strlcpy with strscpy

[Kees Cook]

On Tue, May 23, 2023 at 02:11:50AM +0000, Azeem Shaikh wrote:
> strlcpy() reads the entire source buffer first.
> This read may exceed the destination size limit.
> This is both inefficient and can lead to linear read
> overflows if a source string is not NUL-terminated [1].
> In an effort to remove strlcpy() completely [2], replace
> strlcpy() here with strscpy().
> No return values were used, so direct replacement is safe.
> 
> [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy
> [2] https://github.com/KSPP/linux/issues/89
> 
> Signed-off-by: Azeem Shaikh <azeemshaikh38@gmail.com>

Reviewed-by: Kees Cook <keescook@chromium.org>

-- 
Kees Cook

Re: [PATCH] i2c: Replace all non-returning strlcpy with strscpy

[Sakari Ailus]

Hi Azeema,

On Tue, May 23, 2023 at 10:34:34AM -0400, Azeem Shaikh wrote:
> Thanks for the quick response Lee and Sakari.
> 
> On Tue, May 23, 2023 at 5:13 AM Sakari Ailus
> <sakari.ailus@linux.intel.com> wrote:
> >
> > Hi Lee, Azeem,
> >
> > On Tue, May 23, 2023 at 10:05:40AM +0100, Lee Jones wrote:
> > > On Tue, 23 May 2023, Azeem Shaikh wrote:
> > >
> > > > strlcpy() reads the entire source buffer first.
> > > > This read may exceed the destination size limit.
> > > > This is both inefficient and can lead to linear read
> > > > overflows if a source string is not NUL-terminated [1].
> > > > In an effort to remove strlcpy() completely [2], replace
> > > > strlcpy() here with strscpy().
> > > > No return values were used, so direct replacement is safe.
> > > >
> > > > [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy
> > > > [2] https://github.com/KSPP/linux/issues/89
> > > >
> > > > Signed-off-by: Azeem Shaikh <azeemshaikh38@gmail.com>
> > > > ---
> > > >  drivers/leds/flash/leds-as3645a.c |    4 ++--
> > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > Please resubmit, taking the time to check the subject line please.
> >
> > I'd say also shorter description will suffice. Nowadays people understand
> > the motivation replacing strlcpy() by strscpy() without too much
> > elaboration. Lines may be up to 74 characters long, too, and period isn't
> > automatically followed by a newline.
> >
> 
> Let me know if this commit log looks good to you both and I'll send over a v2.
> 
> Subject: [PATCH] leds: as3645a: Replace all non-returning strlcpy with strscpy

All instances are replaced, so you can drop "all non-returning ".

> 
> Part of a tree-wide effort to remove deprecated strlcpy()[1] and replace
> it with strscpy()[2]. No return values were used, so direct replacement
> is safe.
> 
> [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy
> [2] https://github.com/KSPP/linux/issues/89

Looks good to me.

> 
> > I also prefer my @linux.intel.com address, as in MAINTAINERS for this
> > driver.
> 
> Fyi that the email address mentioned for this driver is not the
> @linux.intel.com -
> https://github.com/torvalds/linux/blob/44c026a73be8038f03dbdeef028b642880cf1511/MAINTAINERS#L3070.
> I'm happy to send the v2 patch to sakari.ailus@linux.intel.com if you
> prefer that instead.

Oops, my mistake then. :-) I thought I already had changed this. Oh well.

-- 
Regards,

Sakari Ailus