Commits · d4309246cf6beb1c73b97b4d3bf976969793eb05 · Kirill Smelkov / linux

25 Apr, 2024 2 commits

scsi: mpi3mr: Avoid possible run-time warning with long manufacturer strings · d4309246

Kees Cook authored Apr 09, 2024

The prior use of strscpy() here expected the manufacture_reply strings to
be NUL-terminated, but it is possible they are not, as the code pattern
here shows, e.g., edev->vendor_id being exactly 1 character larger than
manufacture_reply->vendor_id, and the strscpy() was copying only up to
the size of the source character array. Replace this with memtostr(),
which is the unambiguous way to convert a maybe not-NUL-terminated
character array into a NUL-terminated string.

Fixes: 2bd37e28 ("scsi: mpi3mr: Add framework to issue MPT transport cmds")
Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
Link: https://lore.kernel.org/r/20240410023155.2100422-4-keescook@chromium.orgSigned-off-by: Kees Cook <keescook@chromium.org>

d4309246

scsi: mptfusion: Avoid possible run-time warning with long manufacturer strings · 5bb288c4

Kees Cook authored Apr 09, 2024

The prior strscpy() replacement of strncpy() here expected the
manufacture_reply strings to be NUL-terminated, but it is possible
they are not, as the code pattern here shows, e.g., edev->vendor_id
being exactly 1 character larger than manufacture_reply->vendor_id,
and the replaced strncpy() was copying only up to the size of the
source character array. Replace this with memtostr(), which is the
unambiguous way to convert a maybe not-NUL-terminated character array
into a NUL-terminated string.
Reported-by: Charles Bertsch <cbertsch@cox.net>
Closes: https://lore.kernel.org/all/5445ba0f-3e27-4d43-a9ba-0cc22ada2fce@cox.net/
Fixes: 45e833f0 ("scsi: message: fusion: Replace deprecated strncpy() with strscpy()")
Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
Link: https://lore.kernel.org/r/20240410023155.2100422-2-keescook@chromium.orgSigned-off-by: Kees Cook <keescook@chromium.org>

5bb288c4

24 Apr, 2024 9 commits

fs: ecryptfs: replace deprecated strncpy with strscpy · f700b719

Justin Stitt authored Mar 21, 2024

strncpy() is deprecated for use on NUL-terminated destination strings
[1] and as such we should prefer more robust and less ambiguous string
interfaces. A good alternative is strscpy() as it guarantees
NUL-termination on the destination buffer.

In crypto.c:
We expect cipher_name to be NUL-terminated based on its use with
the C-string format specifier %s and with other string apis like
strlen():
|	printk(KERN_ERR "Error attempting to initialize key TFM "
|		"cipher with name = [%s]; rc = [%d]\n",
|		tmp_tfm->cipher_name, rc);
and
|	int cipher_name_len = strlen(cipher_name);

In main.c:
We can remove the manual NUL-byte assignments as well as the pointers to
destinations (which I assume only existed to trim down on line length?)
in favor of directly using the destination buffer which allows the
compiler to get size information -- enabling the usage of the new
2-argument strscpy().

Note that this patch relies on the _new_ 2-argument versions of
strscpy() and strscpy_pad() introduced in Commit e6584c39 ("string:
Allow 2-argument strscpy()").

Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1]
Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2]
Link: https://github.com/KSPP/linux/issues/90
Cc:  <linux-hardening@vger.kernel.org>
Signed-off-by: Justin Stitt <justinstitt@google.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
Link: https://lore.kernel.org/r/20240321-strncpy-fs-ecryptfs-crypto-c-v1-1-d78b74c214ac@google.comSigned-off-by: Kees Cook <keescook@chromium.org>

f700b719

hfsplus: refactor copy_name to not use strncpy · 7dcbf17e

Justin Stitt authored Apr 01, 2024

strncpy() is deprecated with NUL-terminated destination strings [1].

The copy_name() method does a lot of manual buffer manipulation to
eventually arrive with its desired string. If we don't know the
namespace this attr has or belongs to we want to prepend "osx." to our
final string. Following this, we're copying xattr_name and doing a
bizarre manual NUL-byte assignment with a memset where n=1.

