Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1458408 > unrolled thread

[PATCHSET] kernfs, cgroup: make kernfs_path*() and cgroup_path*() behave in strlcpy() style

Back to article view | Back to linux.kernel

Contents

  [PATCHSET] kernfs, cgroup: make kernfs_path*() and cgroup_path*() behave in strlcpy() style Tejun Heo <tj@kernel.org> - 2016-08-09 07:30 +0200
    [PATCH 1/4] kernfs: add dummy implementation of kernfs_path_from_node() Tejun Heo <tj@kernel.org> - 2016-08-09 07:30 +0200
      [PATCH v2 1/4] kernfs: add dummy implementation of  kernfs_path_from_node() Tejun Heo <tj@kernel.org> - 2016-08-09 08:20 +0200
    [PATCH 3/4] kernfs: remove kernfs_path_len() Tejun Heo <tj@kernel.org> - 2016-08-09 07:30 +0200
    [PATCH 4/4] cgroup: make cgroup_path() and friends behave in the style of strlcpy() Tejun Heo <tj@kernel.org> - 2016-08-09 07:30 +0200
      [PATCH v2 4/4] cgroup: make cgroup_path() and friends behave in the  style of strlcpy() Tejun Heo <tj@kernel.org> - 2016-08-09 08:20 +0200
    Re: [PATCHSET] kernfs, cgroup: make kernfs_path*() and  cgroup_path*() behave in strlcpy() style Greg KH <gregkh@linuxfoundation.org> - 2016-08-09 10:40 +0200
      Re: [PATCHSET] kernfs, cgroup: make kernfs_path*() and  cgroup_path*() behave in strlcpy() style Tejun Heo <tj@kernel.org> - 2016-08-10 23:30 +0200

#1458408 — [PATCHSET] kernfs, cgroup: make kernfs_path*() and cgroup_path*() behave in strlcpy() style

kernfs path formatting functions always return the length of full path
but the content of the output buffer is undefined when the length is
longer than the provided buffer.  Most cgroup path formatting
functions return the start of the output or NULL on errors including
overflow.  These inconsistent and rather peculiar behaviors developed
over time and make these functions unnecessarily difficult to use.
This patchset updates the formatting functions so that they all behave
in the style of strlcpy().

Greg, these changes are used by cgroup tracepoint additions and
shouldn't affect other users much.  Would it be okay to route these
through the cgroup tree?

 0001-kernfs-add-dummy-implementation-of-kernfs_path_from_.patch
 0002-kernfs-make-kernfs_path-behave-in-the-style-of-strlc.patch
 0003-kernfs-remove-kernfs_path_len.patch
 0004-cgroup-make-cgroup_path-and-friends-behave-in-the-st.patch

The patches are also available in the following git branch.

 git://git.kernel.org/pub/scm/linux/kernel/git/tj/cgroup.git review-kernfs_path-strlcpy

diffstat follows.  Thanks.

 fs/kernfs/dir.c            |   84 +++++++++------------------------------------
 fs/sysfs/dir.c             |    6 +--
 include/linux/blk-cgroup.h |   11 -----
 include/linux/cgroup.h     |    9 ++--
 include/linux/kernfs.h     |   28 ++++++++++-----
 kernel/cgroup.c            |   48 +++++++++++--------------
 kernel/cpuset.c            |   12 +++---
 7 files changed, 73 insertions(+), 125 deletions(-)

--
tejun

[toc] | [next] | [standalone]

#1458409 — [PATCH 1/4] kernfs: add dummy implementation of kernfs_path_from_node()

The dummy version of kernfs_path_from_node() was missing.  This
currently doesn't break anything.  Let's add it for consistency and to
ease adding wrappers around it.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 include/linux/kernfs.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/linux/kernfs.h b/include/linux/kernfs.h
index 96356ef..325954f 100644
--- a/include/linux/kernfs.h
+++ b/include/linux/kernfs.h
@@ -344,6 +344,11 @@ static inline int kernfs_name(struct kernfs_node *kn, char *buf, size_t buflen)
 static inline size_t kernfs_path_len(struct kernfs_node *kn)
 { return 0; }
 
