Commit db4aad20 authored by Tejun Heo's avatar Tejun Heo Committed by Greg Kroah-Hartman

kernfs: associate a new kernfs_node with its parent on creation

Once created, a kernfs_node is always destroyed by kernfs_put().
Since ba7443bc ("sysfs, kernfs: implement
kernfs_create/destroy_root()"), kernfs_put() depends on kernfs_root()
to locate the ino_ida.  kernfs_root() in turn depends on
kernfs_node->parent being set for !dir nodes.  This means that
kernfs_put() of a !dir node requires its ->parent to be initialized.

This leads to oops when a newly created !dir node is destroyed without
going through kernfs_add_one() or after failing kernfs_add_one()
before ->parent is set.  kernfs_root() invoked from kernfs_put() will
try to dereference NULL parent.

Fix it by moving parent association to kernfs_new_node() from
kernfs_add_one().  kernfs_new_node() now takes @parent instead of
@root and determines the root from the parent and also sets the new
node's parent properly.  @parent parameter is removed from
kernfs_add_one().  As there's no parent when creating the root node,
__kernfs_new_node() which takes @root as before and doesn't set the
parent is used in that case.

This ensures that a kernfs_node in any stage in its life has its
parent associated and thus can be put.
Signed-off-by: default avatarTejun Heo <tj@kernel.org>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 917f56ca
...@@ -324,8 +324,9 @@ const struct dentry_operations kernfs_dops = { ...@@ -324,8 +324,9 @@ const struct dentry_operations kernfs_dops = {
.d_release = kernfs_dop_release, .d_release = kernfs_dop_release,
}; };
struct kernfs_node *kernfs_new_node(struct kernfs_root *root, const char *name, static struct kernfs_node *__kernfs_new_node(struct kernfs_root *root,
umode_t mode, unsigned flags) const char *name, umode_t mode,
unsigned flags)
{ {
char *dup_name = NULL; char *dup_name = NULL;
struct kernfs_node *kn; struct kernfs_node *kn;
...@@ -362,6 +363,20 @@ struct kernfs_node *kernfs_new_node(struct kernfs_root *root, const char *name, ...@@ -362,6 +363,20 @@ struct kernfs_node *kernfs_new_node(struct kernfs_root *root, const char *name,
return NULL; return NULL;
} }
struct kernfs_node *kernfs_new_node(struct kernfs_node *parent,
const char *name, umode_t mode,
unsigned flags)
{
struct kernfs_node *kn;
kn = __kernfs_new_node(kernfs_root(parent), name, mode, flags);
if (kn) {
kernfs_get(parent);
kn->parent = parent;
}
return kn;
}
/** /**
* kernfs_addrm_start - prepare for kernfs_node add/remove * kernfs_addrm_start - prepare for kernfs_node add/remove
* @acxt: pointer to kernfs_addrm_cxt to be used * @acxt: pointer to kernfs_addrm_cxt to be used
...@@ -386,11 +401,10 @@ void kernfs_addrm_start(struct kernfs_addrm_cxt *acxt) ...@@ -386,11 +401,10 @@ void kernfs_addrm_start(struct kernfs_addrm_cxt *acxt)
* kernfs_add_one - add kernfs_node to parent without warning * kernfs_add_one - add kernfs_node to parent without warning
* @acxt: addrm context to use * @acxt: addrm context to use
* @kn: kernfs_node to be added * @kn: kernfs_node to be added
* @parent: the parent kernfs_node to add @kn to
* *
* Get @parent and set @kn->parent to it and increment nlink of the * The caller must already have initialized @kn->parent. This
* parent inode if @kn is a directory and link into the children list * function increments nlink of the parent's inode if @kn is a
* of the parent. * directory and link into the children list of the parent.
* *
* This function should be called between calls to * This function should be called between calls to
* kernfs_addrm_start() and kernfs_addrm_finish() and should be passed * kernfs_addrm_start() and kernfs_addrm_finish() and should be passed
...@@ -403,9 +417,9 @@ void kernfs_addrm_start(struct kernfs_addrm_cxt *acxt) ...@@ -403,9 +417,9 @@ void kernfs_addrm_start(struct kernfs_addrm_cxt *acxt)
* 0 on success, -EEXIST if entry with the given name already * 0 on success, -EEXIST if entry with the given name already
* exists. * exists.
*/ */
int kernfs_add_one(struct kernfs_addrm_cxt *acxt, struct kernfs_node *kn, int kernfs_add_one(struct kernfs_addrm_cxt *acxt, struct kernfs_node *kn)
struct kernfs_node *parent)
{ {
struct kernfs_node *parent = kn->parent;
bool has_ns = kernfs_ns_enabled(parent); bool has_ns = kernfs_ns_enabled(parent);
struct kernfs_iattrs *ps_iattr; struct kernfs_iattrs *ps_iattr;
int ret; int ret;
...@@ -423,8 +437,6 @@ int kernfs_add_one(struct kernfs_addrm_cxt *acxt, struct kernfs_node *kn, ...@@ -423,8 +437,6 @@ int kernfs_add_one(struct kernfs_addrm_cxt *acxt, struct kernfs_node *kn,
return -ENOENT; return -ENOENT;
kn->hash = kernfs_name_hash(kn->name, kn->ns); kn->hash = kernfs_name_hash(kn->name, kn->ns);
kn->parent = parent;
kernfs_get(parent);
ret = kernfs_link_sibling(kn); ret = kernfs_link_sibling(kn);
if (ret) if (ret)
...@@ -600,7 +612,8 @@ struct kernfs_root *kernfs_create_root(struct kernfs_dir_ops *kdops, void *priv) ...@@ -600,7 +612,8 @@ struct kernfs_root *kernfs_create_root(struct kernfs_dir_ops *kdops, void *priv)
ida_init(&root->ino_ida); ida_init(&root->ino_ida);
kn = kernfs_new_node(root, "", S_IFDIR | S_IRUGO | S_IXUGO, KERNFS_DIR); kn = __kernfs_new_node(root, "", S_IFDIR | S_IRUGO | S_IXUGO,
KERNFS_DIR);
if (!kn) { if (!kn) {
ida_destroy(&root->ino_ida); ida_destroy(&root->ino_ida);
kfree(root); kfree(root);
...@@ -648,8 +661,7 @@ struct kernfs_node *kernfs_create_dir_ns(struct kernfs_node *parent, ...@@ -648,8 +661,7 @@ struct kernfs_node *kernfs_create_dir_ns(struct kernfs_node *parent,
int rc; int rc;
/* allocate */ /* allocate */
kn = kernfs_new_node(kernfs_root(parent), name, mode | S_IFDIR, kn = kernfs_new_node(parent, name, mode | S_IFDIR, KERNFS_DIR);
KERNFS_DIR);
if (!kn) if (!kn)
return ERR_PTR(-ENOMEM); return ERR_PTR(-ENOMEM);
...@@ -659,7 +671,7 @@ struct kernfs_node *kernfs_create_dir_ns(struct kernfs_node *parent, ...@@ -659,7 +671,7 @@ struct kernfs_node *kernfs_create_dir_ns(struct kernfs_node *parent,
/* link in */ /* link in */
kernfs_addrm_start(&acxt); kernfs_addrm_start(&acxt);
rc = kernfs_add_one(&acxt, kn, parent); rc = kernfs_add_one(&acxt, kn);
kernfs_addrm_finish(&acxt); kernfs_addrm_finish(&acxt);
if (!rc) if (!rc)
......
...@@ -829,8 +829,7 @@ struct kernfs_node *__kernfs_create_file(struct kernfs_node *parent, ...@@ -829,8 +829,7 @@ struct kernfs_node *__kernfs_create_file(struct kernfs_node *parent,
if (name_is_static) if (name_is_static)
flags |= KERNFS_STATIC_NAME; flags |= KERNFS_STATIC_NAME;
kn = kernfs_new_node(kernfs_root(parent), name, kn = kernfs_new_node(parent, name, (mode & S_IALLUGO) | S_IFREG, flags);
(mode & S_IALLUGO) | S_IFREG, flags);
if (!kn) if (!kn)
return ERR_PTR(-ENOMEM); return ERR_PTR(-ENOMEM);
...@@ -857,7 +856,7 @@ struct kernfs_node *__kernfs_create_file(struct kernfs_node *parent, ...@@ -857,7 +856,7 @@ struct kernfs_node *__kernfs_create_file(struct kernfs_node *parent,
kn->flags |= KERNFS_HAS_MMAP; kn->flags |= KERNFS_HAS_MMAP;
kernfs_addrm_start(&acxt); kernfs_addrm_start(&acxt);
rc = kernfs_add_one(&acxt, kn, parent); rc = kernfs_add_one(&acxt, kn);
kernfs_addrm_finish(&acxt); kernfs_addrm_finish(&acxt);
if (rc) { if (rc) {
......
...@@ -101,11 +101,11 @@ extern const struct inode_operations kernfs_dir_iops; ...@@ -101,11 +101,11 @@ extern const struct inode_operations kernfs_dir_iops;
struct kernfs_node *kernfs_get_active(struct kernfs_node *kn); struct kernfs_node *kernfs_get_active(struct kernfs_node *kn);
void kernfs_put_active(struct kernfs_node *kn); void kernfs_put_active(struct kernfs_node *kn);
void kernfs_addrm_start(struct kernfs_addrm_cxt *acxt); void kernfs_addrm_start(struct kernfs_addrm_cxt *acxt);
int kernfs_add_one(struct kernfs_addrm_cxt *acxt, struct kernfs_node *kn, int kernfs_add_one(struct kernfs_addrm_cxt *acxt, struct kernfs_node *kn);
struct kernfs_node *parent);
void kernfs_addrm_finish(struct kernfs_addrm_cxt *acxt); void kernfs_addrm_finish(struct kernfs_addrm_cxt *acxt);
struct kernfs_node *kernfs_new_node(struct kernfs_root *root, const char *name, struct kernfs_node *kernfs_new_node(struct kernfs_node *parent,
umode_t mode, unsigned flags); const char *name, umode_t mode,
unsigned flags);
/* /*
* file.c * file.c
......
...@@ -30,8 +30,7 @@ struct kernfs_node *kernfs_create_link(struct kernfs_node *parent, ...@@ -30,8 +30,7 @@ struct kernfs_node *kernfs_create_link(struct kernfs_node *parent,
struct kernfs_addrm_cxt acxt; struct kernfs_addrm_cxt acxt;
int error; int error;
kn = kernfs_new_node(kernfs_root(parent), name, S_IFLNK|S_IRWXUGO, kn = kernfs_new_node(parent, name, S_IFLNK|S_IRWXUGO, KERNFS_LINK);
KERNFS_LINK);
if (!kn) if (!kn)
return ERR_PTR(-ENOMEM); return ERR_PTR(-ENOMEM);
...@@ -41,7 +40,7 @@ struct kernfs_node *kernfs_create_link(struct kernfs_node *parent, ...@@ -41,7 +40,7 @@ struct kernfs_node *kernfs_create_link(struct kernfs_node *parent,
kernfs_get(target); /* ref owned by symlink */ kernfs_get(target); /* ref owned by symlink */
kernfs_addrm_start(&acxt); kernfs_addrm_start(&acxt);
error = kernfs_add_one(&acxt, kn, parent); error = kernfs_add_one(&acxt, kn);
kernfs_addrm_finish(&acxt); kernfs_addrm_finish(&acxt);
if (!error) if (!error)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment