Skip to content

Commit

Permalink
configfs: provide exclusion between IO and removals
Browse files Browse the repository at this point in the history 
Make sure that attribute methods are not called after the item
has been removed from the tree.  To do so, we
	* at the point of no return in removals, grab ->frag_sem
exclusive and mark the fragment dead.
	* call the methods of attributes with ->frag_sem taken
shared and only after having verified that the fragment is still
alive.

	The main benefit is for method instances - they are
guaranteed that the objects they are accessing *and* all ancestors
are still there.  Another win is that we don't need to bother
with extra refcount on config_item when opening a file -
the item will be alive for as long as it stays in the tree, and
we won't touch it/attributes/any associated data after it's
been removed from the tree.

Signed-off-by: Al Viro <[email protected]>
Signed-off-by: Christoph Hellwig <[email protected]>
  • Loading branch information
Al Viro authored and Christoph Hellwig committed Sep 4, 2019
1 parent 47320fb commit b0841ee
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 18 deletions.
23 changes: 23 additions & 0 deletions fs/configfs/dir.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1461,6 +1461,7 @@ static int configfs_rmdir(struct inode *dir, struct dentry *dentry)
struct config_item *item;
struct configfs_subsystem *subsys;
struct configfs_dirent *sd;
struct configfs_fragment *frag;
struct module *subsys_owner = NULL, *dead_item_owner = NULL;
int ret;

Expand Down Expand Up @@ -1518,6 +1519,16 @@ static int configfs_rmdir(struct inode *dir, struct dentry *dentry)
}
} while (ret == -EAGAIN);

frag = sd->s_frag;
if (down_write_killable(&frag->frag_sem)) {
spin_lock(&configfs_dirent_lock);
configfs_detach_rollback(dentry);
spin_unlock(&configfs_dirent_lock);
return -EINTR;
}
frag->frag_dead = true;
up_write(&frag->frag_sem);

/* Get a working ref for the duration of this function */
item = configfs_get_config_item(dentry);

Expand Down Expand Up @@ -1821,6 +1832,12 @@ void configfs_unregister_group(struct config_group *group)
struct configfs_subsystem *subsys = group->cg_subsys;
struct dentry *dentry = group->cg_item.ci_dentry;
struct dentry *parent = group->cg_item.ci_parent->ci_dentry;
struct configfs_dirent *sd = dentry->d_fsdata;
struct configfs_fragment *frag = sd->s_frag;

down_write(&frag->frag_sem);
frag->frag_dead = true;
up_write(&frag->frag_sem);

inode_lock_nested(d_inode(parent), I_MUTEX_PARENT);
spin_lock(&configfs_dirent_lock);
Expand Down Expand Up @@ -1947,12 +1964,18 @@ void configfs_unregister_subsystem(struct configfs_subsystem *subsys)
struct config_group *group = &subsys->su_group;
struct dentry *dentry = group->cg_item.ci_dentry;
struct dentry *root = dentry->d_sb->s_root;
struct configfs_dirent *sd = dentry->d_fsdata;
struct configfs_fragment *frag = sd->s_frag;

if (dentry->d_parent != root) {
pr_err("Tried to unregister non-subsystem!\n");
return;
}

down_write(&frag->frag_sem);
frag->frag_dead = true;
up_write(&frag->frag_sem);

inode_lock_nested(d_inode(root),
I_MUTEX_PARENT);
inode_lock_nested(d_inode(dentry), I_MUTEX_CHILD);
Expand Down
75 changes: 57 additions & 18 deletions fs/configfs/file.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,22 +48,32 @@ struct configfs_buffer {
};
};

static inline struct configfs_fragment *to_frag(struct file *file)
{
struct configfs_dirent *sd = file->f_path.dentry->d_fsdata;

return sd->s_frag;
}

