Trond, this is kind of your baliwick, but do you mind if I take it
instead? It's kind of a pain for my testing currently.
--b.
On Tue, Mar 31, 2015 at 12:03:28PM -0400, Jeff Layton wrote:
> v2: gracefully handle the case where some dentry pointers end up NULL
> and be more dilligent about zeroing out dentry pointers
>
> 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: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> Acked-by: "J. Bruce Fields" <bfields@xxxxxxxxxxxx>
> Signed-off-by: Jeff Layton <jeff.layton@xxxxxxxxxxxxxxx>
> ---
> include/linux/sunrpc/debug.h | 18 +++++++--------
> net/sunrpc/clnt.c | 4 +---
> net/sunrpc/debugfs.c | 52 ++++++++++++++++++++++++--------------------
> net/sunrpc/sunrpc_syms.c | 7 +-----
> net/sunrpc/xprt.c | 7 +-----
> 5 files changed, 41 insertions(+), 47 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..82962f7e6e88 100644
> --- a/net/sunrpc/debugfs.c
> +++ b/net/sunrpc/debugfs.c
> @@ -129,48 +129,52 @@ 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 */
> + struct rpc_xprt *xprt;
>
> /* Already registered? */
> - if (clnt->cl_debugfs)
> - return 0;
> + if (clnt->cl_debugfs || !rpc_clnt_dir)
> + 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();
> + xprt = rcu_dereference(clnt->cl_xprt);
> + /* no "debugfs" dentry? Don't bother with the symlink. */
> + if (!xprt->debugfs) {
> + rcu_read_unlock();
> + return;
> + }
> len = snprintf(name, sizeof(name), "../../rpc_xprt/%s",
> - rcu_dereference(clnt->cl_xprt)->debugfs->d_name.name);
> + xprt->debugfs->d_name.name);
> rcu_read_unlock();
> +
> 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,33 +230,33 @@ 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;
> static atomic_t cur_id;
> char name[9]; /* 8 hex digits + NULL term */
>
> + if (!rpc_xprt_dir)
> + return;
> +
> id = (unsigned int)atomic_inc_return(&cur_id);
>
> 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 +270,17 @@ void __exit
> sunrpc_debugfs_exit(void)
> {
> debugfs_remove_recursive(topdir);
> + topdir = NULL;
> + rpc_clnt_dir = NULL;
> + rpc_xprt_dir = 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 +290,9 @@ 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;
> + rpc_clnt_dir = NULL;
> }
> 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-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
