Skip to content

Commit

Permalink
[PATCH] FUSE: don't allow restarting of system calls
Browse files Browse the repository at this point in the history 
This patch removes ability to interrupt and restart operations while there
hasn't been any side-effect.

The reason: applications.  There are some apps it seems that generate
signals at a fast rate.  This means, that if the operation cannot make
enough progress between two signals, it will be restarted for ever.  This
bug actually manifested itself with 'krusader' trying to open a file for
writing under sshfs.  Thanks to Eduard Czimbalmos for the report.

The problem can be solved just by making open() uninterruptible, because in
this case it was the truncate operation that slowed down the progress.  But
it's better to solve this by simply not allowing interrupts at all (except
SIGKILL), because applications don't expect file operations to be
interruptible anyway.  As an added bonus the code is simplified somewhat.

Signed-off-by: Miklos Szeredi <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
  • Loading branch information
@szmi
szmi authored and Linus Torvalds committed Sep 9, 2005
1 parent 8254798 commit 7c352bd
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 111 deletions.
73 changes: 13 additions & 60 deletions fs/fuse/dev.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,19 +103,8 @@ static struct fuse_req *do_get_request(struct fuse_conn *fc)
return req;
}

/* This can return NULL, but only in case it's interrupted by a SIGKILL */
struct fuse_req *fuse_get_request(struct fuse_conn *fc)
{
if (down_interruptible(&fc->outstanding_sem))
return NULL;
return do_get_request(fc);
}

/*
* Non-interruptible version of the above function is for operations
* which can't legally return -ERESTART{SYS,NOINTR}. This can still
* return NULL, but only in case the signal is SIGKILL.
*/
struct fuse_req *fuse_get_request_nonint(struct fuse_conn *fc)
{
int intr;
sigset_t oldset;
Expand Down Expand Up @@ -241,43 +230,20 @@ static void background_request(struct fuse_conn *fc, struct fuse_req *req)
get_file(req->file);
}

static int request_wait_answer_nonint(struct fuse_req *req)
{
int err;
sigset_t oldset;
block_sigs(&oldset);
err = wait_event_interruptible(req->waitq, req->finished);
restore_sigs(&oldset);
return err;
}