static int fill_read_buffer(struct configfs_buffer * buffer)
static int fill_read_buffer(struct file *file, struct configfs_buffer *buffer)
{
ssize_t count;
struct configfs_fragment *frag = to_frag(file);
ssize_t count = -ENOENT;

if (!buffer->page)
buffer->page = (char *) get_zeroed_page(GFP_KERNEL);
if (!buffer->page)
return -ENOMEM;

count = buffer->attr->show(buffer->item, buffer->page);
down_read(&frag->frag_sem);
if (!frag->frag_dead)
count = buffer->attr->show(buffer->item, buffer->page);
up_read(&frag->frag_sem);

if (count < 0)
return count;
if (WARN_ON_ONCE(count > (ssize_t)SIMPLE_ATTR_SIZE))
return -EIO;

buffer->needs_read_fill = 0;
buffer->count = count;
return 0;
Expand Down Expand Up @@ -96,7 +106,7 @@ configfs_read_file(struct file *file, char __user *buf, size_t count, loff_t *pp

mutex_lock(&buffer->mutex);
if (buffer->needs_read_fill) {
retval = fill_read_buffer(buffer);
retval = fill_read_buffer(file, buffer);
if (retval)
goto out;
}
Expand Down Expand Up @@ -133,6 +143,7 @@ static ssize_t
configfs_read_bin_file(struct file *file, char __user *buf,
size_t count, loff_t *ppos)
{
struct configfs_fragment *frag = to_frag(file);
struct configfs_buffer *buffer = file->private_data;
ssize_t retval = 0;
ssize_t len = min_t(size_t, count, PAGE_SIZE);
Expand All @@ -148,7 +159,12 @@ configfs_read_bin_file(struct file *file, char __user *buf,

if (buffer->needs_read_fill) {
/* perform first read with buf == NULL to get extent */
len = buffer->bin_attr->read(buffer->item, NULL, 0);
down_read(&frag->frag_sem);
if (!frag->frag_dead)
len = buffer->bin_attr->read(buffer->item, NULL, 0);
else
len = -ENOENT;
up_read(&frag->frag_sem);
if (len <= 0) {
retval = len;
goto out;
Expand All @@ -168,8 +184,13 @@ configfs_read_bin_file(struct file *file, char __user *buf,
buffer->bin_buffer_size = len;

/* perform second read to fill buffer */
len = buffer->bin_attr->read(buffer->item,
buffer->bin_buffer, len);
down_read(&frag->frag_sem);
if (!frag->frag_dead)
len = buffer->bin_attr->read(buffer->item,
buffer->bin_buffer, len);
else
len = -ENOENT;
up_read(&frag->frag_sem);
if (len < 0) {
retval = len;
vfree(buffer->bin_buffer);
Expand Down Expand Up @@ -220,9 +241,16 @@ fill_write_buffer(struct configfs_buffer * buffer, const char __user * buf, size
}

static int
flush_write_buffer(struct configfs_buffer *buffer, size_t count)
flush_write_buffer(struct file *file, struct configfs_buffer *buffer, size_t count)
{
return buffer->attr->store(buffer->item, buffer->page, count);
struct configfs_fragment *frag = to_frag(file);
int res = -ENOENT;

down_read(&frag->frag_sem);
if (!frag->frag_dead)
res = buffer->attr->store(buffer->item, buffer->page, count);
up_read(&frag->frag_sem);
return res;
}


Expand Down Expand Up @@ -252,7 +280,7 @@ configfs_write_file(struct file *file, const char __user *buf, size_t count, lof
mutex_lock(&buffer->mutex);
len = fill_write_buffer(buffer, buf, count);
if (len > 0)
len = flush_write_buffer(buffer, len);
len = flush_write_buffer(file, buffer, len);
if (len > 0)
*ppos += len;
mutex_unlock(&buffer->mutex);
Expand Down Expand Up @@ -328,6 +356,7 @@ configfs_write_bin_file(struct file *file, const char __user *buf,
static int __configfs_open_file(struct inode *inode, struct file *file, int type)
{
struct dentry *dentry = file->f_path.dentry;
struct configfs_fragment *frag = to_frag(file);
struct configfs_attribute *attr;
struct configfs_buffer *buffer;
int error;
Expand All @@ -337,8 +366,13 @@ static int __configfs_open_file(struct inode *inode, struct file *file, int type
if (!buffer)
goto out;

error = -ENOENT;
down_read(&frag->frag_sem);
if (unlikely(frag->frag_dead))
goto out_free_buffer;

error = -EINVAL;
buffer->item = configfs_get_config_item(dentry->d_parent);
buffer->item = to_item(dentry->d_parent);
if (!buffer->item)
goto out_free_buffer;

Expand Down Expand Up @@ -396,13 +430,15 @@ static int __configfs_open_file(struct inode *inode, struct file *file, int type
buffer->read_in_progress = false;
buffer->write_in_progress = false;
file->private_data = buffer;
up_read(&frag->frag_sem);
return 0;

out_put_module:
module_put(buffer->owner);
out_put_item:
config_item_put(buffer->item);
out_free_buffer:
up_read(&frag->frag_sem);
kfree(buffer);
out:
return error;
Expand All @@ -412,8 +448,6 @@ static int configfs_release(struct inode *inode, struct file *filp)
{
struct configfs_buffer *buffer = filp->private_data;

if (buffer->item)
config_item_put(buffer->item);
module_put(buffer->owner);
if (buffer->page)
free_page((unsigned long)buffer->page);
Expand All @@ -439,12 +473,17 @@ static int configfs_release_bin_file(struct inode *inode, struct file *file)
buffer->read_in_progress = false;

if (buffer->write_in_progress) {
struct configfs_fragment *frag = to_frag(file);
buffer->write_in_progress = false;

/* result of ->release() is ignored */
buffer->bin_attr->write(buffer->item, buffer->bin_buffer,
buffer->bin_buffer_size);

down_read(&frag->frag_sem);
if (!frag->frag_dead) {
/* result of ->release() is ignored */
buffer->bin_attr->write(buffer->item,
buffer->bin_buffer,
buffer->bin_buffer_size);
}
up_read(&frag->frag_sem);
/* vfree on NULL is safe */
vfree(buffer->bin_buffer);
buffer->bin_buffer = NULL;
Expand Down

0 comments on commit b0841ee

Please sign in to comment.