[RFC PATCH v2 1/1] kernfs: replace deprecated strlcpy() with strscpy()

From: Mirsad Goran Todorovac
Date: Wed Nov 22 2023 - 16:23:16 EST


From: Mirsad Todorovac <mirsad.todorovac@xxxxxxxxxxxx>

According to strlcpy() being officially deprecated and the encouragement
to remove the remaining occurrences, this came as the intriguing example.

In the kernfs_name_locked() the behaviour of truncating the kn->name is
preserved, for it only used in the module for printing in the log and
declared static. It is only called from pr_cont_kernfs_name() via kernfs_name()
and returned result is ignored.

It is avoided to go past the allocated page and cause the internal equivalent
of SEGFAULT in the unlikely case kn->name is not null-terminated, which I
believe was the idea behind replacing strlcpy() with strscpy().

kernfs_path_from_node_locked() has "(null)" which certainly cannot overrun,
and a carefully calculated len and truncated path elsewhere.

Fixes: 17627157cda13 ("kernfs: handle null pointers while printing node name and path")
Fixes: 3eef34ad7dc36 ("kernfs: implement kernfs_get_parent(), kernfs_name/path() and friends")
Fixes: 9f6df573a4041 ("kernfs: Add API to generate relative kernfs path")
Fixes: 3abb1d90f5d93 ("kernfs: make kernfs_path*() behave in the style of strlcpy()")
Fixes: e56ed358afd81 ("kernfs: make kernfs_walk_ns() use kernfs_pr_cont_buf[]")
Cc: Konstantin Khlebnikov <koct9i@xxxxxxxxx>
Cc: Aditya Kali <adityakali@xxxxxxxxxx>
Cc: Tejun Heo <tj@xxxxxxxxxx>
Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
Cc: linux-kernel@xxxxxxxxxxxxxxx
Link: https://lwn.net/ml/ksummit-discuss/20231019054642.GF14346@xxxxxx/#t
Link: https://lwn.net/Articles/659214/
Signed-off-by: Mirsad Todorovac <mirsad.todorovac@xxxxxxxxxxxx>
---
v2:
Fixed a Cc: address bug.
Fixing the patch according to the new definition of strscpy() in
LWN article https://lwn.net/Articles/659214/ that makes strscpy_truncate()
obsoleted.

fs/kernfs/dir.c | 25 ++++++++++++++++---------
1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
index 8b2bd65d70e7..a6971a6756bc 100644
--- a/fs/kernfs/dir.c
+++ b/fs/kernfs/dir.c
@@ -53,10 +53,17 @@ static bool kernfs_lockdep(struct kernfs_node *kn)

static int kernfs_name_locked(struct kernfs_node *kn, char *buf, size_t buflen)
{
+ size_t len;
+
if (!kn)
- return strlcpy(buf, "(null)", buflen);
+ return strscpy(buf, "(null)", buflen);
+
+ len = strscpy(buf, kn->parent ? kn->name : "/", buflen);

- return strlcpy(buf, kn->parent ? kn->name : "/", buflen);
+ if (unlikely(len == -E2BIG)) {
+ return buflen - 1;
+ } else
+ return len;
}

/* kernfs_node_depth - compute depth from @from to @to */
@@ -141,13 +148,13 @@ static int kernfs_path_from_node_locked(struct kernfs_node *kn_to,
int i, j;

if (!kn_to)
- return strlcpy(buf, "(null)", buflen);
+ return strscpy(buf, "(null)", buflen);

if (!kn_from)
kn_from = kernfs_root(kn_to)->kn;

if (kn_from == kn_to)
- return strlcpy(buf, "/", buflen);
+ return strscpy(buf, "/", buflen);

common = kernfs_common_ancestor(kn_from, kn_to);
if (WARN_ON(!common))
@@ -159,16 +166,16 @@ static int kernfs_path_from_node_locked(struct kernfs_node *kn_to,
buf[0] = '\0';

for (i = 0; i < depth_from; i++)
- len += strlcpy(buf + len, parent_str,
+ len += strscpy(buf + len, parent_str,
len < buflen ? buflen - len : 0);

/* Calculate how many bytes we need for the rest */
for (i = depth_to - 1; i >= 0; i--) {
for (kn = kn_to, j = 0; j < i; j++)
kn = kn->parent;
- len += strlcpy(buf + len, "/",
+ len += strscpy(buf + len, "/",
len < buflen ? buflen - len : 0);
- len += strlcpy(buf + len, kn->name,
+ len += strscpy(buf + len, kn->name,
len < buflen ? buflen - len : 0);
}

@@ -857,9 +864,9 @@ static struct kernfs_node *kernfs_walk_ns(struct kernfs_node *parent,

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 (unlikely(len == -E2BIG)) {
spin_unlock_irq(&kernfs_pr_cont_lock);
return NULL;
}
--
2.34.1