linux-kernel - [PATCH] ASoC: fsl_micfil: refactor deprecated strncpy

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 27 Jul 2023 22:26:41 +0000
From:   Justin Stitt <justinstitt@...gle.com>
To:     Shengjiu Wang <shengjiu.wang@...il.com>,
        Xiubo Li <Xiubo.Lee@...il.com>,
        Fabio Estevam <festevam@...il.com>,
        Nicolin Chen <nicoleotsuka@...il.com>,
        Liam Girdwood <lgirdwood@...il.com>,
        Mark Brown <broonie@...nel.org>,
        Jaroslav Kysela <perex@...ex.cz>, Takashi Iwai <tiwai@...e.com>
Cc:     alsa-devel@...a-project.org, linuxppc-dev@...ts.ozlabs.org,
        linux-kernel@...r.kernel.org, Kees Cook <keescook@...omium.org>,
        Justin Stitt <justinstitt@...gle.com>
Subject: [PATCH] ASoC: fsl_micfil: refactor deprecated strncpy

`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 great care taken to ensure that the
destination buffer would be NUL-terminated through the use of `len - 1`
ensuring that the previously zero-initialized buffer would not overwrite
the last NUL byte. This means that there's no bug here.

However, `strscpy` will add a mandatory NUL byte to the destination
buffer as promised by the following `strscpy` implementation [3]:
|       /* Hit buffer length without finding a NUL; force NUL-termination. */
|       if (res)
|               dest[res-1] = '\0';

This means we can lose the `- 1` which clears up whats happening here.
All the while, we get one step closer to eliminating the ambiguous
`strncpy` api in favor of its less ambiguous replacement like `strscpy`,
`strscpy_pad`, `strtomem` and `strtomem_pad` amongst others.

[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@...gle.com>
---
 sound/soc/fsl/fsl_micfil.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/fsl/fsl_micfil.c b/sound/soc/fsl/fsl_micfil.c
index 3f08082a55be..fe28b27e50d0 100644
--- a/sound/soc/fsl/fsl_micfil.c
+++ b/sound/soc/fsl/fsl_micfil.c
@@ -1044,7 +1044,7 @@ static int fsl_micfil_probe(struct platform_device *pdev)
 		return -ENOMEM;

 	micfil->pdev = pdev;
-	strncpy(micfil->name, np->name, sizeof(micfil->name) - 1);
+	strscpy(micfil->name, np->name, sizeof(micfil->name));

 	micfil->soc = of_device_get_match_data(&pdev->dev);

---
base-commit: 57012c57536f8814dec92e74197ee96c3498d24e
change-id: 20230727-sound-soc-fsl-4fc5569d771e

Best regards,
--
Justin Stitt <justinstitt@...gle.com>