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 <[email protected]>
---
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(
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 <[email protected]>
> ---
> 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 [李琼斯]
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 <[email protected]>
> > ---
> > 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
Thanks for the quick response Lee and Sakari.
On Tue, May 23, 2023 at 5:13 AM Sakari Ailus
<[email protected]> 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 <[email protected]>
> > > ---
> > > 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 [email protected] if you
prefer that instead.
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 <[email protected]>
Reviewed-by: Kees Cook <[email protected]>
--
Kees Cook
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
> <[email protected]> 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 <[email protected]>
> > > > ---
> > > > 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 [email protected] if you
> prefer that instead.
Oops, my mistake then. :-) I thought I already had changed this. Oh well.
--
Regards,
Sakari Ailus