Re: [PATCH] proc: Fix the issue of proc_mem_open returning NULL

[Mateusz Guzik]

On Mon, Mar 24, 2025 at 09:23:53AM -0700, Penglei Jiang wrote:
> The following functions call proc_mem_open but do not handle the case
> where it returns NULL:
> 
>   __mem_open in fs/proc/base.c
>   proc_maps_open in fs/proc/task_mmu.c
>   smaps_rollup_open in fs/proc/task_mmu.c
>   pagemap_open in fs/proc/task_mmu.c
>   maps_open in fs/proc/task_nommu.c
> 
> The following reported bugs may be related to this issue:
> 
>   https://lore.kernel.org/all/000000000000f52642060d4e3750@xxxxxxxxxx
>   https://lore.kernel.org/all/0000000000001bc4a00612d9a7f4@xxxxxxxxxx
> 
> Fix:
> 
> Modify proc_mem_open to return an error code in case of errors, instead
> of returning NULL.
> 

The rw routines associated with these consumers explictly NULL check
mm, which becomes redundant with the patch.

While I find it fishy that returning NULL was ever a thing to begin
with, it is unclear to me if it can be easily changed now from
userspace-visible behavior standpoint.

I think the best way forward for the time being is to add the missing
NULL checks instead.

> Signed-off-by: Penglei Jiang <superman.xpt@xxxxxxxxx>
> ---
>  fs/proc/base.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/proc/base.c b/fs/proc/base.c
> index cd89e956c322..b5e7317cf0dc 100644
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -840,7 +840,7 @@ struct mm_struct *proc_mem_open(struct inode *inode, unsigned int mode)
>  	put_task_struct(task);
>  
>  	if (IS_ERR(mm))
> -		return mm == ERR_PTR(-ESRCH) ? NULL : mm;
> +		return mm;
>  
>  	/* ensure this mm_struct can't be freed */
>  	mmgrab(mm);
> -- 
> 2.17.1
>