/* Called with fuse_lock held. Releases, and then reacquires it. */
static void request_wait_answer(struct fuse_conn *fc, struct fuse_req *req,
int interruptible)
static void request_wait_answer(struct fuse_conn *fc, struct fuse_req *req)
{
int intr;
sigset_t oldset;

spin_unlock(&fuse_lock);
if (interruptible)
intr = wait_event_interruptible(req->waitq, req->finished);
else
intr = request_wait_answer_nonint(req);
block_sigs(&oldset);
wait_event_interruptible(req->waitq, req->finished);
restore_sigs(&oldset);
spin_lock(&fuse_lock);
if (intr && interruptible && req->sent) {
/* If request is already in userspace, only allow KILL
signal to interrupt */
spin_unlock(&fuse_lock);
intr = request_wait_answer_nonint(req);
spin_lock(&fuse_lock);
}
if (!intr)
if (req->finished)
return;

if (!interruptible || req->sent)
req->out.h.error = -EINTR;
else
req->out.h.error = -ERESTARTNOINTR;

req->out.h.error = -EINTR;
req->interrupted = 1;
if (req->locked) {
/* This is uninterruptible sleep, because data is
Expand Down Expand Up @@ -330,8 +296,10 @@ static void queue_request(struct fuse_conn *fc, struct fuse_req *req)
wake_up(&fc->waitq);
}

static void request_send_wait(struct fuse_conn *fc, struct fuse_req *req,
int interruptible)
/*
* This can only be interrupted by a SIGKILL
*/
void request_send(struct fuse_conn *fc, struct fuse_req *req)
{
req->isreply = 1;
spin_lock(&fuse_lock);
Expand All @@ -345,26 +313,11 @@ static void request_send_wait(struct fuse_conn *fc, struct fuse_req *req,
after request_end() */
__fuse_get_request(req);

request_wait_answer(fc, req, interruptible);
request_wait_answer(fc, req);
}
spin_unlock(&fuse_lock);
}

void request_send(struct fuse_conn *fc, struct fuse_req *req)
{
request_send_wait(fc, req, 1);
}

/*
* Non-interruptible version of the above function is for operations
* which can't legally return -ERESTART{SYS,NOINTR}. This can still
* be interrupted but only with SIGKILL.
*/
void request_send_nonint(struct fuse_conn *fc, struct fuse_req *req)
{
request_send_wait(fc, req, 0);
}

static void request_send_nowait(struct fuse_conn *fc, struct fuse_req *req)
{
spin_lock(&fuse_lock);
Expand Down
36 changes: 18 additions & 18 deletions fs/fuse/dir.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,12 @@ static int fuse_dentry_revalidate(struct dentry *entry, struct nameidata *nd)
struct inode *inode = entry->d_inode;
struct fuse_inode *fi = get_fuse_inode(inode);
struct fuse_conn *fc = get_fuse_conn(inode);
struct fuse_req *req = fuse_get_request_nonint(fc);
struct fuse_req *req = fuse_get_request(fc);
if (!req)
return 0;

fuse_lookup_init(req, entry->d_parent->d_inode, entry, &outarg);
request_send_nonint(fc, req);
request_send(fc, req);
err = req->out.h.error;
if (!err) {
if (outarg.nodeid != get_node_id(inode)) {
Expand Down Expand Up @@ -91,7 +91,7 @@ static int fuse_lookup_iget(struct inode *dir, struct dentry *entry,

req = fuse_get_request(fc);
if (!req)
return -ERESTARTNOINTR;
return -EINTR;

fuse_lookup_init(req, dir, entry, &outarg);
request_send(fc, req);
Expand Down Expand Up @@ -185,7 +185,7 @@ static int fuse_mknod(struct inode *dir, struct dentry *entry, int mode,
struct fuse_conn *fc = get_fuse_conn(dir);
struct fuse_req *req = fuse_get_request(fc);
if (!req)
return -ERESTARTNOINTR;
return -EINTR;

memset(&inarg, 0, sizeof(inarg));
inarg.mode = mode;
Expand All @@ -211,7 +211,7 @@ static int fuse_mkdir(struct inode *dir, struct dentry *entry, int mode)
struct fuse_conn *fc = get_fuse_conn(dir);
struct fuse_req *req = fuse_get_request(fc);
if (!req)
return -ERESTARTNOINTR;
return -EINTR;

memset(&inarg, 0, sizeof(inarg));
inarg.mode = mode;
Expand All @@ -236,7 +236,7 @@ static int fuse_symlink(struct inode *dir, struct dentry *entry,

req = fuse_get_request(fc);
if (!req)
return -ERESTARTNOINTR;
return -EINTR;

req->in.h.opcode = FUSE_SYMLINK;
req->in.numargs = 2;
Expand All @@ -253,7 +253,7 @@ static int fuse_unlink(struct inode *dir, struct dentry *entry)
struct fuse_conn *fc = get_fuse_conn(dir);
struct fuse_req *req = fuse_get_request(fc);
if (!req)
return -ERESTARTNOINTR;
return -EINTR;

req->in.h.opcode = FUSE_UNLINK;
req->in.h.nodeid = get_node_id(dir);
Expand Down Expand Up @@ -284,7 +284,7 @@ static int fuse_rmdir(struct inode *dir, struct dentry *entry)
struct fuse_conn *fc = get_fuse_conn(dir);
struct fuse_req *req = fuse_get_request(fc);
if (!req)
return -ERESTARTNOINTR;
return -EINTR;

req->in.h.opcode = FUSE_RMDIR;
req->in.h.nodeid = get_node_id(dir);
Expand All @@ -311,7 +311,7 @@ static int fuse_rename(struct inode *olddir, struct dentry *oldent,
struct fuse_conn *fc = get_fuse_conn(olddir);
struct fuse_req *req = fuse_get_request(fc);
if (!req)
return -ERESTARTNOINTR;
return -EINTR;

memset(&inarg, 0, sizeof(inarg));
inarg.newdir = get_node_id(newdir);
Expand Down Expand Up @@ -356,7 +356,7 @@ static int fuse_link(struct dentry *entry, struct inode *newdir,
struct fuse_conn *fc = get_fuse_conn(inode);
struct fuse_req *req = fuse_get_request(fc);
if (!req)
return -ERESTARTNOINTR;
return -EINTR;

memset(&inarg, 0, sizeof(inarg));
inarg.oldnodeid = get_node_id(inode);
Expand Down Expand Up @@ -386,7 +386,7 @@ int fuse_do_getattr(struct inode *inode)
struct fuse_conn *fc = get_fuse_conn(inode);
struct fuse_req *req = fuse_get_request(fc);
if (!req)
return -ERESTARTNOINTR;
return -EINTR;

req->in.h.opcode = FUSE_GETATTR;
req->in.h.nodeid = get_node_id(inode);
Expand Down Expand Up @@ -533,7 +533,7 @@ static int fuse_readdir(struct file *file, void *dstbuf, filldir_t filldir)
struct page *page;
struct inode *inode = file->f_dentry->d_inode;
struct fuse_conn *fc = get_fuse_conn(inode);
struct fuse_req *req = fuse_get_request_nonint(fc);
struct fuse_req *req = fuse_get_request(fc);
if (!req)
return -EINTR;

Expand Down Expand Up @@ -564,7 +564,7 @@ static char *read_link(struct dentry *dentry)
char *link;

if (!req)
return ERR_PTR(-ERESTARTNOINTR);
return ERR_PTR(-EINTR);

link = (char *) __get_free_page(GFP_KERNEL);
if (!link) {
Expand Down Expand Up @@ -677,7 +677,7 @@ static int fuse_setattr(struct dentry *entry, struct iattr *attr)

req = fuse_get_request(fc);
if (!req)
return -ERESTARTNOINTR;
return -EINTR;

memset(&inarg, 0, sizeof(inarg));
inarg.valid = iattr_to_fattr(attr, &inarg.attr);
Expand Down Expand Up @@ -761,7 +761,7 @@ static int fuse_setxattr(struct dentry *entry, const char *name,

req = fuse_get_request(fc);
if (!req)
return -ERESTARTNOINTR;
return -EINTR;

memset(&inarg, 0, sizeof(inarg));
inarg.size = size;
Expand Down Expand Up @@ -801,7 +801,7 @@ static ssize_t fuse_getxattr(struct dentry *entry, const char *name,

req = fuse_get_request(fc);
if (!req)
return -ERESTARTNOINTR;
return -EINTR;

memset(&inarg, 0, sizeof(inarg));
inarg.size = size;
Expand Down Expand Up @@ -851,7 +851,7 @@ static ssize_t fuse_listxattr(struct dentry *entry, char *list, size_t size)

req = fuse_get_request(fc);
if (!req)
return -ERESTARTNOINTR;
return -EINTR;

memset(&inarg, 0, sizeof(inarg));
inarg.size = size;
Expand Down Expand Up @@ -897,7 +897,7 @@ static int fuse_removexattr(struct dentry *entry, const char *name)

req = fuse_get_request(fc);
if (!req)
return -ERESTARTNOINTR;
return -EINTR;

req->in.h.opcode = FUSE_REMOVEXATTR;
req->in.h.nodeid = get_node_id(inode);
Expand Down
33 changes: 12 additions & 21 deletions fs/fuse/file.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,6 @@ int fuse_open_common(struct inode *inode, struct file *file, int isdir)
struct fuse_open_out outarg;
struct fuse_file *ff;
int err;
/* Restarting the syscall is not allowed if O_CREAT and O_EXCL
are both set, because creation will fail on the restart */
int excl = (file->f_flags & (O_CREAT|O_EXCL)) == (O_CREAT|O_EXCL);

err = generic_file_open(inode, file);
if (err)
Expand All @@ -38,12 +35,9 @@ int fuse_open_common(struct inode *inode, struct file *file, int isdir)
return err;
}

if (excl)
req = fuse_get_request_nonint(fc);
else
req = fuse_get_request(fc);
req = fuse_get_request(fc);
if (!req)
return excl ? -EINTR : -ERESTARTSYS;
return -EINTR;

err = -ENOMEM;
ff = kmalloc(sizeof(struct fuse_file), GFP_KERNEL);
Expand All @@ -67,10 +61,7 @@ int fuse_open_common(struct inode *inode, struct file *file, int isdir)
req->out.numargs = 1;
req->out.args[0].size = sizeof(outarg);
req->out.args[0].value = &outarg;
if (excl)
request_send_nonint(fc, req);
else
request_send(fc, req);
request_send(fc, req);
err = req->out.h.error;
if (err) {
fuse_request_free(ff->release_req);
Expand Down Expand Up @@ -133,7 +124,7 @@ static int fuse_flush(struct file *file)
if (fc->no_flush)
return 0;

req = fuse_get_request_nonint(fc);
req = fuse_get_request(fc);
if (!req)
return -EINTR;

Expand All @@ -146,7 +137,7 @@ static int fuse_flush(struct file *file)
req->in.numargs = 1;
req->in.args[0].size = sizeof(inarg);
req->in.args[0].value = &inarg;
request_send_nonint(fc, req);
request_send(fc, req);
err = req->out.h.error;
fuse_put_request(fc, req);
if (err == -ENOSYS) {
Expand All @@ -171,7 +162,7 @@ int fuse_fsync_common(struct file *file, struct dentry *de, int datasync,

req = fuse_get_request(fc);
if (!req)
return -ERESTARTSYS;
return -EINTR;

memset(&inarg, 0, sizeof(inarg));
inarg.fh = ff->fh;
Expand Down Expand Up @@ -224,7 +215,7 @@ size_t fuse_send_read_common(struct fuse_req *req, struct file *file,
req->out.argvar = 1;
req->out.numargs = 1;
req->out.args[0].size = count;
request_send_nonint(fc, req);
request_send(fc, req);
return req->out.args[0].size;
}

Expand All @@ -240,7 +231,7 @@ static int fuse_readpage(struct file *file, struct page *page)
struct inode *inode = page->mapping->host;
struct fuse_conn *fc = get_fuse_conn(inode);
loff_t pos = (loff_t) page->index << PAGE_CACHE_SHIFT;
struct fuse_req *req = fuse_get_request_nonint(fc);
struct fuse_req *req = fuse_get_request(fc);
int err = -EINTR;
if (!req)
goto out;
Expand Down Expand Up @@ -314,7 +305,7 @@ static int fuse_readpages(struct file *file, struct address_space *mapping,
int err;
data.file = file;
data.inode = inode;
data.req = fuse_get_request_nonint(fc);
data.req = fuse_get_request(fc);
if (!data.req)
return -EINTR;

Expand Down Expand Up @@ -350,7 +341,7 @@ static size_t fuse_send_write(struct fuse_req *req, struct file *file,
req->out.numargs = 1;
req->out.args[0].size = sizeof(struct fuse_write_out);
req->out.args[0].value = &outarg;
request_send_nonint(fc, req);
request_send(fc, req);
return outarg.size;
}

Expand All @@ -370,7 +361,7 @@ static int fuse_commit_write(struct file *file, struct page *page,
struct inode *inode = page->mapping->host;
struct fuse_conn *fc = get_fuse_conn(inode);
loff_t pos = ((loff_t) page->index << PAGE_CACHE_SHIFT) + offset;
struct fuse_req *req = fuse_get_request_nonint(fc);
struct fuse_req *req = fuse_get_request(fc);
if (!req)
return -EINTR;

Expand Down Expand Up @@ -444,7 +435,7 @@ static ssize_t fuse_direct_io(struct file *file, const char __user *buf,
ssize_t res = 0;
struct fuse_req *req = fuse_get_request(fc);
if (!req)
return -ERESTARTSYS;
return -EINTR;

while (count) {
size_t tmp;
Expand Down
Loading

0 comments on commit 7c352bd

Please sign in to comment.