* [PATCH v2 0/3] kernfs: Convert from strlcpy() to strscpy()
@ 2023-11-30 20:12 Kees Cook
2023-11-30 20:12 ` [PATCH v2 1/3] kernfs: Convert kernfs_walk_ns() " Kees Cook
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Kees Cook @ 2023-11-30 20:12 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Kees Cook, Tejun Heo, Azeem Shaikh, Zefan Li, Johannes Weiner,
Waiman Long, linux-kernel, cgroups, bpf, linux-hardening
Hi,
One of the last users of strlcpy() is kernfs, which has some complex
calling hierarchies that needed to be carefully examined. This series
refactors the strlcpy() calls into strscpy() calls, and bubbles up all
changes in return value checking for callers. Future work in kernfs and
sysfs will see the replacement of open-coded string handling with the
seq_buf API, but we need to do one thing at a time.
Thanks!
-kees
v2:
- drop extraneous kernel/trace/trace_uprobe.c change
v1: https://lore.kernel.org/linux-hardening/20231116191718.work.246-kees@kernel.org/
Kees Cook (3):
kernfs: Convert kernfs_walk_ns() from strlcpy() to strscpy()
kernfs: Convert kernfs_name_locked() from strlcpy() to strscpy()
kernfs: Convert kernfs_path_from_node_locked() from strlcpy() to
strscpy()
fs/kernfs/dir.c | 53 +++++++++++++++++++++------------------
kernel/cgroup/cgroup-v1.c | 2 +-
kernel/cgroup/cgroup.c | 4 +--
kernel/cgroup/cpuset.c | 2 +-
4 files changed, 32 insertions(+), 29 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/3] kernfs: Convert kernfs_walk_ns() from strlcpy() to strscpy()
2023-11-30 20:12 [PATCH v2 0/3] kernfs: Convert from strlcpy() to strscpy() Kees Cook
@ 2023-11-30 20:12 ` Kees Cook
2023-11-30 20:12 ` [PATCH v2 2/3] kernfs: Convert kernfs_name_locked() " Kees Cook
2023-11-30 20:12 ` [PATCH v2 3/3] kernfs: Convert kernfs_path_from_node_locked() " Kees Cook
2 siblings, 0 replies; 6+ messages in thread
From: Kees Cook @ 2023-11-30 20:12 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Kees Cook, Tejun Heo, Azeem Shaikh, Zefan Li, Johannes Weiner,
Waiman Long, linux-kernel, cgroups, bpf, linux-hardening
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 <gregkh@linuxfoundation.org>
Cc: Tejun Heo <tj@kernel.org>
Cc: Azeem Shaikh <azeemshaikh38@gmail.com>
Link: https://lore.kernel.org/r/20231116192127.1558276-1-keescook@chromium.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 2/3] kernfs: Convert kernfs_name_locked() from strlcpy() to strscpy()
2023-11-30 20:12 [PATCH v2 0/3] kernfs: Convert from strlcpy() to strscpy() Kees Cook
2023-11-30 20:12 ` [PATCH v2 1/3] kernfs: Convert kernfs_walk_ns() " Kees Cook
@ 2023-11-30 20:12 ` Kees Cook
2023-11-30 20:12 ` [PATCH v2 3/3] kernfs: Convert kernfs_path_from_node_locked() " Kees Cook
2 siblings, 0 replies; 6+ messages in thread
From: Kees Cook @ 2023-11-30 20:12 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Kees Cook, Tejun Heo, Azeem Shaikh, Zefan Li, Johannes Weiner,
Waiman Long, linux-kernel, cgroups, bpf, linux-hardening
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().
Nothing actually checks the return value coming from kernfs_name_locked(),
so this has no impact on error paths. The caller hierarchy is:
kernfs_name_locked()
kernfs_name()
pr_cont_kernfs_name()
return value ignored
cgroup_name()
current_css_set_cg_links_read()
return value ignored
print_page_owner_memcg()
return value ignored
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 <gregkh@linuxfoundation.org>
Cc: Tejun Heo <tj@kernel.org>
Cc: Azeem Shaikh <azeemshaikh38@gmail.com>
Link: https://lore.kernel.org/r/20231116192127.1558276-2-keescook@chromium.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
fs/kernfs/dir.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
index 37353901ede1..8c0e5442597e 100644
--- a/fs/kernfs/dir.c
+++ b/fs/kernfs/dir.c
@@ -54,9 +54,9 @@ static bool kernfs_lockdep(struct kernfs_node *kn)
static int kernfs_name_locked(struct kernfs_node *kn, char *buf, size_t buflen)
{
if (!kn)
- return strlcpy(buf, "(null)", buflen);
+ return strscpy(buf, "(null)", buflen);
- return strlcpy(buf, kn->parent ? kn->name : "/", buflen);
+ return strscpy(buf, kn->parent ? kn->name : "/", buflen);
}
/* kernfs_node_depth - compute depth from @from to @to */
@@ -182,12 +182,12 @@ static int kernfs_path_from_node_locked(struct kernfs_node *kn_to,
* @buflen: size of @buf
*
* Copies the name of @kn into @buf of @buflen bytes. The behavior is
- * similar to strlcpy().
+ * similar to strscpy().
*
* Fills buffer with "(null)" if @kn is %NULL.
*
- * Return: the length of @kn's name and if @buf isn't long enough,
- * it's filled up to @buflen-1 and nul terminated.
+ * Return: the resulting length of @buf. If @buf isn't long enough,
+ * it's filled up to @buflen-1 and nul terminated, and returns -E2BIG.
*
* This function can be called from any context.
*/
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 3/3] kernfs: Convert kernfs_path_from_node_locked() from strlcpy() to strscpy()
2023-11-30 20:12 [PATCH v2 0/3] kernfs: Convert from strlcpy() to strscpy() Kees Cook
2023-11-30 20:12 ` [PATCH v2 1/3] kernfs: Convert kernfs_walk_ns() " Kees Cook
2023-11-30 20:12 ` [PATCH v2 2/3] kernfs: Convert kernfs_name_locked() " Kees Cook
@ 2023-11-30 20:12 ` Kees Cook
2023-11-30 20:38 ` Christophe JAILLET
2 siblings, 1 reply; 6+ messages in thread
From: Kees Cook @ 2023-11-30 20:12 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Kees Cook, Tejun Heo, Zefan Li, Johannes Weiner, Waiman Long,
cgroups, Azeem Shaikh, linux-kernel, bpf, linux-hardening
One of the last remaining users of strlcpy() in the kernel is
kernfs_path_from_node_locked(), which passes back the problematic "length
we _would_ have copied" return value to indicate truncation. Convert the
chain of all callers to use the negative return value (some of which
already doing this explicitly). All callers were already also checking
for negative return values, so the risk to missed checks looks very low.
In this analysis, it was found that cgroup1_release_agent() actually
didn't handle the "too large" condition, so this is technically also a
bug fix. :)
Here's the chain of callers, and resolution identifying each one as now
handling the correct return value:
kernfs_path_from_node_locked()
kernfs_path_from_node()
pr_cont_kernfs_path()
returns void
kernfs_path()
sysfs_warn_dup()
return value ignored
cgroup_path()
blkg_path()
bfq_bic_update_cgroup()
return value ignored
TRACE_IOCG_PATH()
return value ignored
TRACE_CGROUP_PATH()
return value ignored
perf_event_cgroup()
return value ignored
task_group_path()
return value ignored
damon_sysfs_memcg_path_eq()
return value ignored
get_mm_memcg_path()
return value ignored
lru_gen_seq_show()
return value ignored
cgroup_path_from_kernfs_id()
return value ignored
cgroup_show_path()
already converted "too large" error to negative value
cgroup_path_ns_locked()
cgroup_path_ns()
bpf_iter_cgroup_show_fdinfo()
return value ignored
cgroup1_release_agent()
wasn't checking "too large" error
proc_cgroup_show()
already converted "too large" to negative value
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Tejun Heo <tj@kernel.org>
Cc: Zefan Li <lizefan.x@bytedance.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: Waiman Long <longman@redhat.com>
Cc: cgroups@vger.kernel.org
Co-developed-by: Azeem Shaikh <azeemshaikh38@gmail.com>
Signed-off-by: Azeem Shaikh <azeemshaikh38@gmail.com>
Link: https://lore.kernel.org/r/20231116192127.1558276-3-keescook@chromium.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
fs/kernfs/dir.c | 37 ++++++++++++++++++++-----------------
kernel/cgroup/cgroup-v1.c | 2 +-
kernel/cgroup/cgroup.c | 4 ++--
kernel/cgroup/cpuset.c | 2 +-
4 files changed, 24 insertions(+), 21 deletions(-)
diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
index 8c0e5442597e..183f353b3852 100644
--- a/fs/kernfs/dir.c
+++ b/fs/kernfs/dir.c
@@ -127,7 +127,7 @@ static struct kernfs_node *kernfs_common_ancestor(struct kernfs_node *a,
*
* [3] when @kn_to is %NULL result will be "(null)"
*
- * Return: the length of the full path. If the full length is equal to or
+ * Return: the length of the constructed path. If the path would have been
* greater than @buflen, @buf contains the truncated path with the trailing
* '\0'. On error, -errno is returned.
*/
@@ -138,16 +138,17 @@ static int kernfs_path_from_node_locked(struct kernfs_node *kn_to,
struct kernfs_node *kn, *common;
const char parent_str[] = "/..";
size_t depth_from, depth_to, len = 0;
+ ssize_t copied;
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))
@@ -158,18 +159,22 @@ 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 < buflen ? buflen - len : 0);
+ for (i = 0; i < depth_from; i++) {
+ copied = strscpy(buf + len, parent_str, buflen - len);
+ if (copied < 0)
+ return copied;
+ len += copied;
+ }
/* 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 < buflen ? buflen - len : 0);
- len += strlcpy(buf + len, kn->name,
- len < buflen ? buflen - len : 0);
+
+ copied = scnprintf(buf + len, buflen - len, "/%s", kn->name);
+ if (copied < 0)
+ return copied;
+ len += copied;
}
return len;
@@ -214,7 +219,7 @@ int kernfs_name(struct kernfs_node *kn, char *buf, size_t buflen)
* path (which includes '..'s) as needed to reach from @from to @to is
* returned.
*
- * Return: the length of the full path. If the full length is equal to or
+ * Return: the length of the constructed path. If the path would have been
* greater than @buflen, @buf contains the truncated path with the trailing
* '\0'. On error, -errno is returned.
*/
@@ -265,12 +270,10 @@ void pr_cont_kernfs_path(struct kernfs_node *kn)
sz = kernfs_path_from_node(kn, NULL, kernfs_pr_cont_buf,
sizeof(kernfs_pr_cont_buf));
if (sz < 0) {
- pr_cont("(error)");
- goto out;
- }
-
- if (sz >= sizeof(kernfs_pr_cont_buf)) {
- pr_cont("(name too long)");
+ if (sz == -E2BIG)
+ pr_cont("(name too long)");
+ else
+ pr_cont("(error)");
goto out;
}
diff --git a/kernel/cgroup/cgroup-v1.c b/kernel/cgroup/cgroup-v1.c
index 76db6c67e39a..9cb00ebe9ac6 100644
--- a/kernel/cgroup/cgroup-v1.c
+++ b/kernel/cgroup/cgroup-v1.c
@@ -802,7 +802,7 @@ void cgroup1_release_agent(struct work_struct *work)
goto out_free;
ret = cgroup_path_ns(cgrp, pathbuf, PATH_MAX, &init_cgroup_ns);
- if (ret < 0 || ret >= PATH_MAX)
+ if (ret < 0)
goto out_free;
argv[0] = agentbuf;
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index 4b9ff41ca603..8d2674c6aaef 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -1893,7 +1893,7 @@ int cgroup_show_path(struct seq_file *sf, struct kernfs_node *kf_node,
len = kernfs_path_from_node(kf_node, ns_cgroup->kn, buf, PATH_MAX);
spin_unlock_irq(&css_set_lock);
- if (len >= PATH_MAX)
+ if (len == -E2BIG)
len = -ERANGE;
else if (len > 0) {
seq_escape(sf, buf, " \t\n\\");
@@ -6301,7 +6301,7 @@ int proc_cgroup_show(struct seq_file *m, struct pid_namespace *ns,
if (cgroup_on_dfl(cgrp) || !(tsk->flags & PF_EXITING)) {
retval = cgroup_path_ns_locked(cgrp, buf, PATH_MAX,
current->nsproxy->cgroup_ns);
- if (retval >= PATH_MAX)
+ if (retval == -E2BIG)
retval = -ENAMETOOLONG;
if (retval < 0)
goto out_unlock;
diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index 615daaf87f1f..fb29158ae825 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -4941,7 +4941,7 @@ int proc_cpuset_show(struct seq_file *m, struct pid_namespace *ns,
retval = cgroup_path_ns(css->cgroup, buf, PATH_MAX,
current->nsproxy->cgroup_ns);
css_put(css);
- if (retval >= PATH_MAX)
+ if (retval == -E2BIG)
retval = -ENAMETOOLONG;
if (retval < 0)
goto out_free;
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 3/3] kernfs: Convert kernfs_path_from_node_locked() from strlcpy() to strscpy()
2023-11-30 20:12 ` [PATCH v2 3/3] kernfs: Convert kernfs_path_from_node_locked() " Kees Cook
@ 2023-11-30 20:38 ` Christophe JAILLET
2023-11-30 21:02 ` Kees Cook
0 siblings, 1 reply; 6+ messages in thread
From: Christophe JAILLET @ 2023-11-30 20:38 UTC (permalink / raw)
To: Kees Cook, Greg Kroah-Hartman
Cc: Tejun Heo, Zefan Li, Johannes Weiner, Waiman Long, cgroups,
Azeem Shaikh, linux-kernel, bpf, linux-hardening
Le 30/11/2023 à 21:12, Kees Cook a écrit :
> One of the last remaining users of strlcpy() in the kernel is
> kernfs_path_from_node_locked(), which passes back the problematic "length
> we _would_ have copied" return value to indicate truncation. Convert the
> chain of all callers to use the negative return value (some of which
> already doing this explicitly). All callers were already also checking
> for negative return values, so the risk to missed checks looks very low.
>
> In this analysis, it was found that cgroup1_release_agent() actually
> didn't handle the "too large" condition, so this is technically also a
> bug fix. :)
>
> Here's the chain of callers, and resolution identifying each one as now
> handling the correct return value:
>
> kernfs_path_from_node_locked()
> kernfs_path_from_node()
> pr_cont_kernfs_path()
> returns void
> kernfs_path()
> sysfs_warn_dup()
> return value ignored
> cgroup_path()
> blkg_path()
> bfq_bic_update_cgroup()
> return value ignored
> TRACE_IOCG_PATH()
> return value ignored
> TRACE_CGROUP_PATH()
> return value ignored
> perf_event_cgroup()
> return value ignored
> task_group_path()
> return value ignored
> damon_sysfs_memcg_path_eq()
> return value ignored
> get_mm_memcg_path()
> return value ignored
> lru_gen_seq_show()
> return value ignored
> cgroup_path_from_kernfs_id()
> return value ignored
> cgroup_show_path()
> already converted "too large" error to negative value
> cgroup_path_ns_locked()
> cgroup_path_ns()
> bpf_iter_cgroup_show_fdinfo()
> return value ignored
> cgroup1_release_agent()
> wasn't checking "too large" error
> proc_cgroup_show()
> already converted "too large" to negative value
>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Tejun Heo <tj@kernel.org>
> Cc: Zefan Li <lizefan.x@bytedance.com>
> Cc: Johannes Weiner <hannes@cmpxchg.org>
> Cc: Waiman Long <longman@redhat.com>
> Cc: cgroups@vger.kernel.org
> Co-developed-by: Azeem Shaikh <azeemshaikh38@gmail.com>
> Signed-off-by: Azeem Shaikh <azeemshaikh38@gmail.com>
> Link: https://lore.kernel.org/r/20231116192127.1558276-3-keescook@chromium.org
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
> fs/kernfs/dir.c | 37 ++++++++++++++++++++-----------------
> kernel/cgroup/cgroup-v1.c | 2 +-
> kernel/cgroup/cgroup.c | 4 ++--
> kernel/cgroup/cpuset.c | 2 +-
> 4 files changed, 24 insertions(+), 21 deletions(-)
>
> diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
> index 8c0e5442597e..183f353b3852 100644
> --- a/fs/kernfs/dir.c
> +++ b/fs/kernfs/dir.c
> @@ -127,7 +127,7 @@ static struct kernfs_node *kernfs_common_ancestor(struct kernfs_node *a,
> *
> * [3] when @kn_to is %NULL result will be "(null)"
> *
> - * Return: the length of the full path. If the full length is equal to or
> + * Return: the length of the constructed path. If the path would have been
> * greater than @buflen, @buf contains the truncated path with the trailing
> * '\0'. On error, -errno is returned.
> */
> @@ -138,16 +138,17 @@ static int kernfs_path_from_node_locked(struct kernfs_node *kn_to,
> struct kernfs_node *kn, *common;
> const char parent_str[] = "/..";
> size_t depth_from, depth_to, len = 0;
> + ssize_t copied;
> 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))
> @@ -158,18 +159,22 @@ 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 < buflen ? buflen - len : 0);
> + for (i = 0; i < depth_from; i++) {
> + copied = strscpy(buf + len, parent_str, buflen - len);
> + if (copied < 0)
> + return copied;
> + len += copied;
> + }
>
> /* 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 < buflen ? buflen - len : 0);
> - len += strlcpy(buf + len, kn->name,
> - len < buflen ? buflen - len : 0);
> +
> + copied = scnprintf(buf + len, buflen - len, "/%s", kn->name);
> + if (copied < 0)
Can scnprintf() return <0 ?
> + return copied;
> + len += copied;
> }
...
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 3/3] kernfs: Convert kernfs_path_from_node_locked() from strlcpy() to strscpy()
2023-11-30 20:38 ` Christophe JAILLET
@ 2023-11-30 21:02 ` Kees Cook
0 siblings, 0 replies; 6+ messages in thread
From: Kees Cook @ 2023-11-30 21:02 UTC (permalink / raw)
To: Christophe JAILLET
Cc: Greg Kroah-Hartman, Tejun Heo, Zefan Li, Johannes Weiner,
Waiman Long, cgroups, Azeem Shaikh, linux-kernel, bpf,
linux-hardening
On Thu, Nov 30, 2023 at 09:38:11PM +0100, Christophe JAILLET wrote:
> Le 30/11/2023 à 21:12, Kees Cook a écrit :
> [...]
> > diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
> > index 8c0e5442597e..183f353b3852 100644
> > --- a/fs/kernfs/dir.c
> > +++ b/fs/kernfs/dir.c
> [...]
> > @@ -158,18 +159,22 @@ 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 < buflen ? buflen - len : 0);
> > + for (i = 0; i < depth_from; i++) {
> > + copied = strscpy(buf + len, parent_str, buflen - len);
> > + if (copied < 0)
> > + return copied;
> > + len += copied;
> > + }
> > /* 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 < buflen ? buflen - len : 0);
> > - len += strlcpy(buf + len, kn->name,
> > - len < buflen ? buflen - len : 0);
> > +
> > + copied = scnprintf(buf + len, buflen - len, "/%s", kn->name);
> > + if (copied < 0)
>
> Can scnprintf() return <0 ?
Ah, yeah, it's can't at all[1]. I forgot! :) Honestly, that function
should return size_t, not int...
I will send a v3 with this adjusted, but I'll wait for more review...
Thanks!
-Kees
[1] https://docs.kernel.org/core-api/kernel-api.html#c.scnprintf
--
Kees Cook
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-11-30 21:02 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-11-30 20:12 [PATCH v2 0/3] kernfs: Convert from strlcpy() to strscpy() Kees Cook
2023-11-30 20:12 ` [PATCH v2 1/3] kernfs: Convert kernfs_walk_ns() " Kees Cook
2023-11-30 20:12 ` [PATCH v2 2/3] kernfs: Convert kernfs_name_locked() " Kees Cook
2023-11-30 20:12 ` [PATCH v2 3/3] kernfs: Convert kernfs_path_from_node_locked() " Kees Cook
2023-11-30 20:38 ` Christophe JAILLET
2023-11-30 21:02 ` Kees Cook
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).