Minki/linux
1
0
Fork 1
mirror of https://github.com/torvalds/linux.git synced 2024-11-24 21:21:41 +00:00
Code Issues Packages Projects Releases Wiki Activity

eventfs: Test for ei->is_freed when accessing ei->dentry

Browse Source 
The eventfs_inode (ei) is protected by SRCU, but the ei->dentry is not. It
is protected by the eventfs_mutex. Anytime the eventfs_mutex is released,
and access to the ei->dentry needs to be done, it should first check if
ei->is_freed is set under the eventfs_mutex. If it is, then the ei->dentry
is invalid and must not be used. The ei->dentry must only be accessed
under the eventfs_mutex and after checking if ei->is_freed is set.

When the ei is being freed, it will (under the eventfs_mutex) set is_freed
and at the same time move the dentry to a free list to be cleared after
the eventfs_mutex is released. This means that any access to the
ei->dentry must check first if ei->is_freed is set, because if it is, then
the dentry is on its way to be freed.

Also add comments to describe this better.

Link: https://lore.kernel.org/all/CA+G9fYt6pY+tMZEOg=SoEywQOe19fGP3uR15SGowkdK+_X85Cg@mail.gmail.com/
Link: https://lore.kernel.org/all/CA+G9fYuDP3hVQ3t7FfrBAjd_WFVSurMgCepTxunSJf=MTe=6aA@mail.gmail.com/
Link: https://lkml.kernel.org/r/20231101172649.477608228@goodmis.org

Cc: Ajay Kaher <akaher@vmware.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Fixes: 5790b1fb3d ("eventfs: Remove eventfs_file and just use eventfs_inode")
Reported-by: Linux Kernel Functional Testing <lkft@linaro.org>
Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org>
Reported-by: Beau Belgrave <beaub@linux.microsoft.com>
Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Tested-by: Linux Kernel Functional Testing <lkft@linaro.org>
Tested-by: Naresh Kamboju <naresh.kamboju@linaro.org>
Tested-by: Beau Belgrave <beaub@linux.microsoft.com>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
This commit is contained in:
Steven Rostedt (Google) 2023-11-01 13:25:44 -04:00
parent db3a397209
commit 77a06c33a2
2 changed files with 41 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

45
fs/tracefs/event_inode.c
View File

@ -24,7 +24,20 @@
#include <linux/delay.h>
#include "internal.h"
/*
* eventfs_mutex protects the eventfs_inode (ei) dentry. Any access
* to the ei->dentry must be done under this mutex and after checking
* if ei->is_freed is not set. The ei->dentry is released under the
* mutex at the same time ei->is_freed is set. If ei->is_freed is set
* then the ei->dentry is invalid.
*/
static DEFINE_MUTEX(eventfs_mutex);
/*
* The eventfs_inode (ei) itself is protected by SRCU. It is released from
* its parent's list and will have is_freed set (under eventfs_mutex).
* After the SRCU grace period is over, the ei may be freed.
*/
DEFINE_STATIC_SRCU(eventfs_srcu);
static struct dentry *eventfs_root_lookup(struct inode *dir,
@ -239,6 +252,10 @@ create_file_dentry(struct eventfs_inode *ei, struct dentry **e_dentry,
bool invalidate = false;
mutex_lock(&eventfs_mutex);
if (ei->is_freed) {
mutex_unlock(&eventfs_mutex);
return NULL;
}
/* If the e_dentry already has a dentry, use it */
if (*e_dentry) {
/* lookup does not need to up the ref count */
@ -312,6 +329,8 @@ static void eventfs_post_create_dir(struct eventfs_inode *ei)
struct eventfs_inode *ei_child;
struct tracefs_inode *ti;
lockdep_assert_held(&eventfs_mutex);
/* srcu lock already held */
/* fill parent-child relation */
list_for_each_entry_srcu(ei_child, &ei->children, list,
@ -325,6 +344,7 @@ static void eventfs_post_create_dir(struct eventfs_inode *ei)
/**
* create_dir_dentry - Create a directory dentry for the eventfs_inode
* @pei: The eventfs_inode parent of ei.
* @ei: The eventfs_inode to create the directory for
* @parent: The dentry of the parent of this directory
* @lookup: True if this is called by the lookup code
@ -332,12 +352,17 @@ static void eventfs_post_create_dir(struct eventfs_inode *ei)
* This creates and attaches a directory dentry to the eventfs_inode @ei.
*/
static struct dentry *
create_dir_dentry(struct eventfs_inode *ei, struct dentry *parent, bool lookup)
create_dir_dentry(struct eventfs_inode *pei, struct eventfs_inode *ei,
struct dentry *parent, bool lookup)
{
bool invalidate = false;
struct dentry *dentry = NULL;
mutex_lock(&eventfs_mutex);
if (pei->is_freed || ei->is_freed) {
mutex_unlock(&eventfs_mutex);
return NULL;
}
if (ei->dentry) {
/* If the dentry already has a dentry, use it */
dentry = ei->dentry;
@ -440,7 +465,7 @@ static struct dentry *eventfs_root_lookup(struct inode *dir,
*/
mutex_lock(&eventfs_mutex);
ei = READ_ONCE(ti->private);
if (ei)
if (ei && !ei->is_freed)
ei_dentry = READ_ONCE(ei->dentry);
mutex_unlock(&eventfs_mutex);
@ -454,7 +479,7 @@ static struct dentry *eventfs_root_lookup(struct inode *dir,
if (strcmp(ei_child->name, name) != 0)
continue;
ret = simple_lookup(dir, dentry, flags);
create_dir_dentry(ei_child, ei_dentry, true);
create_dir_dentry(ei, ei_child, ei_dentry, true);
created = true;
break;
}
@ -588,7 +613,7 @@ static int dcache_dir_open_wrapper(struct inode *inode, struct file *file)
list_for_each_entry_srcu(ei_child, &ei->children, list,
srcu_read_lock_held(&eventfs_srcu)) {
d = create_dir_dentry(ei_child, parent, false);
d = create_dir_dentry(ei, ei_child, parent, false);
if (d) {
ret = add_dentries(&dentries, d, cnt);
if (ret < 0)
@ -705,12 +730,20 @@ struct eventfs_inode *eventfs_create_dir(const char *name, struct eventfs_inode
ei->nr_entries = size;
ei->data = data;
INIT_LIST_HEAD(&ei->children);
INIT_LIST_HEAD(&ei->list);
mutex_lock(&eventfs_mutex);
list_add_tail(&ei->list, &parent->children);
ei->d_parent = parent->dentry;
if (!parent->is_freed) {
list_add_tail(&ei->list, &parent->children);
ei->d_parent = parent->dentry;
}
mutex_unlock(&eventfs_mutex);
/* Was the parent freed? */
if (list_empty(&ei->list)) {
free_ei(ei);
ei = NULL;
}
return ei;
}

3
fs/tracefs/internal.h
View File

@ -24,6 +24,7 @@ struct tracefs_inode {
* @d_children: The array of dentries to represent the files when created
* @data: The private data to pass to the callbacks
* @is_freed: Flag set if the eventfs is on its way to be freed
* Note if is_freed is set, then dentry is corrupted.
* @nr_entries: The number of items in @entries
*/
struct eventfs_inode {
@ -31,7 +32,7 @@ struct eventfs_inode {
const struct eventfs_entry *entries;
const char *name;
struct list_head children;
struct dentry *dentry;
struct dentry *dentry; /* Check is_freed to access */
struct dentry *d_parent;
struct dentry **d_children;
void *data;