Re: [PATCH v6 1/2] proc: pass file instead of inode to proc_mem_open
Adrian Ratiu
adrian.ratiu at collabora.com
Mon Jun 17 10:47:03 UTC 2024
On Monday, June 17, 2024 11:48 EEST, Christian Brauner <brauner at kernel.org> wrote:
> On Thu, Jun 13, 2024 at 04:39:36PM GMT, Adrian Ratiu wrote:
> > The file struct is required in proc_mem_open() so its
> > f_mode can be checked when deciding whether to allow or
> > deny /proc/*/mem open requests via the new read/write
> > and foll_force restriction mechanism.
> >
> > Thus instead of directly passing the inode to the fun,
> > we pass the file and get the inode inside it.
> >
> > Cc: Jann Horn <jannh at google.com>
> > Cc: Kees Cook <keescook at chromium.org>
> > Cc: Christian Brauner <brauner at kernel.org>
> > Cc: Jeff Xu <jeffxu at google.com>
> > Signed-off-by: Adrian Ratiu <adrian.ratiu at collabora.com>
> > Reviewed-by: Kees Cook <kees at kernel.org>
> > ---
>
> I've tentatively applies this patch to #vfs.procfs.
> One comment, one question:
>
> > No changes in v6
> > ---
> > fs/proc/base.c | 6 +++---
> > fs/proc/internal.h | 2 +-
> > fs/proc/task_mmu.c | 6 +++---
> > fs/proc/task_nommu.c | 2 +-
> > 4 files changed, 8 insertions(+), 8 deletions(-)
> >
> > diff --git a/fs/proc/base.c b/fs/proc/base.c
> > index 72a1acd03675..4c607089f66e 100644
> > --- a/fs/proc/base.c
> > +++ b/fs/proc/base.c
> > @@ -794,9 +794,9 @@ static const struct file_operations proc_single_file_operations = {
> > };
> >
> >
> > -struct mm_struct *proc_mem_open(struct inode *inode, unsigned int mode)
> > +struct mm_struct *proc_mem_open(struct file *file, unsigned int mode)
> > {
> > - struct task_struct *task = get_proc_task(inode);
> > + struct task_struct *task = get_proc_task(file->f_inode);
>
> Comment: This should use file_inode(file) but I've just fixed that when I
> applied.
>
> Question: Is this an equivalent transformation. So is the inode that was
> passed to proc_mem_open() always the same inode as file_inode(file)?
Thank you!
Yes, the inode associated with the file struct should be always the same
while the file is opened, so the link set during the top-level mem_open()
callback should still hold while it itself calls into its sub-functions like
proc_mem_open().
More information about the Linux-security-module-archive
mailing list