`strncpy` is deprecated for use on NUL-terminated destination strings
[1].
A suitable replacement is `strscpy` [2].
There are some hopes that someday the `strncpy` api could be ripped out
due to the vast number of suitable replacements (strscpy, strscpy_pad,
strtomem, strtomem_pad, strlcpy) [1].
[1]: http://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
---
Link: https://github.com/KSPP/linux/issues/90
Signed-off-by: Justin Stitt <[email protected]>
---
sound/soc/intel/avs/topology.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/sound/soc/intel/avs/topology.c b/sound/soc/intel/avs/topology.c
index cdb4ec500261..45d0eb2a8e71 100644
--- a/sound/soc/intel/avs/topology.c
+++ b/sound/soc/intel/avs/topology.c
@@ -1388,12 +1388,12 @@ static int avs_route_load(struct snd_soc_component *comp, int index,
port = __ffs(mach->mach_params.i2s_link_mask);
snprintf(buf, len, route->source, port);
- strncpy((char *)route->source, buf, len);
+ strscpy((char *)route->source, buf, len);
snprintf(buf, len, route->sink, port);
- strncpy((char *)route->sink, buf, len);
+ strscpy((char *)route->sink, buf, len);
if (route->control) {
snprintf(buf, len, route->control, port);
- strncpy((char *)route->control, buf, len);
+ strscpy((char *)route->control, buf, len);
}
}
---
base-commit: 0b4a9fdc9317440a71d4d4c264a5650bf4a90f3c
change-id: 20230725-sound-soc-intel-avs-remove-deprecated-strncpy-2bc41a5a5f81
Best regards,
--
Justin Stitt <[email protected]>
On Tue, Jul 25, 2023 at 10:08:38PM +0000, [email protected] wrote:
> `strncpy` is deprecated for use on NUL-terminated destination strings
> [1].
>
> A suitable replacement is `strscpy` [2].
For future patches like this, can you include the rationale for _why_
strscpy() is suitable? (i.e. how did you check that it doesn't need
zero-padding, the dest is expected to always be NUL-terminated, etc.)
I had fun looking through this code -- it's a bit of a maze! I can see
in the initializer for "route" (soc_tplg_dapm_graph_elems_load()), that
all the strings pointed at from "elem" are being checked for NUL-termination.
That this code is doing an in-place rewrite of the string is pretty
unusual (i.e. specifically casting around the "const"), but it looks
quite intentional. :)
> There are some hopes that someday the `strncpy` api could be ripped out
This can be rephrased, perhaps, as:
There is a goal to eliminate the `strncpy` API in the kernel, as its
use is too ambiguous, instead moving to the unambiguous replacements
(strscpy, strscpy_pad, strtomem, strtomem_pad, strlcpy) [1].
> due to the vast number of suitable replacements (strscpy, strscpy_pad,
> strtomem, strtomem_pad, strlcpy) [1].
>
> [1]: http://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
>
> ---
^^^^ this triple-dash is going to cause some tooling to lose your S-o-b,
as it indicates the end of the commit log.
>
>
> Link: https://github.com/KSPP/linux/issues/90
> Signed-off-by: Justin Stitt <[email protected]>
But otherwise, looks good:
Reviewed-by: Kees Cook <[email protected]>
-Kees
> ---
> sound/soc/intel/avs/topology.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/sound/soc/intel/avs/topology.c b/sound/soc/intel/avs/topology.c
> index cdb4ec500261..45d0eb2a8e71 100644
> --- a/sound/soc/intel/avs/topology.c
> +++ b/sound/soc/intel/avs/topology.c
> @@ -1388,12 +1388,12 @@ static int avs_route_load(struct snd_soc_component *comp, int index,
> port = __ffs(mach->mach_params.i2s_link_mask);
>
> snprintf(buf, len, route->source, port);
> - strncpy((char *)route->source, buf, len);
> + strscpy((char *)route->source, buf, len);
> snprintf(buf, len, route->sink, port);
> - strncpy((char *)route->sink, buf, len);
> + strscpy((char *)route->sink, buf, len);
> if (route->control) {
> snprintf(buf, len, route->control, port);
> - strncpy((char *)route->control, buf, len);
> + strscpy((char *)route->control, buf, len);
> }
> }
>
>
> ---
> base-commit: 0b4a9fdc9317440a71d4d4c264a5650bf4a90f3c
> change-id: 20230725-sound-soc-intel-avs-remove-deprecated-strncpy-2bc41a5a5f81
>
> Best regards,
> --
> Justin Stitt <[email protected]>
>
--
Kees Cook
On 7/26/2023 12:08 AM, [email protected] wrote:
> `strncpy` is deprecated for use on NUL-terminated destination strings
> [1].
>
> A suitable replacement is `strscpy` [2].
>
> There are some hopes that someday the `strncpy` api could be ripped out
> due to the vast number of suitable replacements (strscpy, strscpy_pad,
> strtomem, strtomem_pad, strlcpy) [1].
>
> [1]: http://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
>
> ---
>
>
> Link: https://github.com/KSPP/linux/issues/90
> Signed-off-by: Justin Stitt <[email protected]>
> ---
> sound/soc/intel/avs/topology.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/sound/soc/intel/avs/topology.c b/sound/soc/intel/avs/topology.c
> index cdb4ec500261..45d0eb2a8e71 100644
> --- a/sound/soc/intel/avs/topology.c
> +++ b/sound/soc/intel/avs/topology.c
> @@ -1388,12 +1388,12 @@ static int avs_route_load(struct snd_soc_component *comp, int index,
> port = __ffs(mach->mach_params.i2s_link_mask);
>
> snprintf(buf, len, route->source, port);
> - strncpy((char *)route->source, buf, len);
> + strscpy((char *)route->source, buf, len);
> snprintf(buf, len, route->sink, port);
> - strncpy((char *)route->sink, buf, len);
> + strscpy((char *)route->sink, buf, len);
> if (route->control) {
> snprintf(buf, len, route->control, port);
> - strncpy((char *)route->control, buf, len);
> + strscpy((char *)route->control, buf, len);
> }
> }
>
>
In this case snprintf adds NUL at the end and we strncpy using same
size, so there should always be NUL at the end, so replacing it with
strscpy shouldn't really change anything, so
Reviewed-by: Amadeusz Sławiński <[email protected]>
On Tue, Jul 25, 2023 at 10:08:38PM +0000, [email protected] wrote:
> `strncpy` is deprecated for use on NUL-terminated destination strings
> [1].
>
> A suitable replacement is `strscpy` [2].
>
> There are some hopes that someday the `strncpy` api could be ripped out
> due to the vast number of suitable replacements (strscpy, strscpy_pad,
> strtomem, strtomem_pad, strlcpy) [1].
>
> [1]: http://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
>
> ---
>
>
> Link: https://github.com/KSPP/linux/issues/90
> Signed-off-by: Justin Stitt <[email protected]>
> ---
You've put your signoff after a --- which means it gets deleted when
applied, don't do this. The Signoff should be start of the main
changelog.
On Tue, 25 Jul 2023 22:08:38 +0000, [email protected] wrote:
> `strncpy` is deprecated for use on NUL-terminated destination strings
> [1].
>
> A suitable replacement is `strscpy` [2].
>
> There are some hopes that someday the `strncpy` api could be ripped out
> due to the vast number of suitable replacements (strscpy, strscpy_pad,
> strtomem, strtomem_pad, strlcpy) [1].
>
> [...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/1] ASoC: intel: avs: refactor strncpy usage in topology
commit: f6500ec12c1ec745fbec20fd4734b832bbfd4aac
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