lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <ZYTkAV-CE3ZslR7U@mtj.duckdns.org> Date: Fri, 22 Dec 2023 10:18:57 +0900 From: Tejun Heo <tj@...nel.org> To: Kees Cook <keescook@...omium.org> Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Zefan Li <lizefan.x@...edance.com>, Johannes Weiner <hannes@...xchg.org>, Waiman Long <longman@...hat.com>, cgroups@...r.kernel.org, Azeem Shaikh <azeemshaikh38@...il.com>, Christophe JAILLET <christophe.jaillet@...adoo.fr>, linux-kernel@...r.kernel.org, bpf@...r.kernel.org, linux-hardening@...r.kernel.org Subject: Re: [PATCH v3 3/3] kernfs: Convert kernfs_path_from_node_locked() from strlcpy() to strscpy() Hello, On Tue, Dec 12, 2023 at 01:17:40PM -0800, Kees Cook wrote: ... > @@ -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. > */ ... > /* Calculate how many bytes we need for the rest */ We probably should drop this comment. > 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); > + > + len += scnprintf(buf + len, buflen - len, "/%s", kn->name); scnprintf doesn't return -E2BIG on overflow, right? It just returns the truncated length, so the overflow behavior would be different depending on where this function overflows, right? Not a huge problem but it may be better to keep calling strscpy to keep things consistent? > --- 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; I'd just pass up -E2BIG. > 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; Ditto. > 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) Ditto. Thanks. -- tejun
Powered by blists - more mailing lists