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

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

[Tejun Heo]

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

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

[Tejun Heo]

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

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

[Tejun Heo]

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

[PATCH 2/4] kernfs: make kernfs_path*() behave in the style of strlcpy()

[Tejun Heo]

kernfs_path*() functions always return the length of the full path but
the path content is undefined if the length is larger than the
provided buffer.  This makes its behavior different from strlcpy() and
requires error handling in all its users even when they don't care
about truncation.  In addition, the implementation can actully be
simplified by making it behave properly in strlcpy() style.

* Update kernfs_path_from_node_locked() to always fill up the buffer
  with path.  If the buffer is not large enough, the output is
  truncated and terminated.

* kernfs_path() no longer needs error handling.  Make it a simple
  inline wrapper around kernfs_path_from_node().

* sysfs_warn_dup()'s use of kernfs_path() doesn't need error handling.
  Updated accordingly.

* cgroup_path()'s use of kernfs_path() updated to retain the old
  behavior.

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        | 61 ++++++++++++++------------------------------------
 fs/sysfs/dir.c         |  6 ++---
 include/linux/cgroup.h |  7 +++++-
 include/linux/kernfs.h | 21 ++++++++++++-----
 4 files changed, 42 insertions(+), 53 deletions(-)

diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
index e57174d..09242aa 100644
--- a/fs/kernfs/dir.c
+++ b/fs/kernfs/dir.c
@@ -110,8 +110,9 @@ static struct kernfs_node *kernfs_common_ancestor(struct kernfs_node *a,
  * kn_to:   /n1/n2/n3         [depth=3]
  * result:  /../..
  *
- * return value: length of the string.  If greater than buflen,
- * then contents of buf are undefined.  On error, -1 is returned.
+ * Returns the length of the full path.  If the full length is equal to or
+ * greater than @buflen, @buf contains the truncated path with the trailing
+ * '\0'.  On error, -errno is returned.
  */
 static int kernfs_path_from_node_locked(struct kernfs_node *kn_to,
 					struct kernfs_node *kn_from,
@@ -119,9 +120,8 @@ 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, nlen = 0;
-	char *p;
-	int i;
+	size_t depth_from, depth_to, len = 0;
+	int i, j;
 
 	if (!kn_from)
 		kn_from = kernfs_root(kn_to)->kn;
@@ -131,7 +131,7 @@ static int kernfs_path_from_node_locked(struct kernfs_node *kn_to,
 
 	common = kernfs_common_ancestor(kn_from, kn_to);
 	if (WARN_ON(!common))
-		return -1;
+		return -EINVAL;
 
 	depth_to = kernfs_depth(common, kn_to);
 	depth_from = kernfs_depth(common, kn_from);
@@ -144,22 +144,16 @@ static int kernfs_path_from_node_locked(struct kernfs_node *kn_to,
 			       len < buflen ? buflen - len : 0);
 
 	/* Calculate how many bytes we need for the rest */
-	for (kn = kn_to; kn != common; kn = kn->parent)
-		nlen += strlen(kn->name) + 1;
-
-	if (len + nlen >= buflen)
-		return len + nlen;
-
-	p = buf + len + nlen;
-	*p = '\0';
-	for (kn = kn_to; kn != common; kn = kn->parent) {
-		size_t tmp = strlen(kn->name);
-		p -= tmp;
-		memcpy(p, kn->name, tmp);
-		*(--p) = '/';
+	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);
 	}
 
-	return len + nlen;
+	return len;
 }
 
 /**
@@ -220,8 +214,9 @@ size_t kernfs_path_len(struct kernfs_node *kn)
  * path (which includes '..'s) as needed to reach from @from to @to is
  * returned.
  *
- * If @buf isn't long enough, the return value will be greater than @buflen
- * and @buf contents are undefined.
+ * Returns the length of the full path.  If the full length is equal to or
+ * greater than @buflen, @buf contains the truncated path with the trailing
+ * '\0'.  On error, -errno is returned.
  */
 int kernfs_path_from_node(struct kernfs_node *to, struct kernfs_node *from,
 			  char *buf, size_t buflen)
@@ -237,28 +232,6 @@ int kernfs_path_from_node(struct kernfs_node *to, struct kernfs_node *from,
 EXPORT_SYMBOL_GPL(kernfs_path_from_node);
 
 /**
- * kernfs_path - build full path of a given node
- * @kn: kernfs_node of interest
- * @buf: buffer to copy @kn's name into
- * @buflen: size of @buf
- *
- * Builds and returns the full path of @kn in @buf of @buflen bytes.  The
- * path is built from the end of @buf so the returned pointer usually
- * doesn't match @buf.  If @buf isn't long enough, @buf is nul terminated
- * and %NULL is returned.
- */
-char *kernfs_path(struct kernfs_node *kn, char *buf, size_t buflen)
-{
-	int ret;
-
-	ret = kernfs_path_from_node(kn, NULL, buf, buflen);
-	if (ret < 0 || ret >= buflen)
-		return NULL;
-	return buf;
-}
-EXPORT_SYMBOL_GPL(kernfs_path);
-
-/**
  * pr_cont_kernfs_name - pr_cont name of a kernfs_node
  * @kn: kernfs_node of interest
  *
diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c
index 94374e4..2b67bda 100644
--- a/fs/sysfs/dir.c
+++ b/fs/sysfs/dir.c
@@ -21,14 +21,14 @@ DEFINE_SPINLOCK(sysfs_symlink_target_lock);
 
 void sysfs_warn_dup(struct kernfs_node *parent, const char *name)
 {
-	char *buf, *path = NULL;
+	char *buf;
 
 	buf = kzalloc(PATH_MAX, GFP_KERNEL);
 	if (buf)
-		path = kernfs_path(parent, buf, PATH_MAX);
+		kernfs_path(parent, buf, PATH_MAX);
 
 	WARN(1, KERN_WARNING "sysfs: cannot create duplicate filename '%s/%s'\n",
-	     path, name);
+	     buf, name);
 
 	kfree(buf);
 }
diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index 984f73b..5a9abde 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -541,7 +541,12 @@ static inline int cgroup_name(struct cgroup *cgrp, char *buf, size_t buflen)
 static inline char * __must_check cgroup_path(struct cgroup *cgrp, char *buf,
 					      size_t buflen)
 {
-	return kernfs_path(cgrp->kn, buf, buflen);
+	int ret;
+
+	ret = kernfs_path(cgrp->kn, buf, buflen);
+	if (ret < 0 || ret >= buflen)
+		return NULL;
+	return buf;
 }
 
 static inline void pr_cont_cgroup_name(struct cgroup *cgrp)
diff --git a/include/linux/kernfs.h b/include/linux/kernfs.h
index 325954f..64358d2 100644
--- a/include/linux/kernfs.h
+++ b/include/linux/kernfs.h
@@ -272,7 +272,6 @@ 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);
-char *kernfs_path(struct kernfs_node *kn, char *buf, size_t buflen);
 void pr_cont_kernfs_name(struct kernfs_node *kn);
 void pr_cont_kernfs_path(struct kernfs_node *kn);
 struct kernfs_node *kernfs_get_parent(struct kernfs_node *kn);
@@ -349,10 +348,6 @@ static inline int kernfs_path_from_node(struct kernfs_node *root_kn,
 					char *buf, size_t buflen);
 { return -ENOSYS; }
 
-static inline char *kernfs_path(struct kernfs_node *kn, char *buf,
-				size_t buflen)
-{ return NULL; }
-
 static inline void pr_cont_kernfs_name(struct kernfs_node *kn) { }
 static inline void pr_cont_kernfs_path(struct kernfs_node *kn) { }
 
@@ -441,6 +436,22 @@ static inline void kernfs_init(void) { }
 
 #endif	/* CONFIG_KERNFS */
 
+/**
+ * kernfs_path - build full path of a given node
+ * @kn: kernfs_node of interest
+ * @buf: buffer to copy @kn's name into
+ * @buflen: size of @buf
+ *
+ * Builds and returns the full path of @kn in @buf of @buflen bytes.  The
+ * path is built from the end of @buf so the returned pointer usually
+ * doesn't match @buf.  If @buf isn't long enough, @buf is nul terminated
+ * and %NULL is returned.
+ */
+static inline int kernfs_path(struct kernfs_node *kn, char *buf, size_t buflen)
+{
+	return kernfs_path_from_node(kn, NULL, buf, buflen);
+}
+
 static inline struct kernfs_node *
 kernfs_find_and_get(struct kernfs_node *kn, const char *name)
 {
-- 
2.7.4

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

[Tejun Heo]

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

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

[Tejun Heo]

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

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

[Tejun Heo]

>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

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

[Tejun Heo]

From 17d6dfb3afa88253bf1ceee2d4a5e461970fc593 Mon Sep 17 00:00:00 2001
From: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.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-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Greg Kroah-Hartman <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.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

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

[Tejun Heo]

>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

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

[Tejun Heo]

From a374fd0487f8e51c0879acbdec35c8c4e3a9af7b Mon Sep 17 00:00:00 2001
From: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.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-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Serge Hallyn <serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA@public.gmane.org>
Cc: Greg Kroah-Hartman <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>
Cc: Peter Zijlstra <peterz-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.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

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

[Greg KH]

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

Re: [PATCH 2/4] kernfs: make kernfs_path*() behave in the style of strlcpy()

[Serge E. Hallyn]

Quoting Tejun Heo (tj@kernel.org):
> kernfs_path*() functions always return the length of the full path but
> the path content is undefined if the length is larger than the
> provided buffer.  This makes its behavior different from strlcpy() and
> requires error handling in all its users even when they don't care
> about truncation.  In addition, the implementation can actully be
> simplified by making it behave properly in strlcpy() style.
> 
> * Update kernfs_path_from_node_locked() to always fill up the buffer
>   with path.  If the buffer is not large enough, the output is
>   truncated and terminated.
> 
> * kernfs_path() no longer needs error handling.  Make it a simple
>   inline wrapper around kernfs_path_from_node().
> 
> * sysfs_warn_dup()'s use of kernfs_path() doesn't need error handling.
>   Updated accordingly.
> 
> * cgroup_path()'s use of kernfs_path() updated to retain the old
>   behavior.
> 
> 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        | 61 ++++++++++++++------------------------------------
>  fs/sysfs/dir.c         |  6 ++---
>  include/linux/cgroup.h |  7 +++++-
>  include/linux/kernfs.h | 21 ++++++++++++-----
>  4 files changed, 42 insertions(+), 53 deletions(-)
> 
> diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
> index e57174d..09242aa 100644
> --- a/fs/kernfs/dir.c
> +++ b/fs/kernfs/dir.c
> @@ -110,8 +110,9 @@ static struct kernfs_node *kernfs_common_ancestor(struct kernfs_node *a,
>   * kn_to:   /n1/n2/n3         [depth=3]
>   * result:  /../..
>   *
> - * return value: length of the string.  If greater than buflen,
> - * then contents of buf are undefined.  On error, -1 is returned.
> + * Returns the length of the full path.  If the full length is equal to or
> + * greater than @buflen, @buf contains the truncated path with the trailing
> + * '\0'.  On error, -errno is returned.
>   */
>  static int kernfs_path_from_node_locked(struct kernfs_node *kn_to,
>  					struct kernfs_node *kn_from,
> @@ -119,9 +120,8 @@ 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, nlen = 0;
> -	char *p;
> -	int i;
> +	size_t depth_from, depth_to, len = 0;
> +	int i, j;
>  
>  	if (!kn_from)
>  		kn_from = kernfs_root(kn_to)->kn;
> @@ -131,7 +131,7 @@ static int kernfs_path_from_node_locked(struct kernfs_node *kn_to,
>  
>  	common = kernfs_common_ancestor(kn_from, kn_to);
>  	if (WARN_ON(!common))
> -		return -1;
> +		return -EINVAL;
>  
>  	depth_to = kernfs_depth(common, kn_to);
>  	depth_from = kernfs_depth(common, kn_from);
> @@ -144,22 +144,16 @@ static int kernfs_path_from_node_locked(struct kernfs_node *kn_to,
>  			       len < buflen ? buflen - len : 0);
>  
>  	/* Calculate how many bytes we need for the rest */
> -	for (kn = kn_to; kn != common; kn = kn->parent)
> -		nlen += strlen(kn->name) + 1;
> -
> -	if (len + nlen >= buflen)
> -		return len + nlen;
> -
> -	p = buf + len + nlen;
> -	*p = '\0';
> -	for (kn = kn_to; kn != common; kn = kn->parent) {
> -		size_t tmp = strlen(kn->name);
> -		p -= tmp;
> -		memcpy(p, kn->name, tmp);
> -		*(--p) = '/';
> +	for (i = depth_to - 1; i >= 0; i--) {
> +		for (kn = kn_to, j = 0; j < i; j++)
> +			kn = kn->parent;

This is O(n^2) where n is the path depth.  It's not a hot path, though, do
we care?

Re: [PATCH 2/4] kernfs: make kernfs_path*() behave in the style of strlcpy()

[Serge E. Hallyn]

Quoting Tejun Heo (tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org):
> kernfs_path*() functions always return the length of the full path but
> the path content is undefined if the length is larger than the
> provided buffer.  This makes its behavior different from strlcpy() and
> requires error handling in all its users even when they don't care
> about truncation.  In addition, the implementation can actully be
> simplified by making it behave properly in strlcpy() style.
> 
> * Update kernfs_path_from_node_locked() to always fill up the buffer
>   with path.  If the buffer is not large enough, the output is
>   truncated and terminated.
> 
> * kernfs_path() no longer needs error handling.  Make it a simple
>   inline wrapper around kernfs_path_from_node().
> 
> * sysfs_warn_dup()'s use of kernfs_path() doesn't need error handling.
>   Updated accordingly.
> 
> * cgroup_path()'s use of kernfs_path() updated to retain the old
>   behavior.
> 
> Signed-off-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: Serge Hallyn <serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA@public.gmane.org>
> Cc: Greg Kroah-Hartman <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>
> ---
>  fs/kernfs/dir.c        | 61 ++++++++++++++------------------------------------
>  fs/sysfs/dir.c         |  6 ++---
>  include/linux/cgroup.h |  7 +++++-
>  include/linux/kernfs.h | 21 ++++++++++++-----
>  4 files changed, 42 insertions(+), 53 deletions(-)
> 
> diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
> index e57174d..09242aa 100644
> --- a/fs/kernfs/dir.c
> +++ b/fs/kernfs/dir.c
> @@ -110,8 +110,9 @@ static struct kernfs_node *kernfs_common_ancestor(struct kernfs_node *a,
>   * kn_to:   /n1/n2/n3         [depth=3]
>   * result:  /../..
>   *
> - * return value: length of the string.  If greater than buflen,
> - * then contents of buf are undefined.  On error, -1 is returned.
> + * Returns the length of the full path.  If the full length is equal to or
> + * greater than @buflen, @buf contains the truncated path with the trailing
> + * '\0'.  On error, -errno is returned.
>   */
>  static int kernfs_path_from_node_locked(struct kernfs_node *kn_to,
>  					struct kernfs_node *kn_from,
> @@ -119,9 +120,8 @@ 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, nlen = 0;
> -	char *p;
> -	int i;
> +	size_t depth_from, depth_to, len = 0;
> +	int i, j;
>  
>  	if (!kn_from)
>  		kn_from = kernfs_root(kn_to)->kn;
> @@ -131,7 +131,7 @@ static int kernfs_path_from_node_locked(struct kernfs_node *kn_to,
>  
>  	common = kernfs_common_ancestor(kn_from, kn_to);
>  	if (WARN_ON(!common))
> -		return -1;
> +		return -EINVAL;
>  
>  	depth_to = kernfs_depth(common, kn_to);
>  	depth_from = kernfs_depth(common, kn_from);
> @@ -144,22 +144,16 @@ static int kernfs_path_from_node_locked(struct kernfs_node *kn_to,
>  			       len < buflen ? buflen - len : 0);
>  
>  	/* Calculate how many bytes we need for the rest */
> -	for (kn = kn_to; kn != common; kn = kn->parent)
> -		nlen += strlen(kn->name) + 1;
> -
> -	if (len + nlen >= buflen)
> -		return len + nlen;
> -
> -	p = buf + len + nlen;
> -	*p = '\0';
> -	for (kn = kn_to; kn != common; kn = kn->parent) {
> -		size_t tmp = strlen(kn->name);
> -		p -= tmp;
> -		memcpy(p, kn->name, tmp);
> -		*(--p) = '/';
> +	for (i = depth_to - 1; i >= 0; i--) {
> +		for (kn = kn_to, j = 0; j < i; j++)
> +			kn = kn->parent;

This is O(n^2) where n is the path depth.  It's not a hot path, though, do
we care?

Re: [PATCH 2/4] kernfs: make kernfs_path*() behave in the style of strlcpy()

[Tejun Heo]

Hello, Serge.

On Tue, Aug 09, 2016 at 10:33:05AM -0500, Serge E. Hallyn wrote:
> > +	for (i = depth_to - 1; i >= 0; i--) {
> > +		for (kn = kn_to, j = 0; j < i; j++)
> > +			kn = kn->parent;
> 
> This is O(n^2) where n is the path depth.  It's not a hot path, though, do
> we care?

I don't think it matters.  It's a slow path and cgroup hierarchies
aren't supposed to be super deep to begin with.  If it ever does, we
can replace the cgroup->ancestor_ids[] array with ancestor pointer
array and walk that instead.

Thanks.

-- 
tejun

Re: [PATCH 2/4] kernfs: make kernfs_path*() behave in the style of strlcpy()

[Tejun Heo]

Hello, Serge.

On Tue, Aug 09, 2016 at 10:33:05AM -0500, Serge E. Hallyn wrote:
> > +	for (i = depth_to - 1; i >= 0; i--) {
> > +		for (kn = kn_to, j = 0; j < i; j++)
> > +			kn = kn->parent;
> 
> This is O(n^2) where n is the path depth.  It's not a hot path, though, do
> we care?

I don't think it matters.  It's a slow path and cgroup hierarchies
aren't supposed to be super deep to begin with.  If it ever does, we
can replace the cgroup->ancestor_ids[] array with ancestor pointer
array and walk that instead.

Thanks.

-- 
tejun

Re: [PATCH 2/4] kernfs: make kernfs_path*() behave in the style of strlcpy()

[Serge E. Hallyn]

Quoting Tejun Heo (tj@kernel.org):
> Hello, Serge.
> 
> On Tue, Aug 09, 2016 at 10:33:05AM -0500, Serge E. Hallyn wrote:
> > > +	for (i = depth_to - 1; i >= 0; i--) {
> > > +		for (kn = kn_to, j = 0; j < i; j++)
> > > +			kn = kn->parent;
> > 
> > This is O(n^2) where n is the path depth.  It's not a hot path, though, do
> > we care?
> 
> I don't think it matters.  It's a slow path and cgroup hierarchies
> aren't supposed to be super deep to begin with.  If it ever does, we
> can replace the cgroup->ancestor_ids[] array with ancestor pointer
> array and walk that instead.
> 
> Thanks.

Ok, thanks

Acked-by: Serge Hallyn <serge@hallyn.com>

Re: [PATCH 2/4] kernfs: make kernfs_path*() behave in the style of strlcpy()

[Serge E. Hallyn]

Quoting Tejun Heo (tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org):
> Hello, Serge.
> 
> On Tue, Aug 09, 2016 at 10:33:05AM -0500, Serge E. Hallyn wrote:
> > > +	for (i = depth_to - 1; i >= 0; i--) {
> > > +		for (kn = kn_to, j = 0; j < i; j++)
> > > +			kn = kn->parent;
> > 
> > This is O(n^2) where n is the path depth.  It's not a hot path, though, do
> > we care?
> 
> I don't think it matters.  It's a slow path and cgroup hierarchies
> aren't supposed to be super deep to begin with.  If it ever does, we
> can replace the cgroup->ancestor_ids[] array with ancestor pointer
> array and walk that instead.
> 
> Thanks.

Ok, thanks

Acked-by: Serge Hallyn <serge-A9i7LUbDfNHQT0dZR+AlfA@public.gmane.org>

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

[Tejun Heo]

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

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

[Tejun Heo]

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-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>

Applied 1-4 to cgroup/for-4.9.

Thanks.

-- 
tejun