Really, we can use some more obvious string APIs to acomplish this,
improving readability and security. Following the same control flow as
before: if we don't know the namespace let's use scnprintf() to form our
prefix + xattr_name pairing (while NUL-terminating too!). Otherwise, use
strscpy() to return the number of bytes copied into our buffer.
Additionally, for non-empty strings, include the NUL-byte in the length
-- matching the behavior of the previous implementation.

Note that strscpy() _can_ return -E2BIG but this is already handled by
all callsites:

In both hfsplus_listxattr_finder_info() and hfsplus_listxattr(), ret is
already type ssize_t so we can change the return type of copy_name() to
match (understanding that scnprintf()'s return type is different yet
fully representable by ssize_t). Furthermore, listxattr() in fs/xattr.c
is well-equipped to handle a potential -E2BIG return result from
vfs_listxattr():
|	ssize_t error;
...
|	error = vfs_listxattr(d, klist, size);
|	if (error > 0) {
|		if (size && copy_to_user(list, klist, error))
|			error = -EFAULT;
|	} else if (error == -ERANGE && size >= XATTR_LIST_MAX) {
|		/* The file system tried to returned a list bigger
|			than XATTR_LIST_MAX bytes. Not possible. */
|		error = -E2BIG;
|	}
... the error can potentially already be -E2BIG, skipping this else-if
and ending up at the same state as other errors.

Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1]
Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html
Link: https://github.com/KSPP/linux/issues/90
Cc:  <linux-hardening@vger.kernel.org>
Signed-off-by: Justin Stitt <justinstitt@google.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
Link: https://lore.kernel.org/r/20240401-strncpy-fs-hfsplus-xattr-c-v2-1-6e089999355e@google.comSigned-off-by: Kees Cook <keescook@chromium.org>

7dcbf17e

reiserfs: replace deprecated strncpy with scnprintf · 31ca7e77

Justin Stitt authored Mar 28, 2024

strncpy() is deprecated for use on NUL-terminated destination strings
[1] and as such we should prefer more robust and less ambiguous string
interfaces.

Our goal here is to get @namebuf populated with @name's contents but
surrounded with quotes. There is some careful handling done to ensure we
properly truncate @name so that we have room for a literal quote as well
as a NUL-term. All this careful handling can be done with scnprintf
using the dynamic string width specifier %.*s which allows us to pass in
the max size for a source string. Doing this, we can put literal quotes
in our format specifier and ensure @name is truncated to fit inbetween
these quotes (-3 is from 2 quotes + 1 NUL-byte).

All in all, we get to remove a deprecated use of strncpy and clean up
this code nicely!

Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1]
Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2]
Link: https://github.com/KSPP/linux/issues/90
Cc:  <linux-hardening@vger.kernel.org>
Signed-off-by: Justin Stitt <justinstitt@google.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
Link: https://lore.kernel.org/r/20240328-strncpy-fs-reiserfs-item_ops-c-v1-1-2dab6d22a996@google.comSigned-off-by: Kees Cook <keescook@chromium.org>

31ca7e77

virt: acrn: replace deprecated strncpy with strscpy · 61af39e1

Justin Stitt authored Mar 20, 2024

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 can see that client->name should be NUL-terminated based on its usage
with a %s C-string format specifier.
|	client->thread = kthread_run(ioreq_task, client, "VM%u-%s",
|					client->vm->vmid, client->name);

NUL-padding is not required as client is already zero-allocated:
|	client = kzalloc(sizeof(*client), GFP_KERNEL);

Considering the above, a suitable replacement is `strscpy` [2] due to
the fact that it guarantees NUL-termination on the destination buffer
without unnecessarily NUL-padding.

Note that this patch relies on the _new_ 2-argument version of strscpy()
introduced in Commit e6584c39 ("string: Allow 2-argument
strscpy()").

Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1]
Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2]
Link: https://github.com/KSPP/linux/issues/90
Cc:  <linux-hardening@vger.kernel.org>
Signed-off-by: Justin Stitt <justinstitt@google.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
Link: https://lore.kernel.org/r/20240320-strncpy-drivers-virt-acrn-ioreq-c-v1-1-db6996770341@google.comSigned-off-by: Kees Cook <keescook@chromium.org>

