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@xxxxxxxxxx>
> Cc: Kees Cook <keescook@xxxxxxxxxxxx>
> Cc: Christian Brauner <brauner@xxxxxxxxxx>
> Cc: Jeff Xu <jeffxu@xxxxxxxxxx>
> Signed-off-by: Adrian Ratiu <adrian.ratiu@xxxxxxxxxxxxx>
> Reviewed-by: Kees Cook <kees@xxxxxxxxxx>
> ---
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)?
> struct mm_struct *mm = ERR_PTR(-ESRCH);
>
> if (task) {
> @@ -816,7 +816,7 @@ struct mm_struct *proc_mem_open(struct inode *inode, unsigned int mode)
>
> static int __mem_open(struct inode *inode, struct file *file, unsigned int mode)
> {
> - struct mm_struct *mm = proc_mem_open(inode, mode);
> + struct mm_struct *mm = proc_mem_open(file, mode);
>
> if (IS_ERR(mm))
> return PTR_ERR(mm);
> diff --git a/fs/proc/internal.h b/fs/proc/internal.h
> index a71ac5379584..d38b2eea40d1 100644
> --- a/fs/proc/internal.h
> +++ b/fs/proc/internal.h
> @@ -295,7 +295,7 @@ struct proc_maps_private {
> #endif
> } __randomize_layout;
>
> -struct mm_struct *proc_mem_open(struct inode *inode, unsigned int mode);
> +struct mm_struct *proc_mem_open(struct file *file, unsigned int mode);
>
> extern const struct file_operations proc_pid_maps_operations;
> extern const struct file_operations proc_pid_numa_maps_operations;
> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> index f8d35f993fe5..fe3b2182b0aa 100644
> --- a/fs/proc/task_mmu.c
> +++ b/fs/proc/task_mmu.c
> @@ -210,7 +210,7 @@ static int proc_maps_open(struct inode *inode, struct file *file,
> return -ENOMEM;
>
> priv->inode = inode;
> - priv->mm = proc_mem_open(inode, PTRACE_MODE_READ);
> + priv->mm = proc_mem_open(file, PTRACE_MODE_READ);
> if (IS_ERR(priv->mm)) {
> int err = PTR_ERR(priv->mm);
>
> @@ -1030,7 +1030,7 @@ static int smaps_rollup_open(struct inode *inode, struct file *file)
> goto out_free;
>
> priv->inode = inode;
> - priv->mm = proc_mem_open(inode, PTRACE_MODE_READ);
> + priv->mm = proc_mem_open(file, PTRACE_MODE_READ);
> if (IS_ERR(priv->mm)) {
> ret = PTR_ERR(priv->mm);
>
> @@ -1754,7 +1754,7 @@ static int pagemap_open(struct inode *inode, struct file *file)
> {
> struct mm_struct *mm;
>
> - mm = proc_mem_open(inode, PTRACE_MODE_READ);
> + mm = proc_mem_open(file, PTRACE_MODE_READ);
> if (IS_ERR(mm))
> return PTR_ERR(mm);
> file->private_data = mm;
> diff --git a/fs/proc/task_nommu.c b/fs/proc/task_nommu.c
> index bce674533000..a8ab182a4ed1 100644
> --- a/fs/proc/task_nommu.c
> +++ b/fs/proc/task_nommu.c
> @@ -259,7 +259,7 @@ static int maps_open(struct inode *inode, struct file *file,
> return -ENOMEM;
>
> priv->inode = inode;
> - priv->mm = proc_mem_open(inode, PTRACE_MODE_READ);
> + priv->mm = proc_mem_open(file, PTRACE_MODE_READ);
> if (IS_ERR(priv->mm)) {
> int err = PTR_ERR(priv->mm);
>
> --
> 2.44.2
>
