| Message ID | 20230727-asoc-intel-skylake-remove-deprecated-strncpy-v2-1-152830093921@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] ASoC: Intel: Skylake: replace deprecated strncpy with strscpy | expand |
On Thu, Jul 27, 2023 at 08:30:18PM +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_ the case for `strncpy`! Please don't send new patches in reply to old patches or serieses, this makes it harder for both people and tools to understand what is going on - it can bury things in mailboxes and make it difficult to keep track of what current patches are, both for the new patches and the old ones.
On Fri, Jul 28, 2023 at 12:27:24AM +0100, Mark Brown wrote: > On Thu, Jul 27, 2023 at 08:30:18PM +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_ the case for `strncpy`! > > Please don't send new patches in reply to old patches or serieses, this > makes it harder for both people and tools to understand what is going > on - it can bury things in mailboxes and make it difficult to keep track > of what current patches are, both for the new patches and the old ones. Hm, I see "X-Mailer: b4 0.12.3". Is this a default behavior of b4? (If so, that needs fixing.)
On Thu, Jul 27, 2023 at 08:30:18PM +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_ the case for `strncpy`! > > It was pretty difficult, in this case, to try and figure out whether or > not the destination buffer was zero-initialized. If it is and this > behavior is relied on then perhaps `strscpy_pad` is the preferred > option here. > > Kees was able to help me out and identify the following code snippet > which seems to show that the destination buffer is zero-initialized. > > | skl = devm_kzalloc(&pci->dev, sizeof(*skl), GFP_KERNEL); > > With this information, I opted for `strscpy` since padding is seemingly > not required. > > [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 > > Link: https://github.com/KSPP/linux/issues/90 > Suggested-by: Kees Cook <keescook@chromium.org> > Signed-off-by: Justin Stitt <justinstitt@google.com> Thanks for the updates! And based on the details from Amadeusz, it looks safe. Reviewed-by: Kees Cook <keescook@chromium.org>
On Fri, Jul 28, 2023 at 11:56:08AM -0700, Kees Cook wrote: > On Fri, Jul 28, 2023 at 12:27:24AM +0100, Mark Brown wrote: > > Please don't send new patches in reply to old patches or serieses, this > > makes it harder for both people and tools to understand what is going > > on - it can bury things in mailboxes and make it difficult to keep track > > of what current patches are, both for the new patches and the old ones. > Hm, I see "X-Mailer: b4 0.12.3". Is this a default behavior of b4? (If > so, that needs fixing.) I've not noticed it doing that for my outbound patches and can't find any option I tweaked to make it send as new threads, nor can I remember configuring anything. There is a b4.send-same-thread option since v0.13 but it's default no according to the documentation: https://b4.docs.kernel.org/en/latest/config.html#contributor-oriented-settings
On Fri, Jul 28, 2023 at 11:56:08AM -0700, Kees Cook wrote: > On Fri, Jul 28, 2023 at 12:27:24AM +0100, Mark Brown wrote: > > On Thu, Jul 27, 2023 at 08:30:18PM +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_ the case for `strncpy`! > > > > Please don't send new patches in reply to old patches or serieses, this > > makes it harder for both people and tools to understand what is going > > on - it can bury things in mailboxes and make it difficult to keep track > > of what current patches are, both for the new patches and the old ones. > > Hm, I see "X-Mailer: b4 0.12.3". Is this a default behavior of b4? (If > so, that needs fixing.) I don't believe this is the default behavior of b4 but somehow it happened. I believe I did a dry-run of this v2 before sending it and somehow it replied to the top-level thread? I have no idea but I'll double check before I attempt sending a vN next time. > > -- > Kees Cook
diff --git a/sound/soc/intel/skylake/skl-topology.c b/sound/soc/intel/skylake/skl-topology.c index 96cfebded072..0ead3ea605cd 100644 --- a/sound/soc/intel/skylake/skl-topology.c +++ b/sound/soc/intel/skylake/skl-topology.c @@ -3159,7 +3159,7 @@ static int skl_tplg_fill_str_mfest_tkn(struct device *dev, return -EINVAL; } - strncpy(skl->lib_info[ref_count].name, + strscpy(skl->lib_info[ref_count].name, str_elem->string, ARRAY_SIZE(skl->lib_info[ref_count].name)); ref_count++;
`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_ the case for `strncpy`! It was pretty difficult, in this case, to try and figure out whether or not the destination buffer was zero-initialized. If it is and this behavior is relied on then perhaps `strscpy_pad` is the preferred option here. Kees was able to help me out and identify the following code snippet which seems to show that the destination buffer is zero-initialized. | skl = devm_kzalloc(&pci->dev, sizeof(*skl), GFP_KERNEL); With this information, I opted for `strscpy` since padding is seemingly not required. [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 Link: https://github.com/KSPP/linux/issues/90 Suggested-by: Kees Cook <keescook@chromium.org> Signed-off-by: Justin Stitt <justinstitt@google.com> --- Changes in v2: - Remove extraneous logic change (thanks Kees) - Link to v1: https://lore.kernel.org/r/20230726-asoc-intel-skylake-remove-deprecated-strncpy-v1-1-020e04184c7d@google.com --- sound/soc/intel/skylake/skl-topology.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) --- base-commit: 0b4a9fdc9317440a71d4d4c264a5650bf4a90f3c change-id: 20230726-asoc-intel-skylake-remove-deprecated-strncpy-9dbcfc26040c Best regards, -- Justin Stitt <justinstitt@google.com>