Skip to content

Commit db4aad2

Browse files
htejungregkh
htejun
authored and
gregkh
committed
kernfs: associate a new kernfs_node with its parent on creation
Once created, a kernfs_node is always destroyed by kernfs_put(). Since ba7443b ("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: Tejun Heo <tj@kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 917f56c commit db4aad2

File tree

4 files changed

+34
-24
lines changed

4 files changed

+34
-24
lines changed

‎fs/kernfs/dir.c

Lines changed: 26 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -324,8 +324,9 @@ const struct dentry_operations kernfs_dops = {
324324
.d_release = kernfs_dop_release,
325325
};
326326

327-
struct kernfs_node *kernfs_new_node(struct kernfs_root *root, const char *name,
328-
umode_t mode, unsigned flags)
327+
static struct kernfs_node *__kernfs_new_node(struct kernfs_root *root,
328+
const char *name, umode_t mode,
329+
unsigned flags)
329330
{
330331
char *dup_name = NULL;
331332
struct kernfs_node *kn;
@@ -362,6 +363,20 @@ struct kernfs_node *kernfs_new_node(struct kernfs_root *root, const char *name,
362363
return NULL;
363364
}
364365

366+
struct kernfs_node *kernfs_new_node(struct kernfs_node *parent,
367+
const char *name, umode_t mode,
368+
unsigned flags)
369+
{
370+
struct kernfs_node *kn;
371+
372+
kn = __kernfs_new_node(kernfs_root(parent), name, mode, flags);
373+
if (kn) {
374+
kernfs_get(parent);
375+
kn->parent = parent;
376+
}
377+
return kn;
378+
}
379+
365380
/**
366381
* kernfs_addrm_start - prepare for kernfs_node add/remove
367382
* @acxt: pointer to kernfs_addrm_cxt to be used
@@ -386,11 +401,10 @@ void kernfs_addrm_start(struct kernfs_addrm_cxt *acxt)
386401
* kernfs_add_one - add kernfs_node to parent without warning
387402
* @acxt: addrm context to use
388403
* @kn: kernfs_node to be added
389-
* @parent: the parent kernfs_node to add @kn to
390404
*
391-
* Get @parent and set @kn->parent to it and increment nlink of the
392-
* parent inode if @kn is a directory and link into the children list
393-
* of the parent.
405+
* The caller must already have initialized @kn->parent. This
406+
* function increments nlink of the parent's inode if @kn is a
407+
* directory and link into the children list of the parent.
394408
*
395409
* This function should be called between calls to
396410
* kernfs_addrm_start() and kernfs_addrm_finish() and should be passed
@@ -403,9 +417,9 @@ void kernfs_addrm_start(struct kernfs_addrm_cxt *acxt)
403417
* 0 on success, -EEXIST if entry with the given name already
404418
* exists.
405419
*/
406-
int kernfs_add_one(struct kernfs_addrm_cxt *acxt, struct kernfs_node *kn,
407-
struct kernfs_node *parent)
420+
int kernfs_add_one(struct kernfs_addrm_cxt *acxt, struct kernfs_node *kn)
408421
{
422+
struct kernfs_node *parent = kn->parent;
409423
bool has_ns = kernfs_ns_enabled(parent);
410424
struct kernfs_iattrs *ps_iattr;
411425
int ret;
@@ -423,8 +437,6 @@ int kernfs_add_one(struct kernfs_addrm_cxt *acxt, struct kernfs_node *kn,
423437
return -ENOENT;
424438

425439
kn->hash = kernfs_name_hash(kn->name, kn->ns);
426-
kn->parent = parent;
427-
kernfs_get(parent);
428440

429441
ret = kernfs_link_sibling(kn);
430442
if (ret)
@@ -600,7 +612,8 @@ struct kernfs_root *kernfs_create_root(struct kernfs_dir_ops *kdops, void *priv)
600612

601613
ida_init(&root->ino_ida);
602614

603-
kn = kernfs_new_node(root, "", S_IFDIR | S_IRUGO | S_IXUGO, KERNFS_DIR);
615+
kn = __kernfs_new_node(root, "", S_IFDIR | S_IRUGO | S_IXUGO,
616+
KERNFS_DIR);
604617
if (!kn) {
605618
ida_destroy(&root->ino_ida);
606619
kfree(root);
@@ -648,8 +661,7 @@ struct kernfs_node *kernfs_create_dir_ns(struct kernfs_node *parent,
648661
int rc;
649662

650663
/* allocate */
651-
kn = kernfs_new_node(kernfs_root(parent), name, mode | S_IFDIR,
652-
KERNFS_DIR);
664+
kn = kernfs_new_node(parent, name, mode | S_IFDIR, KERNFS_DIR);
653665
if (!kn)
654666
return ERR_PTR(-ENOMEM);
655667

@@ -659,7 +671,7 @@ struct kernfs_node *kernfs_create_dir_ns(struct kernfs_node *parent,
659671

660672
/* link in */
661673
kernfs_addrm_start(&acxt);
662-
rc = kernfs_add_one(&acxt, kn, parent);
674+
rc = kernfs_add_one(&acxt, kn);
663675
kernfs_addrm_finish(&acxt);
664676

665677
if (!rc)

‎fs/kernfs/file.c

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -829,8 +829,7 @@ struct kernfs_node *__kernfs_create_file(struct kernfs_node *parent,
829829
if (name_is_static)
830830
flags |= KERNFS_STATIC_NAME;
831831

832-
kn = kernfs_new_node(kernfs_root(parent), name,
833-
(mode & S_IALLUGO) | S_IFREG, flags);
832+
kn = kernfs_new_node(parent, name, (mode & S_IALLUGO) | S_IFREG, flags);
834833
if (!kn)
835834
return ERR_PTR(-ENOMEM);
836835

@@ -857,7 +856,7 @@ struct kernfs_node *__kernfs_create_file(struct kernfs_node *parent,
857856
kn->flags |= KERNFS_HAS_MMAP;
858857

859858
kernfs_addrm_start(&acxt);
860-
rc = kernfs_add_one(&acxt, kn, parent);
859+
rc = kernfs_add_one(&acxt, kn);
861860
kernfs_addrm_finish(&acxt);
862861

863862
if (rc) {

‎fs/kernfs/kernfs-internal.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -101,11 +101,11 @@ extern const struct inode_operations kernfs_dir_iops;
101101
struct kernfs_node *kernfs_get_active(struct kernfs_node *kn);
102102
void kernfs_put_active(struct kernfs_node *kn);
103103
void kernfs_addrm_start(struct kernfs_addrm_cxt *acxt);
104-
int kernfs_add_one(struct kernfs_addrm_cxt *acxt, struct kernfs_node *kn,
105-
struct kernfs_node *parent);
104+
int kernfs_add_one(struct kernfs_addrm_cxt *acxt, struct kernfs_node *kn);
106105
void kernfs_addrm_finish(struct kernfs_addrm_cxt *acxt);
107-
struct kernfs_node *kernfs_new_node(struct kernfs_root *root, const char *name,
108-
umode_t mode, unsigned flags);
106+
struct kernfs_node *kernfs_new_node(struct kernfs_node *parent,
107+
const char *name, umode_t mode,
108+
unsigned flags);
109109

110110
/*
111111
* file.c

‎fs/kernfs/symlink.c

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,7 @@ struct kernfs_node *kernfs_create_link(struct kernfs_node *parent,
3030
struct kernfs_addrm_cxt acxt;
3131
int error;
3232

33-
kn = kernfs_new_node(kernfs_root(parent), name, S_IFLNK|S_IRWXUGO,
34-
KERNFS_LINK);
33+
kn = kernfs_new_node(parent, name, S_IFLNK|S_IRWXUGO, KERNFS_LINK);
3534
if (!kn)
3635
return ERR_PTR(-ENOMEM);
3736

@@ -41,7 +40,7 @@ struct kernfs_node *kernfs_create_link(struct kernfs_node *parent,
4140
kernfs_get(target); /* ref owned by symlink */
4241

4342
kernfs_addrm_start(&acxt);
44-
error = kernfs_add_one(&acxt, kn, parent);
43+
error = kernfs_add_one(&acxt, kn);
4544
kernfs_addrm_finish(&acxt);
4645

4746
if (!error)

0 commit comments

Comments
 (0)