Message ID | 20210222151231.22572-2-romain.perier@gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | Manual replacement of all strlcpy in favor of strscpy | expand |
Hello. On Mon, Feb 22, 2021 at 04:12:12PM +0100, Romain Perier <romain.perier@gmail.com> wrote: > --- a/kernel/cgroup/cgroup.c > +++ b/kernel/cgroup/cgroup.c > @@ -2265,7 +2265,7 @@ int task_cgroup_path(struct task_struct *task, char *buf, size_t buflen) Actually, this function isn't used at all. So I'd instead propose the patch below. -- >8 -- From 4f7e0b9c0412f60e0b0e8b7d1ef6eb2790dca567 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Koutn=C3=BD?= <mkoutny@suse.com> Date: Tue, 23 Feb 2021 17:05:57 +0100 Subject: [PATCH] cgroup: Drop task_cgroup_path() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The function has no current users and it is a remnant from kdbus enthusiasm era 857a2beb09ab ("cgroup: implement task_cgroup_path_from_hierarchy()"). Drop it to eliminate unused code. Suggested-by: Romain Perier <romain.perier@gmail.com> Signed-off-by: Michal Koutný <mkoutny@suse.com> --- include/linux/cgroup.h | 1 - kernel/cgroup/cgroup.c | 39 --------------------------------------- 2 files changed, 40 deletions(-) diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h index 4f2f79de083e..e9c41b15fd4e 100644 --- a/include/linux/cgroup.h +++ b/include/linux/cgroup.h @@ -115,7 +115,6 @@ int cgroup_add_legacy_cftypes(struct cgroup_subsys *ss, struct cftype *cfts); int cgroup_rm_cftypes(struct cftype *cfts); void cgroup_file_notify(struct cgroup_file *cfile); -int task_cgroup_path(struct task_struct *task, char *buf, size_t buflen); int cgroupstats_build(struct cgroupstats *stats, struct dentry *dentry); int proc_cgroup_show(struct seq_file *m, struct pid_namespace *ns, struct pid *pid, struct task_struct *tsk); diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c index c80fe99f85ae..d75ffd461222 100644 --- a/kernel/cgroup/cgroup.c +++ b/kernel/cgroup/cgroup.c @@ -2235,45 +2235,6 @@ int cgroup_path_ns(struct cgroup *cgrp, char *buf, size_t buflen, } EXPORT_SYMBOL_GPL(cgroup_path_ns); -/** - * task_cgroup_path - cgroup path of a task in the first cgroup hierarchy - * @task: target task - * @buf: the buffer to write the path into - * @buflen: the length of the buffer - * - * Determine @task's cgroup on the first (the one with the lowest non-zero - * hierarchy_id) cgroup hierarchy and copy its path into @buf. This - * function grabs cgroup_mutex and shouldn't be used inside locks used by - * cgroup controller callbacks. - * - * Return value is the same as kernfs_path(). - */ -int task_cgroup_path(struct task_struct *task, char *buf, size_t buflen) -{ - struct cgroup_root *root; - struct cgroup *cgrp; - int hierarchy_id = 1; - int ret; - - mutex_lock(&cgroup_mutex); - spin_lock_irq(&css_set_lock); - - root = idr_get_next(&cgroup_hierarchy_idr, &hierarchy_id); - - if (root) { - cgrp = task_cgroup_from_root(task, root); - ret = cgroup_path_ns_locked(cgrp, buf, buflen, &init_cgroup_ns); - } else { - /* if no hierarchy exists, everyone is in "/" */ - ret = strlcpy(buf, "/", buflen); - } - - spin_unlock_irq(&css_set_lock); - mutex_unlock(&cgroup_mutex); - return ret; -} -EXPORT_SYMBOL_GPL(task_cgroup_path); - /** * cgroup_migrate_add_task - add a migration target task to a migration context * @task: target task
Le mar. 23 févr. 2021 à 17:13, Michal Koutný <mkoutny@suse.com> a écrit : > Hello. > > On Mon, Feb 22, 2021 at 04:12:12PM +0100, Romain Perier < > romain.perier@gmail.com> wrote: > > --- a/kernel/cgroup/cgroup.c > > +++ b/kernel/cgroup/cgroup.c > > @@ -2265,7 +2265,7 @@ int task_cgroup_path(struct task_struct *task, > char *buf, size_t buflen) > Actually, this function isn't used at all. So I'd instead propose the > patch below. > +1 if it is dead code. Thanks, Romain
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c index 1ea995f801ec..bac0dc2ff8ad 100644 --- a/kernel/cgroup/cgroup.c +++ b/kernel/cgroup/cgroup.c @@ -2265,7 +2265,7 @@ int task_cgroup_path(struct task_struct *task, char *buf, size_t buflen) ret = cgroup_path_ns_locked(cgrp, buf, buflen, &init_cgroup_ns); } else { /* if no hierarchy exists, everyone is in "/" */ - ret = strlcpy(buf, "/", buflen); + ret = strscpy(buf, "/", buflen); } spin_unlock_irq(&css_set_lock);
The strlcpy() reads the entire source buffer first, it is dangerous if the source buffer lenght is unbounded or possibility non NULL-terminated. It can lead to linear read overflows, crashes, etc... As recommended in the deprecated interfaces [1], it should be replaced by strscpy. This commit replaces all calls to strlcpy that handle the return values by the corresponding strscpy calls with new handling of the return values (as it is quite different between the two functions). [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy Signed-off-by: Romain Perier <romain.perier@gmail.com> --- kernel/cgroup/cgroup.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)