Re: [PATCH v2 1/4] vfs, fdtable: Add get_task_file helper
flat view
From: Oleg Nesterov <oleg@redhat.com>
Date: 2019-12-09 15:52:58
Also in:
linux-fsdevel, lkml
On 12/09, Sargun Dhillon wrote:
quoted hunk ↗ jump to hunk
+struct file *get_task_file(struct task_struct *task, unsigned int fd) +{ + struct file *file = NULL; + + task_lock(task); + rcu_read_lock(); + + if (task->files) { + file = fcheck_files(task->files, fd); + if (file && !get_file_rcu(file)) + file = NULL; + }
On second thought this is not exactly right, get_file_rcu() can fail if get_task_file() races with dup2(), in this case we need to do fcheck_files() again. And this is what __fget() already does, so may be the patch below makes more sense? I will leave this to other reviewers, but suddenly I recall that I have already sent the patch which adds a similar helper a while ago. See https://lore.kernel.org/lkml/20180915160423.GA31461@redhat.com/ (local) In short, get_files_struct() should be avoided because it can race with exec() and break POSIX locks which use ->fl_owner = files_struct. Oleg.
--- x/fs/file.c
+++ x/fs/file.c@@ -706,9 +706,9 @@ void do_close_on_exec(struct files_struc spin_unlock(&files->file_lock); } -static struct file *__fget(unsigned int fd, fmode_t mask, unsigned int refs) +static struct file *__fget_files(struct files_struct *files, unsigned int fd, + fmode_t mask, unsigned int refs) { - struct files_struct *files = current->files; struct file *file; rcu_read_lock();
@@ -729,6 +729,23 @@ loop: return file; } +struct file *fget_task(struct task_struct *task, unsigned int fd) +{ + struct file *file; + + task_lock(task); + if (task->files) + file = __fget_files(task->files, fd, 0, 1); + task_unlock(task); + + return file; +} + +static struct file *__fget(unsigned int fd, fmode_t mask, unsigned int refs) +{ + return __fget_files(current->files, fd, mask, refs); +} + struct file *fget_many(unsigned int fd, unsigned int refs) { return __fget(fd, FMODE_PATH, refs);