| Message ID | 20191209070609.GA32438@ircssh-2.c.rugged-nimbus-611.internal (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add ptrace get_fd request | expand |
On 12/09, Sargun Dhillon wrote: > > +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/ 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);
diff --git a/fs/file.c b/fs/file.c index 3da91a112bab..98601a503a0f 100644 --- a/fs/file.c +++ b/fs/file.c @@ -1015,3 +1015,22 @@ int iterate_fd(struct files_struct *files, unsigned n, return res; } EXPORT_SYMBOL(iterate_fd); + +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; + } + + rcu_read_unlock(); + task_unlock(task); + + return file; +} diff --git a/include/linux/fdtable.h b/include/linux/fdtable.h index f07c55ea0c22..eacb1a56df44 100644 --- a/include/linux/fdtable.h +++ b/include/linux/fdtable.h @@ -115,6 +115,16 @@ int iterate_fd(struct files_struct *, unsigned, int (*)(const void *, struct file *, unsigned), const void *); +/* + * get_task_file - get a reference to a file from another task + * @task: the task to get the file descriptor from + * @fd: the file descriptor number to fetch + * + * returns NULL on failure, or pointer to the file on success, with a reference + * It requires that the task is pinned prior to calling it. + */ +struct file *get_task_file(struct task_struct *task, unsigned int fd); + extern int __alloc_fd(struct files_struct *files, unsigned start, unsigned end, unsigned flags); extern void __fd_install(struct files_struct *files,
This introduces a function which can be used to fetch a file, given an arbitrary task. As long as the user holds a reference (refcnt) to the task_struct it is safe to call, and will either return NULL on failure, or a pointer to the file, with a refcnt. Signed-off-by: Sargun Dhillon <sargun@sargun.me> --- fs/file.c | 19 +++++++++++++++++++ include/linux/fdtable.h | 10 ++++++++++ 2 files changed, 29 insertions(+)