-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove all strncpy() uses #90
Comments
Simplify the code by using kstrndup instead of kzalloc and strncpy in smk_parse_smack(), which meanwhile remove strncpy as [1] suggests. [1]: KSPP#90 Signed-off-by: GONG, Ruiqi <gongruiqi1@huawei.com>
Simplify the code by using kstrndup instead of kzalloc and strncpy in smk_parse_smack(), which meanwhile remove strncpy as [1] suggests. [1]: KSPP/linux#90 Signed-off-by: GONG, Ruiqi <gongruiqi1@huawei.com> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
Simplify the code by using kstrndup instead of kzalloc and strncpy in smk_parse_smack(), which meanwhile remove strncpy as [1] suggests. [1]: KSPP/linux#90 Signed-off-by: GONG, Ruiqi <gongruiqi1@huawei.com> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
One of the "legitimate" uses of strncpy() is copying a NUL-terminated string into a fixed-size non-NUL-terminated character array. To avoid the weaknesses and ambiguity of intent when using strncpy(), provide replacement functions that explicitly distinguish between trailing padding and not, and require the destination buffer size be discoverable by the compiler. For example: struct obj { int foo; char small[4] __nonstring; char big[8] __nonstring; int bar; }; struct obj p; /* This will truncate to 4 chars with no trailing NUL */ strncpy(p.small, "hello", sizeof(p.small)); /* p.small contains 'h', 'e', 'l', 'l' */ /* This will NUL pad to 8 chars. */ strncpy(p.big, "hello", sizeof(p.big)); /* p.big contains 'h', 'e', 'l', 'l', 'o', '\0', '\0', '\0' */ When the "__nonstring" attributes are missing, the intent of the programmer becomes ambiguous for whether the lack of a trailing NUL in the p.small copy is a bug. Additionally, it's not clear whether the trailing padding in the p.big copy is _needed_. Both cases become unambiguous with: strtomem(p.small, "hello"); strtomem_pad(p.big, "hello"); See also KSPP#90 Expand the memcpy KUnit tests to include these functions. Cc: Wolfram Sang <wsa+renesas@sang-engineering.com> Cc: Nick Desaulniers <ndesaulniers@google.com> Cc: Geert Uytterhoeven <geert@linux-m68k.org> Cc: Guenter Roeck <linux@roeck-us.net> Signed-off-by: Kees Cook <keescook@chromium.org>
One of the "legitimate" uses of strncpy() is copying a NUL-terminated string into a fixed-size non-NUL-terminated character array. To avoid the weaknesses and ambiguity of intent when using strncpy(), provide replacement functions that explicitly distinguish between trailing padding and not, and require the destination buffer size be discoverable by the compiler. For example: struct obj { int foo; char small[4] __nonstring; char big[8] __nonstring; int bar; }; struct obj p; /* This will truncate to 4 chars with no trailing NUL */ strncpy(p.small, "hello", sizeof(p.small)); /* p.small contains 'h', 'e', 'l', 'l' */ /* This will NUL pad to 8 chars. */ strncpy(p.big, "hello", sizeof(p.big)); /* p.big contains 'h', 'e', 'l', 'l', 'o', '\0', '\0', '\0' */ When the "__nonstring" attributes are missing, the intent of the programmer becomes ambiguous for whether the lack of a trailing NUL in the p.small copy is a bug. Additionally, it's not clear whether the trailing padding in the p.big copy is _needed_. Both cases become unambiguous with: strtomem(p.small, "hello"); strtomem_pad(p.big, "hello", 0); See also KSPP#90 Expand the memcpy KUnit tests to include these functions. Cc: Wolfram Sang <wsa+renesas@sang-engineering.com> Cc: Nick Desaulniers <ndesaulniers@google.com> Cc: Geert Uytterhoeven <geert@linux-m68k.org> Cc: Guenter Roeck <linux@roeck-us.net> Signed-off-by: Kees Cook <keescook@chromium.org>
One of the "legitimate" uses of strncpy() is copying a NUL-terminated string into a fixed-size non-NUL-terminated character array. To avoid the weaknesses and ambiguity of intent when using strncpy(), provide replacement functions that explicitly distinguish between trailing padding and not, and require the destination buffer size be discoverable by the compiler. For example: struct obj { int foo; char small[4] __nonstring; char big[8] __nonstring; int bar; }; struct obj p; /* This will truncate to 4 chars with no trailing NUL */ strncpy(p.small, "hello", sizeof(p.small)); /* p.small contains 'h', 'e', 'l', 'l' */ /* This will NUL pad to 8 chars. */ strncpy(p.big, "hello", sizeof(p.big)); /* p.big contains 'h', 'e', 'l', 'l', 'o', '\0', '\0', '\0' */ When the "__nonstring" attributes are missing, the intent of the programmer becomes ambiguous for whether the lack of a trailing NUL in the p.small copy is a bug. Additionally, it's not clear whether the trailing padding in the p.big copy is _needed_. Both cases become unambiguous with: strtomem(p.small, "hello"); strtomem_pad(p.big, "hello", 0); See also KSPP#90 Expand the memcpy KUnit tests to include these functions. Cc: Wolfram Sang <wsa+renesas@sang-engineering.com> Cc: Nick Desaulniers <ndesaulniers@google.com> Cc: Geert Uytterhoeven <geert@linux-m68k.org> Cc: Guenter Roeck <linux@roeck-us.net> Cc: Bagas Sanjaya <bagasdotme@gmail.com> Signed-off-by: Kees Cook <keescook@chromium.org>
One of the "legitimate" uses of strncpy() is copying a NUL-terminated string into a fixed-size non-NUL-terminated character array. To avoid the weaknesses and ambiguity of intent when using strncpy(), provide replacement functions that explicitly distinguish between trailing padding and not, and require the destination buffer size be discoverable by the compiler. For example: struct obj { int foo; char small[4] __nonstring; char big[8] __nonstring; int bar; }; struct obj p; /* This will truncate to 4 chars with no trailing NUL */ strncpy(p.small, "hello", sizeof(p.small)); /* p.small contains 'h', 'e', 'l', 'l' */ /* This will NUL pad to 8 chars. */ strncpy(p.big, "hello", sizeof(p.big)); /* p.big contains 'h', 'e', 'l', 'l', 'o', '\0', '\0', '\0' */ When the "__nonstring" attributes are missing, the intent of the programmer becomes ambiguous for whether the lack of a trailing NUL in the p.small copy is a bug. Additionally, it's not clear whether the trailing padding in the p.big copy is _needed_. Both cases become unambiguous with: strtomem(p.small, "hello"); strtomem_pad(p.big, "hello", 0); See also KSPP#90 Expand the memcpy KUnit tests to include these functions. Cc: Wolfram Sang <wsa+renesas@sang-engineering.com> Cc: Nick Desaulniers <ndesaulniers@google.com> Cc: Geert Uytterhoeven <geert@linux-m68k.org> Cc: Guenter Roeck <linux@roeck-us.net> Signed-off-by: Kees Cook <keescook@chromium.org>
One of the "legitimate" uses of strncpy() is copying a NUL-terminated string into a fixed-size non-NUL-terminated character array. To avoid the weaknesses and ambiguity of intent when using strncpy(), provide replacement functions that explicitly distinguish between trailing padding and not, and require the destination buffer size be discoverable by the compiler. For example: struct obj { int foo; char small[4] __nonstring; char big[8] __nonstring; int bar; }; struct obj p; /* This will truncate to 4 chars with no trailing NUL */ strncpy(p.small, "hello", sizeof(p.small)); /* p.small contains 'h', 'e', 'l', 'l' */ /* This will NUL pad to 8 chars. */ strncpy(p.big, "hello", sizeof(p.big)); /* p.big contains 'h', 'e', 'l', 'l', 'o', '\0', '\0', '\0' */ When the "__nonstring" attributes are missing, the intent of the programmer becomes ambiguous for whether the lack of a trailing NUL in the p.small copy is a bug. Additionally, it's not clear whether the trailing padding in the p.big copy is _needed_. Both cases become unambiguous with: strtomem(p.small, "hello"); strtomem_pad(p.big, "hello", 0); See also KSPP#90 Expand the memcpy KUnit tests to include these functions. Cc: Wolfram Sang <wsa+renesas@sang-engineering.com> Cc: Nick Desaulniers <ndesaulniers@google.com> Cc: Geert Uytterhoeven <geert@linux-m68k.org> Cc: Guenter Roeck <linux@roeck-us.net> Signed-off-by: Kees Cook <keescook@chromium.org>
What is to be done about cases where the destination buffer is length-bounded (not NUL-terminated) and its size is not known by the compiler (i.e: the macros There are some pathological cases in If Take this code, for example, from the case mentioned above: static void dsa_slave_get_strings(struct net_device *dev,
uint32_t stringset, uint8_t *data)
...
int len = 32;
strncpy(data, "tx_packets", len);
strncpy(data + len, "tx_bytes", len);
strncpy(data + 2 * len, "rx_packets", len);
strncpy(data + 3 * len, "rx_bytes", len);
... Simply swapping these calls to the appropriate tl;dr: based on the decision tree provided by OP there exists some cases (mentioned above) where the destination has both qualities 1) length-bounded and 2) impossible for the compiler to determine the size of
Somewhat Reviewed by: @nickdesaulniers |
After some tinkering, I think I've found a viable solution. If there existed some #define strntomem_pad(dest, dest_len, src, pad) \
memcpy_and_pad(dest, dest_len, src, strnlen(src, dest_len), pad); Here's how it would look in action tackling static void dsa_slave_get_strings(struct net_device *dev,
uint32_t stringset, uint8_t *data)
...
int len = 32;
strncpy(data, "tx_packets", len);
strncpy(data + len, "tx_bytes", len);
strncpy(data + 2 * len, "rx_packets", len);
strncpy(data + 3 * len, "rx_bytes", len);
... turns into static void dsa_slave_get_strings(struct net_device *dev,
uint32_t stringset, uint8_t *data)
...
int len = 32;
strntomem_pad(data, len, "tx_packets", 0);
strntomem_pad(data + len, len, "tx_bytes", 0);
strntomem_pad(data + 2 * len, len, "rx_packets", 0);
strntomem_pad(data + 3 * len, len, "rx_bytes", 0);
... Testing reveals these are exactly equivalent (in my isolated godbolt testing) and thus the new "Remove all strncpy() uses" decision tree looks as follows:
Is this a viable solution? |
Depending on the human to get the length argument correct is a new foot-gun that we should work very hard to avoid. Yeah, this is a new ugly case you've found (unknown length and not %NUL terminated). A few thoughts about the specific example:
This internal API looks very fragile. The Anyway, I suspect the best fix here is to teach the handlers the size of the buffers in some way. |
Given that the strings in question will not be truncated and other |
@kees Thoughts on this article: tl;dr: Linus doesn't want large swaths of
|
https://lore.kernel.org/lkml/202307121703.D2BE6DFEE@keescook/ tl;dr: we won't do treewide changes (instead we do it individually ) and we need to do conversions with a very regular process for performing and validating the transformations. (This is how we've approached the flexible array transformations as well.) |
`strncpy` is deprecated for use on NUL-terminated destination strings [1]. Even call sites utilizing length-bounded destination buffers should switch over to using `strtomem` or `strtomem_pad`. In this case, however, the compiler is unable to determine the size of the `data` buffer which renders `strtomem` unusable. Due to this, `strscpy` should be used. It should be noted that most call sites already zero-initialize the destination buffer. However, I've opted to use `strscpy_pad` to maintain the same exact behavior that `strncpy` produced (zero-padded tail up to `len`). Also see [3]. [1]: www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [2]: elixir.bootlin.com/linux/v6.3/source/net/ethtool/ioctl.c#L1944 [3]: manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html Link: KSPP#90 Signed-off-by: Justin Stitt <justinstitt@google.com>
`strncpy` is deprecated for use on NUL-terminated destination strings [1]. Even call sites utilizing length-bounded destination buffers should switch over to using `strtomem` or `strtomem_pad`. In this case, however, the compiler is unable to determine the size of the `data` buffer which renders `strtomem` unusable. Due to this, `strscpy` should be used. It should be noted that most call sites already zero-initialize the destination buffer. However, I've opted to use `strscpy_pad` to maintain the same exact behavior that `strncpy` produced (zero-padded tail up to `len`). Also see [3]. [1]: www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [2]: elixir.bootlin.com/linux/v6.3/source/net/ethtool/ioctl.c#L1944 [3]: manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html Link: KSPP#90 Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> Reviewed-by: Kees Cook <keescook@chromium.org> Signed-off-by: Justin Stitt <justinstitt@google.com>
`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. Also within this patch is a change to an instance of `x > y - 1` to `x >= y` which tends to be more robust and readable. Consider, for instance, if `y` was somehow `INT_MIN`. [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: KSPP#90 Suggested-by: Kees Cook <keescook@chromium.org> Signed-off-by: Justin Stitt <justinstitt@google.com>
`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]: 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: KSPP#90 Signed-off-by: Justin Stitt <justinstitt@google.com> Link: https://lore.kernel.org/r/20230725-sound-soc-intel-avs-remove-deprecated-strncpy-v1-1-6357a1f8e9cf@google.com Signed-off-by: Mark Brown <broonie@kernel.org>
`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: KSPP#90 Suggested-by: Kees Cook <keescook@chromium.org> Signed-off-by: Justin Stitt <justinstitt@google.com>
`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`! It should be noted that, in this case, the destination buffer has a length strictly greater than the source string. Moreover, the source string is NUL-terminated (and so is the destination) which means there was no real bug happening here. Nonetheless, this patch would get us one step closer to eliminating the `strncpy` API in the kernel, as its use is too ambiguous. We need to favor less ambiguous replacements such as: strscpy, strscpy_pad, strtomem and strtomem_pad (amongst others). Technically, my patch yields subtly different behavior. The original implementation with `strncpy` would fill the entire destination buffer with null bytes [3] while `strscpy` will leave the junk, uninitialized bytes trailing after the _mandatory_ NUL-termination. So, if somehow `pcm->name` or `card->driver/shortname/longname` require this NUL-padding behavior then `strscpy_pad` should be used. My interpretation, though, is that the aforementioned fields are just fine as NUL-terminated strings. Please correct my assumptions if needed and I'll send in a v2. [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://linux.die.net/man/3/strncpy Link: KSPP#90 Signed-off-by: Justin Stitt <justinstitt@google.com>
`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`! It should be noted that, in this case, the destination buffer has a length strictly greater than the source string. Moreover, the source string is NUL-terminated (and so is the destination) which means there was no real bug happening here. Nonetheless, this patch would get us one step closer to eliminating the `strncpy` API in the kernel, as its use is too ambiguous. We need to favor less ambiguous replacements such as: strscpy, strscpy_pad, strtomem and strtomem_pad (amongst others). Technically, my patch yields subtly different behavior. The original implementation with `strncpy` would fill the entire destination buffer with null bytes [3] while `strscpy` will leave the junk, uninitialized bytes trailing after the _mandatory_ NUL-termination. So, if somehow `card->driver` or `card->shortname` require this NUL-padding behavior then `strscpy_pad` should be used. My interpretation, though, is that the aforementioned fields are just fine as NUL-terminated strings. Please correct my assumptions if needed and I'll send in a v2. [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://linux.die.net/man/3/strncpy Link: KSPP#90 Link: https://lore.kernel.org/r/20230727-sound-xen-v1-1-89dd161351f1@google.com (related ALSA patch) Signed-off-by: Justin Stitt <justinstitt@google.com>
`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: KSPP#90 Signed-off-by: Justin Stitt <justinstitt@google.com>
`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: KSPP#90 Signed-off-by: Justin Stitt <justinstitt@google.com>
`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`! It should be noted that, in this case, the destination buffer has a length strictly greater than the source string. Moreover, the source string is NUL-terminated (and so is the destination) which means there was no real bug happening here. Nonetheless, this patch would get us one step closer to eliminating the `strncpy` API in the kernel, as its use is too ambiguous. We need to favor less ambiguous replacements such as: strscpy, strscpy_pad, strtomem and strtomem_pad (amongst others). Technically, my patch yields subtly different behavior. The original implementation with `strncpy` would fill the entire destination buffer with null bytes [3] while `strscpy` will leave the junk, uninitialized bytes trailing after the _mandatory_ NUL-termination. So, if somehow `pcm->name` or `card->driver/shortname/longname` require this NUL-padding behavior then `strscpy_pad` should be used. My interpretation, though, is that the aforementioned fields are just fine as NUL-terminated strings. Please correct my assumptions if needed and I'll send in a v2. [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://linux.die.net/man/3/strncpy Link: KSPP#90 Signed-off-by: Justin Stitt <justinstitt@google.com> Reviewed-by: Kees Cook <keescook@chromium.org> Link: https://lore.kernel.org/r/20230727-sound-xen-v1-1-89dd161351f1@google.com Signed-off-by: Takashi Iwai <tiwai@suse.de>
`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`! It should be noted that, in this case, the destination buffer has a length strictly greater than the source string. Moreover, the source string is NUL-terminated (and so is the destination) which means there was no real bug happening here. Nonetheless, this patch would get us one step closer to eliminating the `strncpy` API in the kernel, as its use is too ambiguous. We need to favor less ambiguous replacements such as: strscpy, strscpy_pad, strtomem and strtomem_pad (amongst others). Technically, my patch yields subtly different behavior. The original implementation with `strncpy` would fill the entire destination buffer with null bytes [3] while `strscpy` will leave the junk, uninitialized bytes trailing after the _mandatory_ NUL-termination. So, if somehow `card->driver` or `card->shortname` require this NUL-padding behavior then `strscpy_pad` should be used. My interpretation, though, is that the aforementioned fields are just fine as NUL-terminated strings. Please correct my assumptions if needed and I'll send in a v2. [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://linux.die.net/man/3/strncpy Link: KSPP#90 Link: https://lore.kernel.org/r/20230727-sound-xen-v1-1-89dd161351f1@google.com (related ALSA patch) Signed-off-by: Justin Stitt <justinstitt@google.com> Link: https://lore.kernel.org/r/20230727-sound-usb-bcd2000-v1-1-0dc73684b2f0@google.com Signed-off-by: Takashi Iwai <tiwai@suse.de>
`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: KSPP#90 Signed-off-by: Justin Stitt <justinstitt@google.com> Link: https://lore.kernel.org/r/20230727-sound-soc-codecs-v1-1-562fa2836bf4@google.com Signed-off-by: Mark Brown <broonie@kernel.org>
`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: KSPP#90 Signed-off-by: Justin Stitt <justinstitt@google.com> Acked-by: Shengjiu Wang <shengjiu.wang@gmail.com> Link: https://lore.kernel.org/r/20230727-sound-soc-fsl-v1-1-4fc0ed7e0366@google.com Signed-off-by: Mark Brown <broonie@kernel.org>
strncpy() is deprecated for NUL-terminated destination buffers. Use strscpy_pad() instead and remove the manual NUL-termination. Compile-tested only. Link: KSPP#90 Cc: linux-hardening@vger.kernel.org Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev> Reviewed-by: Kees Cook <kees@kernel.org> Reviewed-by: Allison Henderson <allison.henderson@oracle.com> Tested-by: Allison Henderson <allison.henderson@oracle.com> Signed-off-by: NipaLocal <nipa@local>
strncpy() is deprecated for NUL-terminated destination buffers. Use strscpy_pad() instead and remove the manual NUL-termination. Compile-tested only. Link: KSPP#90 Cc: linux-hardening@vger.kernel.org Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev> Reviewed-by: Kees Cook <kees@kernel.org> Reviewed-by: Allison Henderson <allison.henderson@oracle.com> Tested-by: Allison Henderson <allison.henderson@oracle.com> Signed-off-by: NipaLocal <nipa@local>
strncpy() is deprecated for NUL-terminated destination buffers. Use strscpy_pad() instead and remove the manual NUL-termination. Compile-tested only. Link: KSPP#90 Cc: linux-hardening@vger.kernel.org Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev> Reviewed-by: Kees Cook <kees@kernel.org> Reviewed-by: Allison Henderson <allison.henderson@oracle.com> Tested-by: Allison Henderson <allison.henderson@oracle.com> Signed-off-by: NipaLocal <nipa@local>
strncpy() is deprecated for NUL-terminated destination buffers. Use strscpy_pad() instead and remove the manual NUL-termination. Compile-tested only. Link: KSPP#90 Cc: linux-hardening@vger.kernel.org Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev> Reviewed-by: Kees Cook <kees@kernel.org> Reviewed-by: Allison Henderson <allison.henderson@oracle.com> Tested-by: Allison Henderson <allison.henderson@oracle.com> Signed-off-by: NipaLocal <nipa@local>
strncpy() is deprecated for NUL-terminated destination buffers. Use strscpy_pad() instead and remove the manual NUL-termination. Compile-tested only. Link: KSPP#90 Cc: linux-hardening@vger.kernel.org Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev> Reviewed-by: Kees Cook <kees@kernel.org> Reviewed-by: Allison Henderson <allison.henderson@oracle.com> Tested-by: Allison Henderson <allison.henderson@oracle.com> Signed-off-by: NipaLocal <nipa@local>
strncpy() is deprecated for NUL-terminated destination buffers. Use strscpy_pad() instead and remove the manual NUL-termination. Compile-tested only. Link: KSPP#90 Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev> Reviewed-by: Kees Cook <kees@kernel.org> Reviewed-by: Allison Henderson <allison.henderson@oracle.com> Tested-by: Allison Henderson <allison.henderson@oracle.com> Link: https://patch.msgid.link/20250219224730.73093-2-thorsten.blum@linux.dev Signed-off-by: Jakub Kicinski <kuba@kernel.org>
strncpy() is deprecated for NUL-terminated destination buffers. Use strscpy() instead and remove the manual NUL-termination. Compile-tested only. Link: KSPP#90 Cc: linux-hardening@vger.kernel.org Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
strncpy() is deprecated for NUL-terminated destination buffers. Use strscpy() instead and remove the manual NUL-termination. Compile-tested only. Link: KSPP#90 Cc: linux-hardening@vger.kernel.org Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev> Tested-by: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org> Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org> Reviewed-by: Kees Cook <kees@kernel.org> Signed-off-by: Greg Ungerer <gerg@linux-m68k.org>
strncpy() is deprecated for NUL-terminated destination buffers. Use strscpy_pad() instead and don't zero-initialize the param array. Compile-tested only. Link: KSPP#90 Cc: linux-hardening@vger.kernel.org Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
strncpy() is deprecated for NUL-terminated destination buffers. Use strscpy() instead and don't zero-initialize the param array. Compile-tested only. Link: KSPP#90 Cc: linux-hardening@vger.kernel.org Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
strncpy() is deprecated for NUL-terminated destination buffers. Use strscpy() instead and remove the manual NUL-termination. Compile-tested only. Link: KSPP#90 Cc: linux-hardening@vger.kernel.org Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
Since kstrtol() requires a NUL-terminated string as input and strncpy() is deprecated for NUL-terminated destination buffers, use strscpy() instead. Compile-tested only. Link: KSPP#90 Cc: linux-hardening@vger.kernel.org Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
`strncpy` is deprecated for use on NUL-terminated destination strings [1] and as such we should prefer more robust and less ambiguous string interfaces. We should NUL-pad as there are full struct copies happening in places: | struct drm_mode_modeinfo umode; | | ... | struct drm_property_blob *blob; | | drm_mode_convert_to_umode(&umode, mode); | blob = drm_property_create_blob(crtc->dev, | sizeof(umode), &umode); A suitable replacement is `strscpy_pad` due to the fact that it guarantees both NUL-termination and NUL-padding on the destination buffer. Additionally, replace size macro `DRM_DISPLAY_MODE_LEN` with sizeof() to more directly tie the maximum buffer size to the destination buffer: | struct drm_display_mode { | ... | char name[DRM_DISPLAY_MODE_LEN]; Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1] Link: KSPP/linux#90 Cc: linux-hardening@vger.kernel.org Cc: Xu Panda <xu.panda@zte.com.cn> Signed-off-by: Justin Stitt <justinstitt@google.com> Reviewed-by: Kees Cook <keescook@chromium.org> Link: https://lore.kernel.org/r/20231016-strncpy-drivers-gpu-drm-drm_modes-c-v2-1-d0b60686e1c6@google.com Signed-off-by: Kees Cook <keescook@chromium.org>
strncpy() is deprecated for NUL-terminated destination buffers. Use strscpy() instead and don't zero-initialize the param array. Link: KSPP#90 Cc: linux-hardening@vger.kernel.org Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev> Reviewed-by: David Sterba <dsterba@suse.com> Signed-off-by: David Sterba <dsterba@suse.com>
strncpy() is deprecated for NUL-terminated destination buffers. Use strscpy() instead and don't zero-initialize the param array. Link: KSPP#90 Cc: linux-hardening@vger.kernel.org Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev> Reviewed-by: David Sterba <dsterba@suse.com> Signed-off-by: David Sterba <dsterba@suse.com>
strncpy() is deprecated for NUL-terminated destination buffers. Use strscpy() instead and don't zero-initialize the param array. Link: KSPP#90 Cc: linux-hardening@vger.kernel.org Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev> Reviewed-by: David Sterba <dsterba@suse.com> Signed-off-by: David Sterba <dsterba@suse.com>
Since strncpy() is deprecated for NUL-terminated destination buffers, use strscpy() instead. Compile-tested only. Link: KSPP#90 Cc: linux-hardening@vger.kernel.org Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
strncpy() is deprecated for NUL-terminated destination buffers; use strscpy() instead. Compile-tested only. Link: KSPP#90 Cc: linux-hardening@vger.kernel.org Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
strncpy() is deprecated for use on NUL-terminated destination strings [1] and as such we should prefer more robust and less ambiguous string interfaces. Based on the deliberate `sizeof(dest) ... - 1` pattern we can see that both dump_info->dev_human_readable and dump_info->bus_human_readable are intended to be NUL-terminated. Moreover, since this seems to cross the file boundary let's NUL-pad to ensure no behavior change. strscpy_pad() covers both the NUL-termination and NUL-padding, let's use it. Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1] Link: KSPP#90 Cc: linux-hardening@vger.kernel.org Signed-off-by: Justin Stitt <justinstitt@google.com> Reviewed-by: Kees Cook <keescook@chromium.org> Signed-off-by: Kalle Valo <kvalo@kernel.org> Link: https://lore.kernel.org/r/20231019-strncpy-drivers-net-wireless-intel-iwlwifi-fw-dbg-c-v2-1-179b211a374b@google.com (cherry picked from commit 70582e2)
strncpy() is deprecated for NUL-terminated destination buffers; use strscpy() instead. Compile-tested only. Link: KSPP#90 Cc: linux-hardening@vger.kernel.org Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
strncpy() is deprecated for NUL-terminated destination buffers. Use strscpy() instead and remove the manual NUL-termination. Compile-tested only. Link: KSPP#90 Cc: linux-hardening@vger.kernel.org Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev> Signed-off-by: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
strncpy() is deprecated for NUL-terminated destination buffers. Use strscpy() instead and remove the manual NUL-termination. Compile-tested only. Link: KSPP/linux#90 Signed-off-by: Michael Estner <michaelestner@web.de>
strncpy() is deprecated for NUL-terminated destination buffers. Use strscpy() instead and remove the manual NUL-termination. Compile-tested only. Link: KSPP#90 Signed-off-by: Michael Estner <michaelestner@web.de>
strncpy() is deprecated for NUL-terminated destination buffers. Use strscpy() instead and remove the manual NUL-termination. Compile-tested only. Link: KSPP/linux#90 Signed-off-by: Michael Estner <michaelestner@web.de>
strncpy() is deprecated for NUL-terminated destination buffers; use strscpy() instead. The destination buffer db_root is only used with "%s" format strings and must therefore be NUL-terminated, but not NUL-padded. Use scnprintf() because snprintf() could return a value >= DB_ROOT_LEN and lead to an out-of-bounds access. This doesn't happen because count is explicitly checked against DB_ROOT_LEN before. However, scnprintf() always returns the number of characters actually written to the string buffer, which is always within the bounds of db_root_stage, and should be preferred over snprintf(). The size parameter of strscpy() is optional and since DB_ROOT_LEN is the size of the destination buffer, it can be removed. Remove it to simplify the code. Compile-tested only. Link: KSPP#90 Link: KSPP#105 Cc: linux-hardening@vger.kernel.org Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
strncpy() is deprecated for NUL-terminated destination buffers; use strscpy() instead. Compile-tested only. Note(groeck): strscpy() uses sizeof() to determine the length of the destination buffer if it is not provided as argument. Link: KSPP#90 Cc: linux-hardening@vger.kernel.org Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev> Link: https://lore.kernel.org/r/20250227173936.7746-2-thorsten.blum@linux.dev Signed-off-by: Guenter Roeck <linux@roeck-us.net>
strncpy() is deprecated for NUL-terminated destination buffers. Use strscpy() instead and don't zero-initialize the param array. Link: KSPP#90 Cc: linux-hardening@vger.kernel.org Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev> Reviewed-by: David Sterba <dsterba@suse.com> Signed-off-by: David Sterba <dsterba@suse.com>
strncpy() is deprecated for use on NUL-terminated destination strings [1] and as such we should prefer more robust and less ambiguous string interfaces. Based on the deliberate `sizeof(dest) ... - 1` pattern we can see that both dump_info->dev_human_readable and dump_info->bus_human_readable are intended to be NUL-terminated. Moreover, since this seems to cross the file boundary let's NUL-pad to ensure no behavior change. strscpy_pad() covers both the NUL-termination and NUL-padding, let's use it. Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1] Link: KSPP#90 Cc: linux-hardening@vger.kernel.org Signed-off-by: Justin Stitt <justinstitt@google.com> Reviewed-by: Kees Cook <keescook@chromium.org> Signed-off-by: Kalle Valo <kvalo@kernel.org> Link: https://lore.kernel.org/r/20231019-strncpy-drivers-net-wireless-intel-iwlwifi-fw-dbg-c-v2-1-179b211a374b@google.com (cherry picked from commit 70582e2)
`strncpy` is deprecated for use on NUL-terminated destination strings [1] and as such we should prefer more robust and less ambiguous string interfaces. We should NUL-pad as there are full struct copies happening in places: | struct drm_mode_modeinfo umode; | | ... | struct drm_property_blob *blob; | | drm_mode_convert_to_umode(&umode, mode); | blob = drm_property_create_blob(crtc->dev, | sizeof(umode), &umode); A suitable replacement is `strscpy_pad` due to the fact that it guarantees both NUL-termination and NUL-padding on the destination buffer. Additionally, replace size macro `DRM_DISPLAY_MODE_LEN` with sizeof() to more directly tie the maximum buffer size to the destination buffer: | struct drm_display_mode { | ... | char name[DRM_DISPLAY_MODE_LEN]; Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1] Link: KSPP/linux#90 Cc: linux-hardening@vger.kernel.org Cc: Xu Panda <xu.panda@zte.com.cn> Signed-off-by: Justin Stitt <justinstitt@google.com> Reviewed-by: Kees Cook <keescook@chromium.org> Link: https://lore.kernel.org/r/20231016-strncpy-drivers-gpu-drm-drm_modes-c-v2-1-d0b60686e1c6@google.com Signed-off-by: Kees Cook <keescook@chromium.org>
strncpy() is deprecated for NUL-terminated destination buffers; use strscpy() instead. The destination buffer db_root is only used with "%s" format strings and must therefore be NUL-terminated, but not NUL-padded. Use scnprintf() because snprintf() could return a value >= DB_ROOT_LEN and lead to an out-of-bounds access. This doesn't happen because count is explicitly checked against DB_ROOT_LEN before. However, scnprintf() always returns the number of characters actually written to the string buffer, which is always within the bounds of db_root_stage, and should be preferred over snprintf(). The size parameter of strscpy() is optional and since DB_ROOT_LEN is the size of the destination buffer, it can be removed. Remove it to simplify the code. Compile-tested only. Link: KSPP#90 Link: KSPP#105 Cc: linux-hardening@vger.kernel.org Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev> Link: https://lore.kernel.org/r/20250302225641.245127-2-thorsten.blum@linux.dev Reviewed-by: Kees Cook <kees@kernel.org> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
The
strncpy()
function is actively dangerous to use since it may not NUL-terminate the destination string, resulting in potential memory content exposures, unbounded reads, or crashes. Replacing uses requires some careful attention, though, sincestrncpy
gets used also for two other cases:NLA_STRING
where the length stored separately, andNLA_NUL_STRING
which uses a "traditional" NUL-terminated string. For the cases wherestrncpy()
is used to copy non-NUL-terminated strings, the destination buffer needs to be marked with the__nonstring
attribute, so that compiler diagnostics will avoid warning about cases where the character array is considered NUL-terminated.strscpy_pad()
, ormemcpy_and_pad()
when the destination is length-bounded.https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings
The workflow to replace
strncpy()
, therefore, needs to be:kstrndup()
strscpy_pad()
strscpy()
__nonstring
attribute and:memcpy_and_pad(dst, sizeof(dst), src, min(sizeof(dst), strnlen(src, sizeof(dst)), pad_char)
(perhaps this needs a macro)memcpy(dst, src, min(sizeof(dst), strnlen(src, sizeof(dst))
(perhaps this needs a macro)One can find instances to replace using this search:
git grep '\bstrncpy\b' | grep -vE '^(Documentation|tools|samples|scripts|arch/.*/lib)/' | grep -vE 'fortify|include/linux/string.h|lib/string.c'
The text was updated successfully, but these errors were encountered: