Skip to content
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

Open
kees opened this issue Aug 11, 2020 · 6 comments
Open

Remove all strncpy() uses #90

kees opened this issue Aug 11, 2020 · 6 comments
Labels
good first issue Good for newcomers [Refactor] strcpy Replace uses of unsafe strcpy-family functions

Comments

@kees
Copy link

kees commented Aug 11, 2020

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, since strncpy gets used also for two other cases:

  • length-bounded (not NUL-terminated) strings. For example, see the difference between NLA_STRING where the length stored separately, and NLA_NUL_STRING which uses a "traditional" NUL-terminated string. For the cases where strncpy() 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.
  • destinations require trailing NUL-padding. For example, when a fixed-size buffer is used to carry a string (NUL-terminated or not) across API boundaries, like when writing an ESSIDs WiFi hardware where the driver uses a fixed-size buffer. For NUL-terminated destinations, this can be done safely with strscpy_pad(), or memcpy_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:

  • if the destination buffer is NUL-terminated:
    • if allocation of the destination buffer is happening at the same time, refactor the code to use kstrndup()
    • if the destination buffer requires trailing NUL-padding, use strscpy_pad()
    • otherwise, use strscpy()
  • if the destination buffer is length-bounded (i.e. not required to be NUL-terminated):
    • mark the destination buffer variable (or structure member) with the __nonstring attribute and:
      • if the destination buffer requires trailing padding, use memcpy_and_pad(dst, sizeof(dst), src, min(sizeof(dst), strnlen(src, sizeof(dst)), pad_char) (perhaps this needs a macro)
      • otherwise, use 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'

@kees kees added the good first issue Good for newcomers label May 5, 2021
@kees kees added the [Refactor] strcpy Replace uses of unsafe strcpy-family functions label Apr 7, 2022
intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this issue Jun 7, 2022
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>
ColinIanKing pushed a commit to ColinIanKing/linux-next that referenced this issue Jun 14, 2022
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>
cschaufler pushed a commit to cschaufler/smack-next that referenced this issue Aug 1, 2022
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>
@kees kees changed the title Remove all strncpy() uses on NUL-terminated strings in favor of strscpy() or strscpy_pad() Remove all strncpy() uses Aug 26, 2022
intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this issue Aug 31, 2022
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>
intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this issue Sep 1, 2022
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>
intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this issue Sep 2, 2022
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>
intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this issue Sep 6, 2022
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>
intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this issue Sep 8, 2022
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>
@JustinStitt
Copy link
Collaborator

JustinStitt commented Jul 14, 2023

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 strtomem...() fail)

There are some pathological cases in net: dsa such as: net/dsa/slave.c:dsa_slave_get_strings which seems to populate a length-bounded array with string literals + padding. There is some heavy indirection (via function pointers and mismatched casts) that make it hard to determine whether or not the dest buffer will have its size properly computed by __builtin_object_size().

If dest cannot have its sized computed by the compiler then the macros strtomem...() will fail (by design).

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 strtomem_pad calls yields a BUILD_BUG_ON asssertion failure. What is the next best option to rid strncpy uses in cases like these (or do we leave them be)?

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

                 ┌───────────────────────┐
                 │Is dest NUL-terminated?│
                 └──────────┬────────────┘
                            │
                            │
                       N    │   Y
                   ┌────────┴───────┐
                   ▼                ▼
       ┌──────────────────────┐    ...
       │dest requires padding?│
       └──────────┬───────────┘
                  │
              N   │   Y
         ┌────────┴───────┐
         ▼                ▼
┌───────────────┐  ┌──────────────────┐
│ use strtomem()│  │use strtomem_pad()│
└──────┬────────┘  └─────────────────┬┘
       │                             │
       ▼                             ▼
      ...             ┌────────────────────────────────┐
                      │dest size compiler-determinable?│
                      └──────────────┬─────────────────┘
                               N     │   Y
                          ┌──────────┴──────┐
                          ▼                 ▼
                     ┌─────┐              ┌────┐
      I am here ---> │Uh,Oh│              │ OK!│
                     └─────┘              └────┘

Somewhat Reviewed by: @nickdesaulniers

@JustinStitt
Copy link
Collaborator

JustinStitt commented Jul 14, 2023

After some tinkering, I think I've found a viable solution.

If there existed some strntomem_pad (name tentative) which accepted dest_len as a macro parameter then use cases where the dest buffer's length is not determinable can simply provide dest_len.

#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 net/dsa/slave.c:dsa_slave_get_strings:

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 dest NUL-terminated?│
                 └──────────┬────────────┘
                            │
                            │
                       N    │   Y
                   ┌────────┴───────┐
                   ▼                ▼
       ┌──────────────────────┐    ...
       │dest requires padding?│
       └──────────┬───────────┘
                  │
              N   │   Y
         ┌────────┴───────┐
         ▼                ▼
┌───────────────┐  ┌──────────────────┐
│ use strtomem()│  │use strtomem_pad()│
└──────┬────────┘  └─────────────────┬┘
       │                             │
       ▼                             ▼
      ...             ┌────────────────────────────────┐
                      │dest size compiler-determinable?│
                      └──────────────┬─────────────────┘
                               N     │         Y
                          ┌──────────┴───────────────┐
                          ▼                          ▼
                     ┌─────────────┐              ┌────┐
   ✅ I am here ---> │strntomem_pad│              │ OK!│
                     └─────────────┘              └────┘

Is this a viable solution?

@kees
Copy link
Author

kees commented Jul 16, 2023

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 get_strings handlers all have to trust that enough memory has been allocated. 😡

Anyway, I suspect the best fix here is to teach the handlers the size of the buffers in some way.

@kees
Copy link
Author

kees commented Jul 16, 2023

Given that the strings in question will not be truncated and other get_strings implementations already use strscpy and the buffer is already zero-filled (vzalloc), I think just swapping the strncpy calls here with strscpy is fine. Refactoring the entire get_strings API to pass allocation size looks unpleasant, but would probably find real bugs.

@JustinStitt
Copy link
Collaborator

@kees Thoughts on this article:

tl;dr: Linus doesn't want large swaths of strncpy -> strscpy patches.

Linus was clearly afraid that the strscpy() patch could end up being a source of regressions as well. That wouldn't happen with the patch set itself, which does not convert any existing strncpy() or strlcpy() call sites. The problem happens when other, well-intentioned developers start doing those conversions. Linus described his worries in the merge commit that brought in strscpy():

So why did I waffle about this for so long?
Every time we introduce a new-and-improved interface, people start doing these interminable series of trivial conversion patches.

And every time that happens, somebody does some silly mistake, and the conversion patch to the improved interface actually makes things worse. Because the patch is mindnumbing and trivial, nobody has the attention span to look at it carefully, and it's usually done over large swatches of source code which means that not every conversion gets tested.

To try to head off such an outcome, Linus has made it clear that he will not be accepting patches that do mass conversions to strscpy() (note though that certain developers are already considering mass conversions anyway). It is there to be used with new code, but existing code should not be converted without some compelling reason to do so — or without a high level of attention to the possible implications of the change.

One might be tempted to think that this proclamation from Linus signals the end of the "trivial clean-up patch" era. But that would almost certainly be reading too much into what he said. Patches that do not make functional changes to the code do not, one would hope, pose the same sort of risk that API replacements do. So the flow of white-space adjustments is likely to continue unabated. But developers who want to convert a bunch of working code to a "safer" interface may want to think twice before sending in a patch.

@kees
Copy link
Author

kees commented Jul 17, 2023

@kees Thoughts on this article:

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.)

intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this issue Jul 18, 2023
`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>
intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this issue Jul 18, 2023
`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>
intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this issue Jul 26, 2023
`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>
intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this issue Jul 27, 2023
`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>
intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this issue Jul 27, 2023
`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>
intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this issue Jul 27, 2023
`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>
intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this issue Jul 27, 2023
`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>
intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this issue Jul 27, 2023
`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>
intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this issue Jul 27, 2023
`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>
tiwai pushed a commit to tiwai/sound that referenced this issue Jul 29, 2023
`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>
tiwai pushed a commit to tiwai/sound that referenced this issue Jul 29, 2023
`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>
intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this issue Jul 30, 2023
`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>
intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this issue Jul 30, 2023
`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>
kuba-moo pushed a commit to linux-netdev/testing that referenced this issue Feb 21, 2025
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>
kuba-moo pushed a commit to linux-netdev/testing that referenced this issue Feb 21, 2025
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>
kuba-moo pushed a commit to linux-netdev/testing that referenced this issue Feb 21, 2025
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>
kuba-moo pushed a commit to linux-netdev/testing that referenced this issue Feb 21, 2025
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>
kuba-moo pushed a commit to linux-netdev/testing that referenced this issue Feb 21, 2025
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>
kuba-moo pushed a commit to linux-netdev/testing that referenced this issue Feb 22, 2025
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>
intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this issue Feb 24, 2025
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>
github-actions bot pushed a commit to anon503/linux that referenced this issue Feb 25, 2025
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>
intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this issue Feb 25, 2025
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>
intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this issue Feb 25, 2025
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>
intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this issue Feb 25, 2025
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>
intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this issue Feb 25, 2025
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>
dumbbell pushed a commit to dumbbell/drm-kmod that referenced this issue Feb 26, 2025
`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>
kdave pushed a commit to btrfs/linux that referenced this issue Feb 26, 2025
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>
kdave pushed a commit to kdave/btrfs-devel that referenced this issue Feb 26, 2025
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>
kdave pushed a commit to btrfs/linux that referenced this issue Feb 26, 2025
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>
intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this issue Feb 26, 2025
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>
intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this issue Feb 26, 2025
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>
Sh3Rm4n pushed a commit to Sh3Rm4n/linux that referenced this issue Feb 27, 2025
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)
intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this issue Feb 27, 2025
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>
staging-kernelci-org pushed a commit to kernelci/linux that referenced this issue Feb 28, 2025
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>
kernel-patches-daemon-bpf-rc bot pushed a commit to kernel-patches/bpf-rc that referenced this issue Feb 28, 2025
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>
intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this issue Feb 28, 2025
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>
kernel-patches-daemon-bpf bot pushed a commit to kernel-patches/bpf that referenced this issue Feb 28, 2025
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>
intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this issue Mar 2, 2025
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>
github-actions bot pushed a commit to anon503/linux that referenced this issue Mar 3, 2025
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>
kdave pushed a commit to btrfs/linux that referenced this issue Mar 3, 2025
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>
Sh3Rm4n pushed a commit to Sh3Rm4n/linux that referenced this issue Mar 3, 2025
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)
dumbbell pushed a commit to dumbbell/drm-kmod that referenced this issue Mar 3, 2025
`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>
github-actions bot pushed a commit to anon503/linux that referenced this issue Mar 4, 2025
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers [Refactor] strcpy Replace uses of unsafe strcpy-family functions
Projects
None yet
Development

No branches or pull requests

2 participants