Skip to content

Commit

Permalink
debugfs: return error values, not NULL
Browse files Browse the repository at this point in the history
When an error happens, debugfs should return an error pointer value, not
NULL.  This will prevent the totally theoretical error where a debugfs
call fails due to lack of memory, returning NULL, and that dentry value
is then passed to another debugfs call, which would end up succeeding,
creating a file at the root of the debugfs tree, but would then be
impossible to remove (because you can not remove the directory NULL).

So, to make everyone happy, always return errors, this makes the users
of debugfs much simpler (they do not have to ever check the return
value), and everyone can rest easy.

Reported-by: Gary R Hook <ghook@amd.com>
Reported-by: Heiko Carstens <heiko.carstens@de.ibm.com>
Reported-by: Masami Hiramatsu <mhiramat@kernel.org>
Reported-by: Michal Hocko <mhocko@kernel.org>
Reported-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Reported-by: Ulf Hansson <ulf.hansson@linaro.org>
Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org>
Reviewed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  • Loading branch information
gregkh committed Jan 29, 2019
1 parent d88c93f commit ff9fb72
Showing 1 changed file with 22 additions and 17 deletions.
39 changes: 22 additions & 17 deletions fs/debugfs/inode.c
Original file line number Diff line number Diff line change
Expand Up @@ -254,8 +254,8 @@ MODULE_ALIAS_FS("debugfs");
* @parent: a pointer to the parent dentry of the file.
*
* This function will return a pointer to a dentry if it succeeds. If the file
* doesn't exist or an error occurs, %NULL will be returned. The returned
* dentry must be passed to dput() when it is no longer needed.
* doesn't exist or an error occurs, %ERR_PTR(-ERROR) will be returned. The
* returned dentry must be passed to dput() when it is no longer needed.
*
* If debugfs is not enabled in the kernel, the value -%ENODEV will be
* returned.
Expand All @@ -265,17 +265,17 @@ struct dentry *debugfs_lookup(const char *name, struct dentry *parent)
struct dentry *dentry;

if (IS_ERR(parent))
return NULL;
return parent;

if (!parent)
parent = debugfs_mount->mnt_root;

dentry = lookup_one_len_unlocked(name, parent, strlen(name));
if (IS_ERR(dentry))
return NULL;
return dentry;
if (!d_really_is_positive(dentry)) {
dput(dentry);
return NULL;
return ERR_PTR(-EINVAL);
}
return dentry;
}
Expand Down Expand Up @@ -324,7 +324,7 @@ static struct dentry *failed_creating(struct dentry *dentry)
inode_unlock(d_inode(dentry->d_parent));
dput(dentry);
simple_release_fs(&debugfs_mount, &debugfs_mount_count);
return NULL;
return ERR_PTR(-ENOMEM);
}

static struct dentry *end_creating(struct dentry *dentry)
Expand All @@ -347,7 +347,7 @@ static struct dentry *__debugfs_create_file(const char *name, umode_t mode,
dentry = start_creating(name, parent);

if (IS_ERR(dentry))
return NULL;
return dentry;

inode = debugfs_get_inode(dentry->d_sb);
if (unlikely(!inode))
Expand Down Expand Up @@ -386,7 +386,8 @@ static struct dentry *__debugfs_create_file(const char *name, umode_t mode,
* This function will return a pointer to a dentry if it succeeds. This
* pointer must be passed to the debugfs_remove() function when the file is
* to be removed (no automatic cleanup happens if your module is unloaded,
* you are responsible here.) If an error occurs, %NULL will be returned.
* you are responsible here.) If an error occurs, %ERR_PTR(-ERROR) will be
* returned.
*
* If debugfs is not enabled in the kernel, the value -%ENODEV will be
* returned.
Expand Down Expand Up @@ -464,7 +465,8 @@ EXPORT_SYMBOL_GPL(debugfs_create_file_unsafe);
* This function will return a pointer to a dentry if it succeeds. This
* pointer must be passed to the debugfs_remove() function when the file is
* to be removed (no automatic cleanup happens if your module is unloaded,
* you are responsible here.) If an error occurs, %NULL will be returned.
* you are responsible here.) If an error occurs, %ERR_PTR(-ERROR) will be
* returned.
*
* If debugfs is not enabled in the kernel, the value -%ENODEV will be
* returned.
Expand Down Expand Up @@ -495,7 +497,8 @@ EXPORT_SYMBOL_GPL(debugfs_create_file_size);
* This function will return a pointer to a dentry if it succeeds. This
* pointer must be passed to the debugfs_remove() function when the file is
* to be removed (no automatic cleanup happens if your module is unloaded,
* you are responsible here.) If an error occurs, %NULL will be returned.
* you are responsible here.) If an error occurs, %ERR_PTR(-ERROR) will be
* returned.
*
* If debugfs is not enabled in the kernel, the value -%ENODEV will be
* returned.
Expand All @@ -506,7 +509,7 @@ struct dentry *debugfs_create_dir(const char *name, struct dentry *parent)
struct inode *inode;

if (IS_ERR(dentry))
return NULL;
return dentry;

inode = debugfs_get_inode(dentry->d_sb);
if (unlikely(!inode))
Expand Down Expand Up @@ -545,7 +548,7 @@ struct dentry *debugfs_create_automount(const char *name,
struct inode *inode;

if (IS_ERR(dentry))
return NULL;
return dentry;

inode = debugfs_get_inode(dentry->d_sb);
if (unlikely(!inode))
Expand Down Expand Up @@ -581,8 +584,8 @@ EXPORT_SYMBOL(debugfs_create_automount);
* This function will return a pointer to a dentry if it succeeds. This
* pointer must be passed to the debugfs_remove() function when the symbolic
* link is to be removed (no automatic cleanup happens if your module is
* unloaded, you are responsible here.) If an error occurs, %NULL will be
* returned.
* unloaded, you are responsible here.) If an error occurs, %ERR_PTR(-ERROR)
* will be returned.
*
* If debugfs is not enabled in the kernel, the value -%ENODEV will be
* returned.
Expand All @@ -594,12 +597,12 @@ struct dentry *debugfs_create_symlink(const char *name, struct dentry *parent,
struct inode *inode;
char *link = kstrdup(target, GFP_KERNEL);
if (!link)
return NULL;
return ERR_PTR(-ENOMEM);

dentry = start_creating(name, parent);
if (IS_ERR(dentry)) {
kfree(link);
return NULL;
return dentry;
}

inode = debugfs_get_inode(dentry->d_sb);
Expand Down Expand Up @@ -827,7 +830,9 @@ struct dentry *debugfs_rename(struct dentry *old_dir, struct dentry *old_dentry,
if (dentry && !IS_ERR(dentry))
dput(dentry);
unlock_rename(new_dir, old_dir);
return NULL;
if (IS_ERR(dentry))
return dentry;
return ERR_PTR(-EINVAL);
}
EXPORT_SYMBOL_GPL(debugfs_rename);

Expand Down

0 comments on commit ff9fb72

Please sign in to comment.