+static inline int kernfs_path_from_node(struct kernfs_node *root_kn,
+					struct kernfs_node *kn,
+					char *buf, size_t buflen);
+{ return -ENOSYS; }
+
 static inline char *kernfs_path(struct kernfs_node *kn, char *buf,
 				size_t buflen)
 { return NULL; }
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1458426 — [PATCH v2 1/4] kernfs: add dummy implementation of kernfs_path_from_node()

From 17d6dfb3afa88253bf1ceee2d4a5e461970fc593 Mon Sep 17 00:00:00 2001
From: Tejun Heo <tj@kernel.org>
Date: Tue, 9 Aug 2016 02:12:21 -0400

The dummy version of kernfs_path_from_node() was missing.  This
currently doesn't break anything.  Let's add it for consistency and to
ease adding wrappers around it.

v2: Removed stray ';' which was causing build failures.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 include/linux/kernfs.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/linux/kernfs.h b/include/linux/kernfs.h
index 96356ef..7d2efd2 100644
--- a/include/linux/kernfs.h
+++ b/include/linux/kernfs.h
@@ -344,6 +344,11 @@ static inline int kernfs_name(struct kernfs_node *kn, char *buf, size_t buflen)
 static inline size_t kernfs_path_len(struct kernfs_node *kn)
 { return 0; }
 
+static inline int kernfs_path_from_node(struct kernfs_node *root_kn,
+					struct kernfs_node *kn,
+					char *buf, size_t buflen)
+{ return -ENOSYS; }
+
 static inline char *kernfs_path(struct kernfs_node *kn, char *buf,
 				size_t buflen)
 { return NULL; }
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1458411 — [PATCH 3/4] kernfs: remove kernfs_path_len()

It doesn't have any in-kernel user and the same result can be obtained
from kernfs_path(@kn, NULL, 0).  Remove it.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Serge Hallyn <serge.hallyn@ubuntu.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 fs/kernfs/dir.c        | 23 -----------------------
 include/linux/kernfs.h |  4 ----
 2 files changed, 27 deletions(-)

diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
index 09242aa..6e7fd37 100644
--- a/fs/kernfs/dir.c
+++ b/fs/kernfs/dir.c
@@ -180,29 +180,6 @@ int kernfs_name(struct kernfs_node *kn, char *buf, size_t buflen)
 }
 
 /**
- * kernfs_path_len - determine the length of the full path of a given node
- * @kn: kernfs_node of interest
- *
- * The returned length doesn't include the space for the terminating '\0'.
- */
-size_t kernfs_path_len(struct kernfs_node *kn)
-{
-	size_t len = 0;
-	unsigned long flags;
-
-	spin_lock_irqsave(&kernfs_rename_lock, flags);
-
-	do {
-		len += strlen(kn->name) + 1;
-		kn = kn->parent;
-	} while (kn && kn->parent);
-
-	spin_unlock_irqrestore(&kernfs_rename_lock, flags);
-
-	return len;
-}
-
-/**
  * kernfs_path_from_node - build path of node @to relative to @from.
  * @from: parent kernfs_node relative to which we need to build the path
  * @to: kernfs_node of interest
diff --git a/include/linux/kernfs.h b/include/linux/kernfs.h
index 64358d2..a410c75 100644
--- a/include/linux/kernfs.h
+++ b/include/linux/kernfs.h
@@ -269,7 +269,6 @@ static inline bool kernfs_ns_enabled(struct kernfs_node *kn)
 }
 
 int kernfs_name(struct kernfs_node *kn, char *buf, size_t buflen);
-size_t kernfs_path_len(struct kernfs_node *kn);
 int kernfs_path_from_node(struct kernfs_node *root_kn, struct kernfs_node *kn,
 			  char *buf, size_t buflen);
 void pr_cont_kernfs_name(struct kernfs_node *kn);
@@ -340,9 +339,6 @@ static inline bool kernfs_ns_enabled(struct kernfs_node *kn)
 static inline int kernfs_name(struct kernfs_node *kn, char *buf, size_t buflen)
 { return -ENOSYS; }
 
-static inline size_t kernfs_path_len(struct kernfs_node *kn)
-{ return 0; }
-
 static inline int kernfs_path_from_node(struct kernfs_node *root_kn,
 					struct kernfs_node *kn,
 					char *buf, size_t buflen);
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1458412 — [PATCH 4/4] cgroup: make cgroup_path() and friends behave in the style of strlcpy()

cgroup_path() and friends used to format the path from the end and
thus the resulting path usually didn't start at the start of the
passed in buffer.  Also, when the buffer was too small, the partial
result was truncated from the head rather than tail and there was no
way to tell how long the full path would be.  These make the functions
less robust and more awkward to use.

With recent updates to kernfs_path(), cgroup_path() and friends can be
made to behave in strlcpy() style.

* cgroup_path(), cgroup_path_ns[_locked]() and task_cgroup_path() now
  always return the length of the full path.  If buffer is too small,
  it contains nul terminated truncated output.

* All users updated accordingly.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Serge Hallyn <serge.hallyn@ubuntu.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 include/linux/blk-cgroup.h | 11 +----------
 include/linux/cgroup.h     | 16 +++++-----------
 kernel/cgroup.c            | 48 +++++++++++++++++++++-------------------------
 kernel/cpuset.c            | 12 ++++++------
 4 files changed, 34 insertions(+), 53 deletions(-)

diff --git a/include/linux/blk-cgroup.h b/include/linux/blk-cgroup.h
index 10648e3..4e8c215 100644
--- a/include/linux/blk-cgroup.h
+++ b/include/linux/blk-cgroup.h
@@ -343,16 +343,7 @@ static inline struct blkcg *cpd_to_blkcg(struct blkcg_policy_data *cpd)
  */
 static inline int blkg_path(struct blkcg_gq *blkg, char *buf, int buflen)
 {
-	char *p;
-
-	p = cgroup_path(blkg->blkcg->css.cgroup, buf, buflen);
-	if (!p) {
-		strncpy(buf, "<unavailable>", buflen);
-		return -ENAMETOOLONG;
-	}
-
-	memmove(buf, p, buf + buflen - p);
-	return 0;
+	return cgroup_path(blkg->blkcg->css.cgroup, buf, buflen);
 }
 
 /**
diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index 5a9abde..6df3636 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -97,7 +97,7 @@ 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);
 
-char *task_cgroup_path(struct task_struct *task, char *buf, size_t buflen);
+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);
@@ -538,15 +538,9 @@ static inline int cgroup_name(struct cgroup *cgrp, char *buf, size_t buflen)
 	return kernfs_name(cgrp->kn, buf, buflen);
 }
 
-static inline char * __must_check cgroup_path(struct cgroup *cgrp, char *buf,
-					      size_t buflen)
+static inline int cgroup_path(struct cgroup *cgrp, char *buf, size_t buflen)
 {
-	int ret;
-
-	ret = kernfs_path(cgrp->kn, buf, buflen);
-	if (ret < 0 || ret >= buflen)
-		return NULL;
-	return buf;
+	return kernfs_path(cgrp->kn, buf, buflen);
 }
 
 static inline void pr_cont_cgroup_name(struct cgroup *cgrp)
@@ -639,8 +633,8 @@ struct cgroup_namespace *copy_cgroup_ns(unsigned long flags,
 					struct user_namespace *user_ns,
 					struct cgroup_namespace *old_ns);
 
-char *cgroup_path_ns(struct cgroup *cgrp, char *buf, size_t buflen,
-		     struct cgroup_namespace *ns);
+int cgroup_path_ns(struct cgroup *cgrp, char *buf, size_t buflen,
+		   struct cgroup_namespace *ns);
 
 #else /* !CONFIG_CGROUPS */
 
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index d1c51b7..3861161 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -2315,22 +2315,18 @@ static struct file_system_type cgroup2_fs_type = {
 	.fs_flags = FS_USERNS_MOUNT,
 };
 
