tjh.dev / kernel
Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

kernfs: Convert kernfs_path_from_node_locked() from strlcpy() to strscpy()

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>
Link: https://lore.kernel.org/r/20231212211741.164376-3-keescook@chromium.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

authored by

Kees Cook and committed by
Greg Kroah-Hartman ff6d413b 5b56bf5c

+21 -21
+17 -17
fs/kernfs/dir.c
··· 127 127 * 128 128 * [3] when @kn_to is %NULL result will be "(null)" 129 129 * 130 - * Return: the length of the full path. If the full length is equal to or 130 + * Return: the length of the constructed path. If the path would have been 131 131 * greater than @buflen, @buf contains the truncated path with the trailing 132 132 * '\0'. On error, -errno is returned. 133 133 */ ··· 138 138 struct kernfs_node *kn, *common; 139 139 const char parent_str[] = "/.."; 140 140 size_t depth_from, depth_to, len = 0; 141 + ssize_t copied; 141 142 int i, j; 142 143 143 144 if (!kn_to) 144 - return strlcpy(buf, "(null)", buflen); 145 + return strscpy(buf, "(null)", buflen); 145 146 146 147 if (!kn_from) 147 148 kn_from = kernfs_root(kn_to)->kn; 148 149 149 150 if (kn_from == kn_to) 150 - return strlcpy(buf, "/", buflen); 151 + return strscpy(buf, "/", buflen); 151 152 152 153 common = kernfs_common_ancestor(kn_from, kn_to); 153 154 if (WARN_ON(!common)) ··· 159 158 160 159 buf[0] = '\0'; 161 160 162 - for (i = 0; i < depth_from; i++) 163 - len += strlcpy(buf + len, parent_str, 164 - len < buflen ? buflen - len : 0); 161 + for (i = 0; i < depth_from; i++) { 162 + copied = strscpy(buf + len, parent_str, buflen - len); 163 + if (copied < 0) 164 + return copied; 165 + len += copied; 166 + } 165 167 166 168 /* Calculate how many bytes we need for the rest */ 167 169 for (i = depth_to - 1; i >= 0; i--) { 168 170 for (kn = kn_to, j = 0; j < i; j++) 169 171 kn = kn->parent; 170 - len += strlcpy(buf + len, "/", 171 - len < buflen ? buflen - len : 0); 172 - len += strlcpy(buf + len, kn->name, 173 - len < buflen ? buflen - len : 0); 172 + 173 + len += scnprintf(buf + len, buflen - len, "/%s", kn->name); 174 174 } 175 175 176 176 return len; ··· 216 214 * path (which includes '..'s) as needed to reach from @from to @to is 217 215 * returned. 218 216 * 219 - * Return: the length of the full path. If the full length is equal to or 217 + * Return: the length of the constructed path. If the path would have been 220 218 * greater than @buflen, @buf contains the truncated path with the trailing 221 219 * '\0'. On error, -errno is returned. 222 220 */ ··· 267 265 sz = kernfs_path_from_node(kn, NULL, kernfs_pr_cont_buf, 268 266 sizeof(kernfs_pr_cont_buf)); 269 267 if (sz < 0) { 270 - pr_cont("(error)"); 271 - goto out; 272 - } 273 - 274 - if (sz >= sizeof(kernfs_pr_cont_buf)) { 275 - pr_cont("(name too long)"); 268 + if (sz == -E2BIG) 269 + pr_cont("(name too long)"); 270 + else 271 + pr_cont("(error)"); 276 272 goto out; 277 273 } 278 274
+1 -1
kernel/cgroup/cgroup-v1.c
··· 802 802 goto out_free; 803 803 804 804 ret = cgroup_path_ns(cgrp, pathbuf, PATH_MAX, &init_cgroup_ns); 805 - if (ret < 0 || ret >= PATH_MAX) 805 + if (ret < 0) 806 806 goto out_free; 807 807 808 808 argv[0] = agentbuf;
+2 -2
kernel/cgroup/cgroup.c
··· 1893 1893 len = kernfs_path_from_node(kf_node, ns_cgroup->kn, buf, PATH_MAX); 1894 1894 spin_unlock_irq(&css_set_lock); 1895 1895 1896 - if (len >= PATH_MAX) 1896 + if (len == -E2BIG) 1897 1897 len = -ERANGE; 1898 1898 else if (len > 0) { 1899 1899 seq_escape(sf, buf, " \t\n\\"); ··· 6278 6278 if (cgroup_on_dfl(cgrp) || !(tsk->flags & PF_EXITING)) { 6279 6279 retval = cgroup_path_ns_locked(cgrp, buf, PATH_MAX, 6280 6280 current->nsproxy->cgroup_ns); 6281 - if (retval >= PATH_MAX) 6281 + if (retval == -E2BIG) 6282 6282 retval = -ENAMETOOLONG; 6283 6283 if (retval < 0) 6284 6284 goto out_unlock;
+1 -1
kernel/cgroup/cpuset.c
··· 4941 4941 retval = cgroup_path_ns(css->cgroup, buf, PATH_MAX, 4942 4942 current->nsproxy->cgroup_ns); 4943 4943 css_put(css); 4944 - if (retval >= PATH_MAX) 4944 + if (retval == -E2BIG) 4945 4945 retval = -ENAMETOOLONG; 4946 4946 if (retval < 0) 4947 4947 goto out_free;