Re: [PATCH] sunrpc: make debugfs file creation failure non-fatal

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



ACK.--b.

On Mon, Mar 30, 2015 at 05:58:18PM -0400, Jeff Layton wrote:
> We currently have a problem that SELinux policy is being enforced when
> creating debugfs files. If a debugfs file is created as a side effect of
> doing some syscall, then that creation can fail if the SELinux policy
> for that process prevents it.
> 
> This seems wrong. We don't do that for files under /proc, for instance,
> so Bruce has proposed a patch to fix that.
> 
> While discussing that patch however, Greg K.H. stated:
> 
>     "No kernel code should care / fail if a debugfs function fails, so
>      please fix up the sunrpc code first."
> 
> This patch converts all of the sunrpc debugfs setup code to be void
> return functins, and the callers to not look for errors from those
> functions.
> 
> This should allow rpc_clnt and rpc_xprt creation to work, even if the
> kernel fails to create debugfs files for some reason.
> 
> Cc: "J. Bruce Fields" <bfields@xxxxxxxxxxxx>
> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Jeff Layton <jeff.layton@xxxxxxxxxxxxxxx>
> ---
>  include/linux/sunrpc/debug.h | 18 +++++++++---------
>  net/sunrpc/clnt.c            |  4 +---
>  net/sunrpc/debugfs.c         | 34 +++++++++++++---------------------
>  net/sunrpc/sunrpc_syms.c     |  7 +------
>  net/sunrpc/xprt.c            |  7 +------
>  5 files changed, 25 insertions(+), 45 deletions(-)
> 
> diff --git a/include/linux/sunrpc/debug.h b/include/linux/sunrpc/debug.h
> index c57d8ea0716c..59a7889e15db 100644
> --- a/include/linux/sunrpc/debug.h
> +++ b/include/linux/sunrpc/debug.h
> @@ -60,17 +60,17 @@ struct rpc_xprt;
>  #if IS_ENABLED(CONFIG_SUNRPC_DEBUG)
>  void		rpc_register_sysctl(void);
>  void		rpc_unregister_sysctl(void);
> -int		sunrpc_debugfs_init(void);
> +void		sunrpc_debugfs_init(void);
>  void		sunrpc_debugfs_exit(void);
> -int		rpc_clnt_debugfs_register(struct rpc_clnt *);
> +void		rpc_clnt_debugfs_register(struct rpc_clnt *);
>  void		rpc_clnt_debugfs_unregister(struct rpc_clnt *);
> -int		rpc_xprt_debugfs_register(struct rpc_xprt *);
> +void		rpc_xprt_debugfs_register(struct rpc_xprt *);
>  void		rpc_xprt_debugfs_unregister(struct rpc_xprt *);
>  #else
> -static inline int
> +static inline void
>  sunrpc_debugfs_init(void)
>  {
> -	return 0;
> +	return;
>  }
>  
>  static inline void
> @@ -79,10 +79,10 @@ sunrpc_debugfs_exit(void)
>  	return;
>  }
>  
> -static inline int
> +static inline void
>  rpc_clnt_debugfs_register(struct rpc_clnt *clnt)
>  {
> -	return 0;
> +	return;
>  }
>  
>  static inline void
> @@ -91,10 +91,10 @@ rpc_clnt_debugfs_unregister(struct rpc_clnt *clnt)
>  	return;
>  }
>  
> -static inline int
> +static inline void
>  rpc_xprt_debugfs_register(struct rpc_xprt *xprt)
>  {
> -	return 0;
> +	return;
>  }
>  
>  static inline void
> diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
> index 612aa73bbc60..e6ce1517367f 100644
> --- a/net/sunrpc/clnt.c
> +++ b/net/sunrpc/clnt.c
> @@ -303,9 +303,7 @@ static int rpc_client_register(struct rpc_clnt *clnt,
>  	struct super_block *pipefs_sb;
>  	int err;
>  
> -	err = rpc_clnt_debugfs_register(clnt);
> -	if (err)
> -		return err;
> +	rpc_clnt_debugfs_register(clnt);
>  
>  	pipefs_sb = rpc_get_sb_net(net);
>  	if (pipefs_sb) {
> diff --git a/net/sunrpc/debugfs.c b/net/sunrpc/debugfs.c
> index e811f390f9f6..1c2e3960b939 100644
> --- a/net/sunrpc/debugfs.c
> +++ b/net/sunrpc/debugfs.c
> @@ -129,32 +129,30 @@ static const struct file_operations tasks_fops = {
>  	.release	= tasks_release,
>  };
>  
> -int
> +void
>  rpc_clnt_debugfs_register(struct rpc_clnt *clnt)
>  {
> -	int len, err;
> +	int len;
>  	char name[24]; /* enough for "../../rpc_xprt/ + 8 hex digits + NULL */
>  
>  	/* Already registered? */
>  	if (clnt->cl_debugfs)
> -		return 0;
> +		return;
>  
>  	len = snprintf(name, sizeof(name), "%x", clnt->cl_clid);
>  	if (len >= sizeof(name))
> -		return -EINVAL;
> +		return;
>  
>  	/* make the per-client dir */
>  	clnt->cl_debugfs = debugfs_create_dir(name, rpc_clnt_dir);
>  	if (!clnt->cl_debugfs)
> -		return -ENOMEM;
> +		return;
>  
>  	/* make tasks file */
> -	err = -ENOMEM;
>  	if (!debugfs_create_file("tasks", S_IFREG | S_IRUSR, clnt->cl_debugfs,
>  				 clnt, &tasks_fops))
>  		goto out_err;
>  
> -	err = -EINVAL;
>  	rcu_read_lock();
>  	len = snprintf(name, sizeof(name), "../../rpc_xprt/%s",
>  			rcu_dereference(clnt->cl_xprt)->debugfs->d_name.name);
> @@ -162,15 +160,13 @@ rpc_clnt_debugfs_register(struct rpc_clnt *clnt)
>  	if (len >= sizeof(name))
>  		goto out_err;
>  
> -	err = -ENOMEM;
>  	if (!debugfs_create_symlink("xprt", clnt->cl_debugfs, name))
>  		goto out_err;
>  
> -	return 0;
> +	return;
>  out_err:
>  	debugfs_remove_recursive(clnt->cl_debugfs);
>  	clnt->cl_debugfs = NULL;
> -	return err;
>  }
>  
>  void
> @@ -226,7 +222,7 @@ static const struct file_operations xprt_info_fops = {
>  	.release	= xprt_info_release,
>  };
>  
> -int
> +void
>  rpc_xprt_debugfs_register(struct rpc_xprt *xprt)
>  {
>  	int len, id;
> @@ -237,22 +233,19 @@ rpc_xprt_debugfs_register(struct rpc_xprt *xprt)
>  
>  	len = snprintf(name, sizeof(name), "%x", id);
>  	if (len >= sizeof(name))
> -		return -EINVAL;
> +		return;
>  
>  	/* make the per-client dir */
>  	xprt->debugfs = debugfs_create_dir(name, rpc_xprt_dir);
>  	if (!xprt->debugfs)
> -		return -ENOMEM;
> +		return;
>  
>  	/* make tasks file */
>  	if (!debugfs_create_file("info", S_IFREG | S_IRUSR, xprt->debugfs,
>  				 xprt, &xprt_info_fops)) {
>  		debugfs_remove_recursive(xprt->debugfs);
>  		xprt->debugfs = NULL;
> -		return -ENOMEM;
>  	}
> -
> -	return 0;
>  }
>  
>  void
> @@ -266,14 +259,15 @@ void __exit
>  sunrpc_debugfs_exit(void)
>  {
>  	debugfs_remove_recursive(topdir);
> +	topdir = NULL;
>  }
>  
> -int __init
> +void __init
>  sunrpc_debugfs_init(void)
>  {
>  	topdir = debugfs_create_dir("sunrpc", NULL);
>  	if (!topdir)
> -		goto out;
> +		return;
>  
>  	rpc_clnt_dir = debugfs_create_dir("rpc_clnt", topdir);
>  	if (!rpc_clnt_dir)
> @@ -283,10 +277,8 @@ sunrpc_debugfs_init(void)
>  	if (!rpc_xprt_dir)
>  		goto out_remove;
>  
> -	return 0;
> +	return;
>  out_remove:
>  	debugfs_remove_recursive(topdir);
>  	topdir = NULL;
> -out:
> -	return -ENOMEM;
>  }
> diff --git a/net/sunrpc/sunrpc_syms.c b/net/sunrpc/sunrpc_syms.c
> index e37fbed87956..ee5d3d253102 100644
> --- a/net/sunrpc/sunrpc_syms.c
> +++ b/net/sunrpc/sunrpc_syms.c
> @@ -98,10 +98,7 @@ init_sunrpc(void)
>  	if (err)
>  		goto out4;
>  
> -	err = sunrpc_debugfs_init();
> -	if (err)
> -		goto out5;
> -
> +	sunrpc_debugfs_init();
>  #if IS_ENABLED(CONFIG_SUNRPC_DEBUG)
>  	rpc_register_sysctl();
>  #endif
> @@ -109,8 +106,6 @@ init_sunrpc(void)
>  	init_socket_xprt();	/* clnt sock transport */
>  	return 0;
>  
> -out5:
> -	unregister_rpc_pipefs();
>  out4:
>  	unregister_pernet_subsys(&sunrpc_net_ops);
>  out3:
> diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c
> index e3015aede0d9..9949722d99ce 100644
> --- a/net/sunrpc/xprt.c
> +++ b/net/sunrpc/xprt.c
> @@ -1331,7 +1331,6 @@ static void xprt_init(struct rpc_xprt *xprt, struct net *net)
>   */
>  struct rpc_xprt *xprt_create_transport(struct xprt_create *args)
>  {
> -	int err;
>  	struct rpc_xprt	*xprt;
>  	struct xprt_class *t;
>  
> @@ -1372,11 +1371,7 @@ found:
>  		return ERR_PTR(-ENOMEM);
>  	}
>  
> -	err = rpc_xprt_debugfs_register(xprt);
> -	if (err) {
> -		xprt_destroy(xprt);
> -		return ERR_PTR(err);
> -	}
> +	rpc_xprt_debugfs_register(xprt);
>  
>  	dprintk("RPC:       created transport %p with %u slots\n", xprt,
>  			xprt->max_reqs);
> -- 
> 2.1.0
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]
  Powered by Linux