-static char *cgroup_path_ns_locked(struct cgroup *cgrp, char *buf, size_t buflen,
-				   struct cgroup_namespace *ns)
+static int cgroup_path_ns_locked(struct cgroup *cgrp, char *buf, size_t buflen,
+				 struct cgroup_namespace *ns)
 {
 	struct cgroup *root = cset_cgroup_from_root(ns->root_cset, cgrp->root);
-	int ret;
 
-	ret = kernfs_path_from_node(cgrp->kn, root->kn, buf, buflen);
-	if (ret < 0 || ret >= buflen)
-		return NULL;
-	return buf;
+	return kernfs_path_from_node(cgrp->kn, root->kn, buf, buflen);
 }
 
-char *cgroup_path_ns(struct cgroup *cgrp, char *buf, size_t buflen,
-		     struct cgroup_namespace *ns)
+int cgroup_path_ns(struct cgroup *cgrp, char *buf, size_t buflen,
+		   struct cgroup_namespace *ns)
 {
-	char *ret;
+	int ret;
 
 	mutex_lock(&cgroup_mutex);
 	spin_lock_irq(&css_set_lock);
@@ -2357,12 +2353,12 @@ EXPORT_SYMBOL_GPL(cgroup_path_ns);
  *
  * Return value is the same as kernfs_path().
  */
-char *task_cgroup_path(struct task_struct *task, char *buf, size_t buflen)
+int task_cgroup_path(struct task_struct *task, char *buf, size_t buflen)
 {
 	struct cgroup_root *root;
 	struct cgroup *cgrp;
 	int hierarchy_id = 1;
-	char *path = NULL;
+	int ret;
 
 	mutex_lock(&cgroup_mutex);
 	spin_lock_irq(&css_set_lock);
@@ -2371,16 +2367,15 @@ char *task_cgroup_path(struct task_struct *task, char *buf, size_t buflen)
 
 	if (root) {
 		cgrp = task_cgroup_from_root(task, root);
-		path = cgroup_path_ns_locked(cgrp, buf, buflen, &init_cgroup_ns);
+		ret = cgroup_path_ns_locked(cgrp, buf, buflen, &init_cgroup_ns);
 	} else {
 		/* if no hierarchy exists, everyone is in "/" */
-		if (strlcpy(buf, "/", buflen) < buflen)
-			path = buf;
+		ret = strlcpy(buf, "/", buflen);
 	}
 
 	spin_unlock_irq(&css_set_lock);
 	mutex_unlock(&cgroup_mutex);
-	return path;
+	return ret;
 }
 EXPORT_SYMBOL_GPL(task_cgroup_path);
 
@@ -5716,7 +5711,7 @@ core_initcall(cgroup_wq_init);
 int proc_cgroup_show(struct seq_file *m, struct pid_namespace *ns,
 		     struct pid *pid, struct task_struct *tsk)
 {
-	char *buf, *path;
+	char *buf;
 	int retval;
 	struct cgroup_root *root;
 
@@ -5759,18 +5754,18 @@ int proc_cgroup_show(struct seq_file *m, struct pid_namespace *ns,
 		 * " (deleted)" is appended to the cgroup path.
 		 */
 		if (cgroup_on_dfl(cgrp) || !(tsk->flags & PF_EXITING)) {
-			path = cgroup_path_ns_locked(cgrp, buf, PATH_MAX,
+			retval = cgroup_path_ns_locked(cgrp, buf, PATH_MAX,
 						current->nsproxy->cgroup_ns);
-			if (!path) {
+			if (retval >= PATH_MAX) {
 				retval = -ENAMETOOLONG;
 				goto out_unlock;
 			}
+
+			seq_puts(m, buf);
 		} else {
-			path = "/";
+			seq_puts(m, "/");
 		}
 
-		seq_puts(m, path);
-
 		if (cgroup_on_dfl(cgrp) && cgroup_is_dead(cgrp))
 			seq_puts(m, " (deleted)\n");
 		else
@@ -6035,8 +6030,9 @@ static void cgroup_release_agent(struct work_struct *work)
 {
 	struct cgroup *cgrp =
 		container_of(work, struct cgroup, release_agent_work);
-	char *pathbuf = NULL, *agentbuf = NULL, *path;
+	char *pathbuf = NULL, *agentbuf = NULL;
 	char *argv[3], *envp[3];
+	int ret;
 
 	mutex_lock(&cgroup_mutex);
 
@@ -6046,13 +6042,13 @@ static void cgroup_release_agent(struct work_struct *work)
 		goto out;
 
 	spin_lock_irq(&css_set_lock);
-	path = cgroup_path_ns_locked(cgrp, pathbuf, PATH_MAX, &init_cgroup_ns);
+	ret = cgroup_path_ns_locked(cgrp, pathbuf, PATH_MAX, &init_cgroup_ns);
 	spin_unlock_irq(&css_set_lock);
-	if (!path)
+	if (ret >= PATH_MAX)
 		goto out;
 
 	argv[0] = agentbuf;
-	argv[1] = path;
+	argv[1] = pathbuf;
 	argv[2] = NULL;
 
 	/* minimal command environment */
diff --git a/kernel/cpuset.c b/kernel/cpuset.c
index c7fd277..793ae6f 100644
--- a/kernel/cpuset.c
+++ b/kernel/cpuset.c
@@ -2689,7 +2689,7 @@ void __cpuset_memory_pressure_bump(void)
 int proc_cpuset_show(struct seq_file *m, struct pid_namespace *ns,
 		     struct pid *pid, struct task_struct *tsk)
 {
-	char *buf, *p;
+	char *buf;
 	struct cgroup_subsys_state *css;
 	int retval;
 
@@ -2700,18 +2700,18 @@ int proc_cpuset_show(struct seq_file *m, struct pid_namespace *ns,
 
 	retval = -ENAMETOOLONG;
 	css = task_get_css(tsk, cpuset_cgrp_id);
-	p = cgroup_path_ns(css->cgroup, buf, PATH_MAX,
-			   current->nsproxy->cgroup_ns);
+	retval = cgroup_path_ns(css->cgroup, buf, PATH_MAX,
+				current->nsproxy->cgroup_ns);
 	css_put(css);
-	if (!p)
+	if (retval >= PATH_MAX)
 		goto out_free;
-	seq_puts(m, p);
+	seq_puts(m, buf);
 	seq_putc(m, '\n');
 	retval = 0;
 out_free:
 	kfree(buf);
 out:
-	return retval;
+	return 0;
 }
 #endif /* CONFIG_PROC_PID_CPUSET */
 
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1458427 — [PATCH v2 4/4] cgroup: make cgroup_path() and friends behave in the style of strlcpy()

From a374fd0487f8e51c0879acbdec35c8c4e3a9af7b Mon Sep 17 00:00:00 2001
From: Tejun Heo <tj@kernel.org>
Date: Tue, 9 Aug 2016 02:12:22 -0400

cgroup_path() and friends used to format the path from the end and
thus the resulting path usually didn't start at the start of the
passed in buffer.  Also, when the buffer was too small, the partial
result was truncated from the head rather than tail and there was no
way to tell how long the full path would be.  These make the functions
less robust and more awkward to use.

With recent updates to kernfs_path(), cgroup_path() and friends can be
made to behave in strlcpy() style.

* cgroup_path(), cgroup_path_ns[_locked]() and task_cgroup_path() now
  always return the length of the full path.  If buffer is too small,
  it contains nul terminated truncated output.

* All users updated accordingly.

v2: cgroup_path() usage in kernel/sched/debug.c converted.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Serge Hallyn <serge.hallyn@ubuntu.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
---
 include/linux/blk-cgroup.h | 11 +----------
 include/linux/cgroup.h     | 16 +++++-----------
 kernel/cgroup.c            | 48 +++++++++++++++++++++-------------------------
 kernel/cpuset.c            | 12 ++++++------
 kernel/sched/debug.c       |  3 ++-
 5 files changed, 36 insertions(+), 54 deletions(-)

diff --git a/include/linux/blk-cgroup.h b/include/linux/blk-cgroup.h
index 10648e3..4e8c215 100644
--- a/include/linux/blk-cgroup.h
+++ b/include/linux/blk-cgroup.h
@@ -343,16 +343,7 @@ static inline struct blkcg *cpd_to_blkcg(struct blkcg_policy_data *cpd)
  */
 static inline int blkg_path(struct blkcg_gq *blkg, char *buf, int buflen)
 {
-	char *p;
-
-	p = cgroup_path(blkg->blkcg->css.cgroup, buf, buflen);
-	if (!p) {
-		strncpy(buf, "<unavailable>", buflen);
-		return -ENAMETOOLONG;
-	}
-
-	memmove(buf, p, buf + buflen - p);
-	return 0;
+	return cgroup_path(blkg->blkcg->css.cgroup, buf, buflen);
 }
 
 /**
diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index 5a9abde..6df3636 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -97,7 +97,7 @@ 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);
 
-char *task_cgroup_path(struct task_struct *task, char *buf, size_t buflen);
+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);
@@ -538,15 +538,9 @@ static inline int cgroup_name(struct cgroup *cgrp, char *buf, size_t buflen)
 	return kernfs_name(cgrp->kn, buf, buflen);
 }
 
-static inline char * __must_check cgroup_path(struct cgroup *cgrp, char *buf,
-					      size_t buflen)
+static inline int cgroup_path(struct cgroup *cgrp, char *buf, size_t buflen)
 {
-	int ret;
-
-	ret = kernfs_path(cgrp->kn, buf, buflen);
-	if (ret < 0 || ret >= buflen)
-		return NULL;
-	return buf;
+	return kernfs_path(cgrp->kn, buf, buflen);
 }
 
 static inline void pr_cont_cgroup_name(struct cgroup *cgrp)
@@ -639,8 +633,8 @@ struct cgroup_namespace *copy_cgroup_ns(unsigned long flags,
 					struct user_namespace *user_ns,
 					struct cgroup_namespace *old_ns);
 
-char *cgroup_path_ns(struct cgroup *cgrp, char *buf, size_t buflen,
-		     struct cgroup_namespace *ns);
+int cgroup_path_ns(struct cgroup *cgrp, char *buf, size_t buflen,
+		   struct cgroup_namespace *ns);
 
 #else /* !CONFIG_CGROUPS */
 
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index d1c51b7..3861161 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -2315,22 +2315,18 @@ static struct file_system_type cgroup2_fs_type = {
 	.fs_flags = FS_USERNS_MOUNT,
 };
 
-static char *cgroup_path_ns_locked(struct cgroup *cgrp, char *buf, size_t buflen,
-				   struct cgroup_namespace *ns)
+static int cgroup_path_ns_locked(struct cgroup *cgrp, char *buf, size_t buflen,
+				 struct cgroup_namespace *ns)
 {
 	struct cgroup *root = cset_cgroup_from_root(ns->root_cset, cgrp->root);
-	int ret;
 
-	ret = kernfs_path_from_node(cgrp->kn, root->kn, buf, buflen);
-	if (ret < 0 || ret >= buflen)
-		return NULL;
-	return buf;
+	return kernfs_path_from_node(cgrp->kn, root->kn, buf, buflen);
 }
 
-char *cgroup_path_ns(struct cgroup *cgrp, char *buf, size_t buflen,
-		     struct cgroup_namespace *ns)
+int cgroup_path_ns(struct cgroup *cgrp, char *buf, size_t buflen,
+		   struct cgroup_namespace *ns)
 {
-	char *ret;
+	int ret;
 
 	mutex_lock(&cgroup_mutex);
 	spin_lock_irq(&css_set_lock);
@@ -2357,12 +2353,12 @@ EXPORT_SYMBOL_GPL(cgroup_path_ns);
  *
  * Return value is the same as kernfs_path().
  */
-char *task_cgroup_path(struct task_struct *task, char *buf, size_t buflen)
+int task_cgroup_path(struct task_struct *task, char *buf, size_t buflen)
 {
 	struct cgroup_root *root;
 	struct cgroup *cgrp;
 	int hierarchy_id = 1;
-	char *path = NULL;
+	int ret;
 
 	mutex_lock(&cgroup_mutex);
 	spin_lock_irq(&css_set_lock);
@@ -2371,16 +2367,15 @@ char *task_cgroup_path(struct task_struct *task, char *buf, size_t buflen)
 
 	if (root) {
 		cgrp = task_cgroup_from_root(task, root);
-		path = cgroup_path_ns_locked(cgrp, buf, buflen, &init_cgroup_ns);
+		ret = cgroup_path_ns_locked(cgrp, buf, buflen, &init_cgroup_ns);
 	} else {
 		/* if no hierarchy exists, everyone is in "/" */
-		if (strlcpy(buf, "/", buflen) < buflen)
-			path = buf;
+		ret = strlcpy(buf, "/", buflen);
 	}
 
 	spin_unlock_irq(&css_set_lock);
 	mutex_unlock(&cgroup_mutex);
-	return path;
+	return ret;
 }
 EXPORT_SYMBOL_GPL(task_cgroup_path);
 
@@ -5716,7 +5711,7 @@ core_initcall(cgroup_wq_init);
 int proc_cgroup_show(struct seq_file *m, struct pid_namespace *ns,
 		     struct pid *pid, struct task_struct *tsk)
 {
-	char *buf, *path;
+	char *buf;
 	int retval;
 	struct cgroup_root *root;
 
@@ -5759,18 +5754,18 @@ int proc_cgroup_show(struct seq_file *m, struct pid_namespace *ns,
 		 * " (deleted)" is appended to the cgroup path.
 		 */
 		if (cgroup_on_dfl(cgrp) || !(tsk->flags & PF_EXITING)) {
-			path = cgroup_path_ns_locked(cgrp, buf, PATH_MAX,
+			retval = cgroup_path_ns_locked(cgrp, buf, PATH_MAX,
 						current->nsproxy->cgroup_ns);
-			if (!path) {
+			if (retval >= PATH_MAX) {
 				retval = -ENAMETOOLONG;
 				goto out_unlock;
 			}
+
+			seq_puts(m, buf);
 		} else {
-			path = "/";
+			seq_puts(m, "/");
 		}
 
-		seq_puts(m, path);
-
 		if (cgroup_on_dfl(cgrp) && cgroup_is_dead(cgrp))
 			seq_puts(m, " (deleted)\n");
 		else
@@ -6035,8 +6030,9 @@ static void cgroup_release_agent(struct work_struct *work)
 {
 	struct cgroup *cgrp =
 		container_of(work, struct cgroup, release_agent_work);
-	char *pathbuf = NULL, *agentbuf = NULL, *path;
+	char *pathbuf = NULL, *agentbuf = NULL;
 	char *argv[3], *envp[3];
+	int ret;
 
 	mutex_lock(&cgroup_mutex);
 
@@ -6046,13 +6042,13 @@ static void cgroup_release_agent(struct work_struct *work)
 		goto out;
 
 	spin_lock_irq(&css_set_lock);
-	path = cgroup_path_ns_locked(cgrp, pathbuf, PATH_MAX, &init_cgroup_ns);
+	ret = cgroup_path_ns_locked(cgrp, pathbuf, PATH_MAX, &init_cgroup_ns);
 	spin_unlock_irq(&css_set_lock);
-	if (!path)
+	if (ret >= PATH_MAX)
 		goto out;
 
 	argv[0] = agentbuf;
-	argv[1] = path;
+	argv[1] = pathbuf;
 	argv[2] = NULL;
 
 	/* minimal command environment */
diff --git a/kernel/cpuset.c b/kernel/cpuset.c
index c7fd277..793ae6f 100644
--- a/kernel/cpuset.c
+++ b/kernel/cpuset.c
@@ -2689,7 +2689,7 @@ void __cpuset_memory_pressure_bump(void)
 int proc_cpuset_show(struct seq_file *m, struct pid_namespace *ns,
 		     struct pid *pid, struct task_struct *tsk)
 {
-	char *buf, *p;
+	char *buf;
 	struct cgroup_subsys_state *css;
 	int retval;
 
@@ -2700,18 +2700,18 @@ int proc_cpuset_show(struct seq_file *m, struct pid_namespace *ns,
 
 	retval = -ENAMETOOLONG;
 	css = task_get_css(tsk, cpuset_cgrp_id);
-	p = cgroup_path_ns(css->cgroup, buf, PATH_MAX,
-			   current->nsproxy->cgroup_ns);
+	retval = cgroup_path_ns(css->cgroup, buf, PATH_MAX,
+				current->nsproxy->cgroup_ns);
 	css_put(css);
-	if (!p)
+	if (retval >= PATH_MAX)
 		goto out_free;
-	seq_puts(m, p);
+	seq_puts(m, buf);
 	seq_putc(m, '\n');
 	retval = 0;
 out_free:
 	kfree(buf);
 out:
-	return retval;
+	return 0;
 }
 #endif /* CONFIG_PROC_PID_CPUSET */
 
diff --git a/kernel/sched/debug.c b/kernel/sched/debug.c
index 2a0a999..23cb609 100644
--- a/kernel/sched/debug.c
+++ b/kernel/sched/debug.c
@@ -410,7 +410,8 @@ static char *task_group_path(struct task_group *tg)
 	if (autogroup_path(tg, group_path, PATH_MAX))
 		return group_path;
 
-	return cgroup_path(tg->css.cgroup, group_path, PATH_MAX);
+	cgroup_path(tg->css.cgroup, group_path, PATH_MAX);
+	return group_path;
 }
 #endif
 
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1458513 — Re: [PATCHSET] kernfs, cgroup: make kernfs_path*() and cgroup_path*() behave in strlcpy() style

On Tue, Aug 09, 2016 at 01:23:20AM -0400, Tejun Heo wrote:
> kernfs path formatting functions always return the length of full path
> but the content of the output buffer is undefined when the length is
> longer than the provided buffer.  Most cgroup path formatting
> functions return the start of the output or NULL on errors including
> overflow.  These inconsistent and rather peculiar behaviors developed
> over time and make these functions unnecessarily difficult to use.
> This patchset updates the formatting functions so that they all behave
> in the style of strlcpy().
> 
> Greg, these changes are used by cgroup tracepoint additions and
> shouldn't affect other users much.  Would it be okay to route these
> through the cgroup tree?
> 
>  0001-kernfs-add-dummy-implementation-of-kernfs_path_from_.patch
>  0002-kernfs-make-kernfs_path-behave-in-the-style-of-strlc.patch
>  0003-kernfs-remove-kernfs_path_len.patch
>  0004-cgroup-make-cgroup_path-and-friends-behave-in-the-st.patch
> 
> The patches are also available in the following git branch.
> 
>  git://git.kernel.org/pub/scm/linux/kernel/git/tj/cgroup.git review-kernfs_path-strlcpy
> 
> diffstat follows.  Thanks.

It's fine with me if you take these yourself:

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

thanks,

greg k-h

[toc] | [prev] | [next] | [standalone]

#1459990 — Re: [PATCHSET] kernfs, cgroup: make kernfs_path*() and cgroup_path*() behave in strlcpy() style

On Tue, Aug 09, 2016 at 10:18:19AM +0200, Greg KH wrote:
> On Tue, Aug 09, 2016 at 01:23:20AM -0400, Tejun Heo wrote:
> > kernfs path formatting functions always return the length of full path
> > but the content of the output buffer is undefined when the length is
> > longer than the provided buffer.  Most cgroup path formatting
> > functions return the start of the output or NULL on errors including
> > overflow.  These inconsistent and rather peculiar behaviors developed
> > over time and make these functions unnecessarily difficult to use.
> > This patchset updates the formatting functions so that they all behave
> > in the style of strlcpy().
> > 
> > Greg, these changes are used by cgroup tracepoint additions and
> > shouldn't affect other users much.  Would it be okay to route these
> > through the cgroup tree?
> > 
> >  0001-kernfs-add-dummy-implementation-of-kernfs_path_from_.patch
> >  0002-kernfs-make-kernfs_path-behave-in-the-style-of-strlc.patch
> >  0003-kernfs-remove-kernfs_path_len.patch
> >  0004-cgroup-make-cgroup_path-and-friends-behave-in-the-st.patch
> > 
> > The patches are also available in the following git branch.
> > 
> >  git://git.kernel.org/pub/scm/linux/kernel/git/tj/cgroup.git review-kernfs_path-strlcpy
> > 
> > diffstat follows.  Thanks.
> 
> It's fine with me if you take these yourself:
> 
> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Applied 1-4 to cgroup/for-4.9.

Thanks.

-- 
tejun

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web