From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932240AbdBHMvg (ORCPT ); Wed, 8 Feb 2017 07:51:36 -0500 Received: from mail.linuxfoundation.org ([140.211.169.12]:50798 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932072AbdBHMvc (ORCPT ); Wed, 8 Feb 2017 07:51:32 -0500 Date: Wed, 8 Feb 2017 13:51:21 +0100 From: Greg Kroah-Hartman To: Konstantin Khlebnikov Cc: linux-kernel@vger.kernel.org, Tejun Heo Subject: Re: [PATCH] kernfs: handle null pointers while printing node name and path Message-ID: <20170208125121.GA27175@kroah.com> References: <148655333504.426064.1880193250859194893.stgit@buzz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <148655333504.426064.1880193250859194893.stgit@buzz> User-Agent: Mutt/1.7.2 (2016-11-26) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Feb 08, 2017 at 02:28:55PM +0300, Konstantin Khlebnikov wrote: > Null kernfs nodes could be found at cgroups during construction. Really? Does this happen today? Is this an issue for older kernels as well? > It seems safer to handle these null pointers right in kernfs in > the same way as printf prints "(null)" for null pointer string. > > Signed-off-by: Konstantin Khlebnikov > Acked-by: Tejun Heo > --- > fs/kernfs/dir.c | 10 ++++++++++ > include/trace/events/cgroup.h | 20 ++++++-------------- > 2 files changed, 16 insertions(+), 14 deletions(-) > > diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c > index cf4c636ff4da..439b946c4808 100644 > --- a/fs/kernfs/dir.c > +++ b/fs/kernfs/dir.c > @@ -41,6 +41,9 @@ static bool kernfs_lockdep(struct kernfs_node *kn) > > static int kernfs_name_locked(struct kernfs_node *kn, char *buf, size_t buflen) > { > + if (!kn) > + return strlcpy(buf, "(null)", buflen); Why not return an error? > + > return strlcpy(buf, kn->parent ? kn->name : "/", buflen); > } > > @@ -110,6 +113,8 @@ static struct kernfs_node *kernfs_common_ancestor(struct kernfs_node *a, > * kn_to: /n1/n2/n3 [depth=3] > * result: /../.. > * > + * [3] when @kn_to is NULL result will be "(null)" > + * > * 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. > @@ -123,6 +128,9 @@ static int kernfs_path_from_node_locked(struct kernfs_node *kn_to, > size_t depth_from, depth_to, len = 0; > int i, j; > > + if (!kn_to) > + return strlcpy(buf, "(null)", buflen); > + > if (!kn_from) > kn_from = kernfs_root(kn_to)->kn; > > @@ -166,6 +174,8 @@ static int kernfs_path_from_node_locked(struct kernfs_node *kn_to, > * similar to strlcpy(). It returns the length of @kn's name and if @buf > * isn't long enough, it's filled upto @buflen-1 and nul terminated. > * > + * Fills buffer with "(null)" if @kn is NULL. > + * > * This function can be called from any context. > */ > int kernfs_name(struct kernfs_node *kn, char *buf, size_t buflen) > diff --git a/include/trace/events/cgroup.h b/include/trace/events/cgroup.h > index ab68640a18d0..c226f50e88fa 100644 > --- a/include/trace/events/cgroup.h > +++ b/include/trace/events/cgroup.h > @@ -61,19 +61,15 @@ DECLARE_EVENT_CLASS(cgroup, > __field( int, id ) > __field( int, level ) > __dynamic_array(char, path, > - cgrp->kn ? cgroup_path(cgrp, NULL, 0) + 1 > - : strlen("(null)")) > + cgroup_path(cgrp, NULL, 0) + 1) Ah, you are trying to make this "simpler", is that the case? So this is just a "cleanup", not a bugfix? thanks, greg k-h