| Message ID | 20230727-sound-soc-codecs-v1-1-562fa2836bf4@google.com |
|---|---|
| State | Accepted |
| Commit | a9a65b87a5553a4ecabad7093ef6a1088bb71b88 |
| Headers | show |
| Series | ASoC: 88pm860x: refactor deprecated strncpy | expand |
On Thu, 27 Jul 2023 22:46:13 +0000, Justin Stitt wrote: > `strncpy` is deprecated for use on NUL-terminated destination strings [1]. > > A suitable replacement is `strscpy` [2] due to the fact that it > guarantees NUL-termination on its destination buffer argument which is > _not_ always the case for `strncpy`! > > In this case, though, there was care taken to ensure that the > destination buffer would be NUL-terminated. The destination buffer is > zero-initialized and each `pm860x->name[i]` has a size of > `MAX_NAME_LENGTH + 1`. This means that there is unlikely to be a bug > here. > > [...] Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next Thanks! [1/1] ASoC: 88pm860x: refactor deprecated strncpy commit: a9a65b87a5553a4ecabad7093ef6a1088bb71b88 All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark
On Thu, Jul 27, 2023 at 10:46:13PM +0000, Justin Stitt wrote: > `strncpy` is deprecated for use on NUL-terminated destination strings [1]. > > A suitable replacement is `strscpy` [2] due to the fact that it > guarantees NUL-termination on its destination buffer argument which is > _not_ always the case for `strncpy`! > > In this case, though, there was care taken to ensure that the > destination buffer would be NUL-terminated. The destination buffer is > zero-initialized and each `pm860x->name[i]` has a size of > `MAX_NAME_LENGTH + 1`. This means that there is unlikely to be a bug > here. > > However, in an attempt to eliminate the usage of the `strncpy` API as > well as disambiguate implementations, replacements such as: `strscpy`, > `strscpy_pad`, `strtomem` and `strtomem_pad` should be preferred. > > We are able to eliminate the need for `len + 1` since `strscpy` > guarantees NUL-termination for its destination buffer as per its > implementation [3]: > > | /* Hit buffer length without finding a NUL; force NUL-termination. */ > | if (res) > | dest[res-1] = '\0'; > > [1]: www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings > [2]: manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html > [3]: https://elixir.bootlin.com/linux/v6.3/source/lib/string.c#L183 > > Link: https://github.com/KSPP/linux/issues/90 > Signed-off-by: Justin Stitt <justinstitt@google.com> > --- > sound/soc/codecs/88pm860x-codec.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/sound/soc/codecs/88pm860x-codec.c b/sound/soc/codecs/88pm860x-codec.c > index 3574c68e0dda..d99b674d574b 100644 > --- a/sound/soc/codecs/88pm860x-codec.c > +++ b/sound/soc/codecs/88pm860x-codec.c > @@ -143,7 +143,7 @@ struct pm860x_priv { > struct pm860x_det det; > > int irq[4]; > - unsigned char name[4][MAX_NAME_LEN+1]; > + unsigned char name[4][MAX_NAME_LEN]; > }; > > /* -9450dB to 0dB in 150dB steps ( mute instead of -9450dB) */ > @@ -1373,7 +1373,7 @@ static int pm860x_codec_probe(struct platform_device *pdev) > return -EINVAL; > } > pm860x->irq[i] = res->start + chip->irq_base; > - strncpy(pm860x->name[i], res->name, MAX_NAME_LEN); > + strscpy(pm860x->name[i], res->name, MAX_NAME_LEN); res->name is (perhaps) unbounded in length: struct resource { ... const char *name; ... }; So reducing struct pm860x_priv::name's size _might_ have a user-visible effect, but probably not. Reviewed-by: Kees Cook <keescook@chromium.org> -Kees
diff --git a/sound/soc/codecs/88pm860x-codec.c b/sound/soc/codecs/88pm860x-codec.c index 3574c68e0dda..d99b674d574b 100644 --- a/sound/soc/codecs/88pm860x-codec.c +++ b/sound/soc/codecs/88pm860x-codec.c @@ -143,7 +143,7 @@ struct pm860x_priv { struct pm860x_det det; int irq[4]; - unsigned char name[4][MAX_NAME_LEN+1]; + unsigned char name[4][MAX_NAME_LEN]; }; /* -9450dB to 0dB in 150dB steps ( mute instead of -9450dB) */ @@ -1373,7 +1373,7 @@ static int pm860x_codec_probe(struct platform_device *pdev) return -EINVAL; } pm860x->irq[i] = res->start + chip->irq_base; - strncpy(pm860x->name[i], res->name, MAX_NAME_LEN); + strscpy(pm860x->name[i], res->name, MAX_NAME_LEN); } ret = devm_snd_soc_register_component(&pdev->dev,
`strncpy` is deprecated for use on NUL-terminated destination strings [1]. A suitable replacement is `strscpy` [2] due to the fact that it guarantees NUL-termination on its destination buffer argument which is _not_ always the case for `strncpy`! In this case, though, there was care taken to ensure that the destination buffer would be NUL-terminated. The destination buffer is zero-initialized and each `pm860x->name[i]` has a size of `MAX_NAME_LENGTH + 1`. This means that there is unlikely to be a bug here. However, in an attempt to eliminate the usage of the `strncpy` API as well as disambiguate implementations, replacements such as: `strscpy`, `strscpy_pad`, `strtomem` and `strtomem_pad` should be preferred. We are able to eliminate the need for `len + 1` since `strscpy` guarantees NUL-termination for its destination buffer as per its implementation [3]: | /* Hit buffer length without finding a NUL; force NUL-termination. */ | if (res) | dest[res-1] = '\0'; [1]: www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [2]: manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [3]: https://elixir.bootlin.com/linux/v6.3/source/lib/string.c#L183 Link: https://github.com/KSPP/linux/issues/90 Signed-off-by: Justin Stitt <justinstitt@google.com> --- sound/soc/codecs/88pm860x-codec.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) --- base-commit: 57012c57536f8814dec92e74197ee96c3498d24e change-id: 20230727-sound-soc-codecs-947fcb9536a7 Best regards, -- Justin Stitt <justinstitt@google.com>