2023-12-12 21:18:01

by Kees Cook

Subject: [PATCH v3 1/3] kernfs: Convert kernfs_walk_ns() from strlcpy() to strscpy()

strlcpy() reads the entire source buffer first. This read may exceed
the destination size limit. This is both inefficient and can lead
to linear read overflows if a source string is not NUL-terminated[1].
Additionally, it returns the size of the source string, not the
resulting size of the destination string. In an effort to remove strlcpy()
completely[2], replace strlcpy() here with strscpy().

Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy [1]
Link: https://github.com/KSPP/linux/issues/89 [2]
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Tejun Heo <[email protected]>
Cc: Azeem Shaikh <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Kees Cook <[email protected]>
---
fs/kernfs/dir.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
index 8b2bd65d70e7..37353901ede1 100644
--- a/fs/kernfs/dir.c
+++ b/fs/kernfs/dir.c
@@ -850,16 +850,16 @@ static struct kernfs_node *kernfs_walk_ns(struct kernfs_node *parent,
const unsigned char *path,
const void *ns)
{
- size_t len;
+ ssize_t len;
char *p, *name;

lockdep_assert_held_read(&kernfs_root(parent)->kernfs_rwsem);

spin_lock_irq(&kernfs_pr_cont_lock);

- len = strlcpy(kernfs_pr_cont_buf, path, sizeof(kernfs_pr_cont_buf));
+ len = strscpy(kernfs_pr_cont_buf, path, sizeof(kernfs_pr_cont_buf));

- if (len >= sizeof(kernfs_pr_cont_buf)) {
+ if (len < 0) {
spin_unlock_irq(&kernfs_pr_cont_lock);
return NULL;
}
--
2.34.1

2023-12-22 00:58:25

by Tejun Heo

Subject: Re: [PATCH v3 1/3] kernfs: Convert kernfs_walk_ns() from strlcpy() to strscpy()

On Tue, Dec 12, 2023 at 01:17:38PM -0800, Kees Cook wrote:
> strlcpy() reads the entire source buffer first. This read may exceed
> the destination size limit. This is both inefficient and can lead
> to linear read overflows if a source string is not NUL-terminated[1].
> Additionally, it returns the size of the source string, not the
> resulting size of the destination string. In an effort to remove strlcpy()
> completely[2], replace strlcpy() here with strscpy().
>
> Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy [1]
> Link: https://github.com/KSPP/linux/issues/89 [2]
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: Tejun Heo <[email protected]>
> Cc: Azeem Shaikh <[email protected]>
> Link: https://lore.kernel.org/r/[email protected]
> Signed-off-by: Kees Cook <[email protected]>

Acked-by: Tejun Heo <[email protected]>

Thanks.

--
tejun