diff options
| -rw-r--r-- | Documentation/filesystems/Locking | 7 | ||||
| -rw-r--r-- | fs/fcntl.c | 29 | ||||
| -rw-r--r-- | fs/ioctl.c | 13 | ||||
| -rw-r--r-- | net/socket.c | 7 |
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 | |||
| 437 | can and should be done using the internal locking with smaller critical areas). | 437 | can and should be done using the internal locking with smaller critical areas). |
| 438 | Current worst offender is ext2_get_block()... | 438 | Current 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 |
| 441 | affect locking. | 441 | maintaining the FASYNC bit in filp->f_flags. Most instances call |
| 442 | fasync_helper(), which does that maintenance, so it's not normally | ||
| 443 | something one needs to worry about. Return values > 0 will be mapped to | ||
| 444 | zero 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 |
| 444 | move ->readdir() to inode_operations and use a separate method for directory | 447 | move ->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 | ||
| 146 | static int setfl(int fd, struct file * filp, unsigned long arg) | 146 | static 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); | |||
| 518 | static struct kmem_cache *fasync_cache __read_mostly; | 514 | static 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 | } |
| 559 | out: | 555 | out: |
| 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) |
