Re: [PATCH 1/3] proc: refactor /proc/$pid/mem to use struct as private_data

On Tue 18-08-26 21:51:05, Jann Horn wrote:
> Refactor the handlers for proc_mem_operations to use the new struct
> mem_private as ->private_data, rather than directly storing an mm_struct*
> in ->private_data.
> 
> This is in preparation for adding more state in mem_private in the next
> commit.
> 
> Signed-off-by: Jann Horn <[email protected]>

Looks good to me. Feel free to add:

Reviewed-by: Jan Kara <[email protected]>

								Honza

> ---
>  fs/proc/base.c | 29 ++++++++++++++++++++++++++---
>  1 file changed, 26 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/proc/base.c b/fs/proc/base.c
> index 780f81259052..bec6197329dc 100644
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -848,11 +848,24 @@ static int __mem_open(struct inode *inode, struct file *file, unsigned int mode)
>  	return 0;
>  }
>  
> +/* private_data for proc_mem_operations */
> +struct mem_private {
> +	struct mm_struct *mm;
> +};
> +
>  static int mem_open(struct inode *inode, struct file *file)
>  {
> +	struct mem_private *priv __free(kfree) = kmalloc_obj(struct mem_private);
> +
> +	if (!priv)
> +		return -ENOMEM;
>  	if (WARN_ON_ONCE(!(file->f_op->fop_flags & FOP_UNSIGNED_OFFSET)))
>  		return -EINVAL;
> -	return __mem_open(inode, file, PTRACE_MODE_ATTACH);
> +	priv->mm = proc_mem_open(inode, PTRACE_MODE_ATTACH);
> +	if (IS_ERR_OR_NULL(priv->mm))
> +		return priv->mm ? PTR_ERR(priv->mm) : -ESRCH;
> +	file->private_data = no_free_ptr(priv);
> +	return 0;
>  }
>  
>  static bool proc_mem_foll_force(struct file *file, struct mm_struct *mm)
> @@ -880,7 +893,8 @@ static bool proc_mem_foll_force(struct file *file, struct mm_struct *mm)
>  static ssize_t mem_rw(struct file *file, char __user *buf,
>  			size_t count, loff_t *ppos, int write)
>  {
> -	struct mm_struct *mm = file->private_data;
> +	struct mem_private *priv = file->private_data;
> +	struct mm_struct *mm = priv->mm;
>  	unsigned long addr = *ppos;
>  	ssize_t copied;
>  	char *page;
> @@ -970,12 +984,21 @@ static int mem_release(struct inode *inode, struct file *file)
>  	return 0;
>  }
>  
> +static int mem_release_with_private(struct inode *inode, struct file *file)
> +{
> +	struct mem_private *priv = file->private_data;
> +
> +	mmdrop(priv->mm);
> +	kfree(priv);
> +	return 0;
> +}
> +
>  static const struct file_operations proc_mem_operations = {
>  	.llseek		= mem_lseek,
>  	.read		= mem_read,
>  	.write		= mem_write,
>  	.open		= mem_open,
> -	.release	= mem_release,
> +	.release	= mem_release_with_private,
>  	.fop_flags	= FOP_UNSIGNED_OFFSET,
>  };
>  
> 
> -- 
> 2.55.0.737.g08866a6d13-goog
> 
-- 
Jan Kara <[email protected]>
SUSE Labs, CR