aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--Documentation/filesystems/Locking7
-rw-r--r--fs/fcntl.c29
-rw-r--r--fs/ioctl.c13
-rw-r--r--net/socket.c7
4 files changed, 29 insertions, 27 deletions
diff --git a/Documentation/filesystems/Locking b/Documentation/filesystems/Locking
index ec6a9392a173..4e78ce677843 100644
--- a/Documentation/filesystems/Locking
+++ b/Documentation/filesystems/Locking
@@ -437,8 +437,11 @@ grab BKL for cases when we close a file that had been opened r/w, but that
437can and should be done using the internal locking with smaller critical areas). 437can and should be done using the internal locking with smaller critical areas).
438Current worst offender is ext2_get_block()... 438Current worst offender is ext2_get_block()...
439 439
440->fasync() is a mess. This area needs a big cleanup and that will probably 440->fasync() is called without BKL protection, and is responsible for
441affect locking. 441maintaining the FASYNC bit in filp->f_flags. Most instances call
442fasync_helper(), which does that maintenance, so it's not normally
443something one needs to worry about. Return values > 0 will be mapped to
444zero in the VFS layer.
442 445
443->readdir() and ->ioctl() on directories must be changed. Ideally we would 446->readdir() and ->ioctl() on directories must be changed. Ideally we would
444move ->readdir() to inode_operations and use a separate method for directory 447move ->readdir() to inode_operations and use a separate method for directory
diff --git a/fs/fcntl.c b/fs/fcntl.c
index 04df8570a2d2..431bb6459273 100644
--- a/fs/fcntl.c
+++ b/fs/fcntl.c
@@ -141,7 +141,7 @@ SYSCALL_DEFINE1(dup, unsigned int, fildes)
141 return ret; 141 return ret;
142} 142}
143 143
144#define SETFL_MASK (O_APPEND | O_NONBLOCK | O_NDELAY | FASYNC | O_DIRECT | O_NOATIME) 144#define SETFL_MASK (O_APPEND | O_NONBLOCK | O_NDELAY | O_DIRECT | O_NOATIME)
145 145
146static int setfl(int fd, struct file * filp, unsigned long arg) 146static int setfl(int fd, struct file * filp, unsigned long arg)
147{ 147{
@@ -177,23 +177,19 @@ static int setfl(int fd, struct file * filp, unsigned long arg)
177 return error; 177 return error;
178 178
179 /* 179 /*
180 * We still need a lock here for now to keep multiple FASYNC calls 180 * ->fasync() is responsible for setting the FASYNC bit.
181 * from racing with each other.
182 */ 181 */
183 lock_kernel(); 182 if (((arg ^ filp->f_flags) & FASYNC) && filp->f_op &&
184 if ((arg ^ filp->f_flags) & FASYNC) { 183 filp->f_op->fasync) {
185 if (filp->f_op && filp->f_op->fasync) { 184 error = filp->f_op->fasync(fd, filp, (arg & FASYNC) != 0);
186 error = filp->f_op->fasync(fd, filp, (arg & FASYNC) != 0); 185 if (error < 0)
187 if (error < 0) 186 goto out;
188 goto out;
189 }
190 } 187 }
191
192 spin_lock(&filp->f_lock); 188 spin_lock(&filp->f_lock);
193 filp->f_flags = (arg & SETFL_MASK) | (filp->f_flags & ~SETFL_MASK); 189 filp->f_flags = (arg & SETFL_MASK) | (filp->f_flags & ~SETFL_MASK);
194 spin_unlock(&filp->f_lock); 190 spin_unlock(&filp->f_lock);
191
195 out: 192 out:
196 unlock_kernel();
197 return error; 193 return error;
198} 194}
199 195
@@ -518,7 +514,7 @@ static DEFINE_RWLOCK(fasync_lock);
518static struct kmem_cache *fasync_cache __read_mostly; 514static struct kmem_cache *fasync_cache __read_mostly;
519 515
520/* 516/*
521 * fasync_helper() is used by some character device drivers (mainly mice) 517 * fasync_helper() is used by almost all character device drivers
522 * to set up the fasync queue. It returns negative on error, 0 if it did 518 * to set up the fasync queue. It returns negative on error, 0 if it did
523 * no changes and positive if it added/deleted the entry. 519 * no changes and positive if it added/deleted the entry.
524 */ 520 */
@@ -557,6 +553,13 @@ int fasync_helper(int fd, struct file * filp, int on, struct fasync_struct **fap
557 result = 1; 553 result = 1;
558 } 554 }
559out: 555out:
556 /* Fix up FASYNC bit while still holding fasync_lock */
557 spin_lock(&filp->f_lock);
558 if (on)
559 filp->f_flags |= FASYNC;
560 else
561 filp->f_flags &= ~FASYNC;
562 spin_unlock(&filp->f_lock);
560 write_unlock_irq(&fasync_lock); 563 write_unlock_irq(&fasync_lock);
561 return result; 564 return result;
562} 565}
diff --git a/fs/ioctl.c b/fs/ioctl.c
index 421aab465dab..e8e89edba576 100644
--- a/fs/ioctl.c
+++ b/fs/ioctl.c
@@ -427,19 +427,11 @@ static int ioctl_fioasync(unsigned int fd, struct file *filp,
427 /* Did FASYNC state change ? */ 427 /* Did FASYNC state change ? */
428 if ((flag ^ filp->f_flags) & FASYNC) { 428 if ((flag ^ filp->f_flags) & FASYNC) {
429 if (filp->f_op && filp->f_op->fasync) 429 if (filp->f_op && filp->f_op->fasync)
430 /* fasync() adjusts filp->f_flags */
430 error = filp->f_op->fasync(fd, filp, on); 431 error = filp->f_op->fasync(fd, filp, on);
431 else 432 else
432 error = -ENOTTY; 433 error = -ENOTTY;
433 } 434 }
434 if (error)
435 return error;
436
437 spin_lock(&filp->f_lock);
438 if (on)
439 filp->f_flags |= FASYNC;
440 else
441 filp->f_flags &= ~FASYNC;
442 spin_unlock(&filp->f_lock);
443 return error; 435 return error;
444} 436}
445 437
@@ -507,10 +499,7 @@ int do_vfs_ioctl(struct file *filp, unsigned int fd, unsigned int cmd,
507 break; 499 break;
508 500
509 case FIOASYNC: 501 case FIOASYNC:
510 /* BKL needed to avoid races tweaking f_flags */
511 lock_kernel();
512 error = ioctl_fioasync(fd, filp, argp); 502 error = ioctl_fioasync(fd, filp, argp);
513 unlock_kernel();
514 break; 503 break;
515 504
516 case FIOQSIZE: 505 case FIOQSIZE:
diff --git a/net/socket.c b/net/socket.c
index 35dd7371752a..0f75746ab06e 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -1030,6 +1030,13 @@ static int sock_fasync(int fd, struct file *filp, int on)
1030 1030
1031 lock_sock(sk); 1031 lock_sock(sk);
1032 1032
1033 spin_lock(&filp->f_lock);
1034 if (on)
1035 filp->f_flags |= FASYNC;
1036 else
1037 filp->f_flags &= ~FASYNC;
1038 spin_unlock(&filp->f_lock);
1039
1033 prev = &(sock->fasync_list); 1040 prev = &(sock->fasync_list);
1034 1041
1035 for (fa = *prev; fa != NULL; prev = &fa->fa_next, fa = *prev) 1042 for (fa = *prev; fa != NULL; prev = &fa->fa_next, fa = *prev)