| Message ID | 20230523021150.2406032-1-azeemshaikh38@gmail.com (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | 36af333a753a22b10771af7d11a28b01183c4190 |
| Headers | show |
| Series | i2c: Replace all non-returning strlcpy with strscpy | expand |
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( >
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.
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.
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>
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.
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(
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(-)