Messages in this thread Patch in this message | | | Subject | [PATCH] fs/file.c: __fget() and dup2() atomicity rules | From | Eric Dumazet <> | Date | Mon, 29 Jun 2015 17:10:30 +0200 |
| |
From: Eric Dumazet <edumazet@google.com>
__fget() makes sure a file refcount is not zero before taking a reference. It should also fetch again file pointer in order to respect dup2() atomicity requirements.
It should either read a NULL pointer or a file on which a refcount can be taken.
Dmitry had following test failing sometimes :
#include <pthread.h> #include <stdio.h> #include <unistd.h> #include <errno.h> #include <fcntl.h>
int fd; void *Thread(void *x) { char buf; int n = read(fd, &buf, 1); if (n != 1) exit(printf("read failed: n=%d errno=%d\n", n, errno)); return 0; }
int main() { fd = open("/dev/urandom", O_RDONLY); int fd2 = open("/dev/urandom", O_RDONLY); if (fd == -1 || fd2 == -1) exit(printf("open failed\n")); pthread_t th; pthread_create(&th, 0, Thread, 0); if (dup2(fd2, fd) == -1) exit(printf("dup2 failed\n")); pthread_join(th, 0); if (close(fd) == -1) exit(printf("close failed\n")); if (close(fd2) == -1) exit(printf("close failed\n")); printf("DONE\n"); return 0; }
Signed-off-by: Eric Dumazet <edumazet@google.com> Reported-by: Dmitry Vyukov <dvyukov@google.com> --- fs/file.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/fs/file.c b/fs/file.c index 93c5f89c248b..492bd74c4433 100644 --- a/fs/file.c +++ b/fs/file.c @@ -635,11 +635,17 @@ static struct file *__fget(unsigned int fd, fmode_t mask) struct file *file; rcu_read_lock(); +loop: file = fcheck_files(files, fd); if (file) { - /* File object ref couldn't be taken */ - if ((file->f_mode & mask) || !get_file_rcu(file)) + /* File object ref couldn't be taken. + * dup2() atomicity guarantee is the reason + * we loop to catch the new file (or NULL pointer) + */ + if (file->f_mode & mask) file = NULL; + else if (!get_file_rcu(file)) + goto loop; } rcu_read_unlock();
| |