61af39e1

ubsan: Avoid i386 UBSAN handler crashes with Clang · 2e431b23

Kees Cook authored Apr 24, 2024

When generating Runtime Calls, Clang doesn't respect the -mregparm=3
option used on i386. Hopefully this will be fixed correctly in Clang 19:
https://github.com/llvm/llvm-project/pull/89707
but we need to fix this for earlier Clang versions today. Force the
calling convention to use non-register arguments.
Reported-by: Erhard Furtner <erhard_f@mailbox.org>
Closes: https://github.com/KSPP/linux/issues/350
Link: https://lore.kernel.org/r/20240424224026.it.216-kees@kernel.orgAcked-by: Nathan Chancellor <nathan@kernel.org>
Acked-by: Justin Stitt <justinstitt@google.com>
Signed-off-by: Kees Cook <keescook@chromium.org>

2e431b23

ubsan: Remove 1-element array usage in debug reporting · c2098267

Kees Cook authored Apr 24, 2024

The "type_name" character array was still marked as a 1-element array.
While we don't validate strings used in format arguments yet, let's fix
this before it causes trouble some future day.

Link: https://lore.kernel.org/r/20240424162739.work.492-kees@kernel.orgReviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org>
Signed-off-by: Kees Cook <keescook@chromium.org>

c2098267

MAINTAINERS: Add ubsan.h to the UBSAN section · 30c32991

Kees Cook authored Apr 24, 2024

The "ubsan.h" file was missed in the creation of the UBSAN section. Add
it.

Link: https://lore.kernel.org/r/20240424162722.work.576-kees@kernel.orgAcked-by: Marco Elver <elver@google.com>
Signed-off-by: Kees Cook <keescook@chromium.org>

30c32991

string_kunit: Move strtomem KUnit test to string_kunit.c · c01c41e5

Kees Cook authored Apr 24, 2024

It is more logical to have the strtomem() test in string_kunit.c instead
of the memcpy() suite. Move it to live with memtostr().
Signed-off-by: Kees Cook <keescook@chromium.org>

c01c41e5

string.h: Introduce memtostr() and memtostr_pad() · 0efc5990

Kees Cook authored Apr 09, 2024

Another ambiguous use of strncpy() is to copy from strings that may not
be NUL-terminated. These cases depend on having the destination buffer
be explicitly larger than the source buffer's maximum size, having
the size of the copy exactly match the source buffer's maximum size,
and for the destination buffer to get explicitly NUL terminated.

This usually happens when parsing protocols or hardware character arrays
that are not guaranteed to be NUL-terminated. The code pattern is
effectively this:

	char dest[sizeof(src) + 1];

	strncpy(dest, src, sizeof(src));
	dest[sizeof(dest) - 1] = '\0';

In practice it usually looks like:

struct from_hardware {
	...
	char name[HW_NAME_SIZE] __nonstring;
	...
};

	struct from_hardware *p = ...;
	char name[HW_NAME_SIZE + 1];

	strncpy(name, p->name, HW_NAME_SIZE);
	name[NW_NAME_SIZE] = '\0';

This cannot be replaced with:

	strscpy(name, p->name, sizeof(name));

because p->name is smaller and not NUL-terminated, so FORTIFY will
trigger when strnlen(p->name, sizeof(name)) is used. And it cannot be
replaced with:

	strscpy(name, p->name, sizeof(p->name));

because then "name" may contain a 1 character early truncation of
p->name.

Provide an unambiguous interface for converting a maybe not-NUL-terminated
string to a NUL-terminated string, with compile-time buffer size checking
so that it can never fail at runtime: memtostr() and memtostr_pad(). Also
add KUnit tests for both.

Link: https://lore.kernel.org/r/20240410023155.2100422-1-keescook@chromium.orgSigned-off-by: Kees Cook <keescook@chromium.org>

0efc5990

19 Apr, 2024 5 commits

string: Convert KUnit test names to standard convention · dde915c5