Re: [PATCH] procfs: also fix proc_reg_get_unmapped_area() for !MMU case

[Andrew Morton <akpm@linux-foundation.org>]

On Mon, 25 Nov 2013 16:22:31 +0000 "Jan Beulich" <JBeulich@suse.com> wrote:

> Commit fad1a86e ("procfs: call default get_unmapped_area on MMU-present
> architectures"), as its title says, took care of only the MMU case,
> leaving the !MMU side still in the regressed state (returning -EIO in
> all cases where pde->proc_fops->get_unmapped_area is NULL).

The changelog is rather mystifying unless the reader goes off and finds
the fad1a86e changelog, so let's do that for them by adding this:

>From the fad1a86e changelog:

: Commit c4fe24485729 ("sparc: fix PCI device proc file mmap(2)") added
: proc_reg_get_unmapped_area in proc_reg_file_ops and
: proc_reg_file_ops_no_compat, by which now mmap always returns EIO if
: get_unmapped_area method is not defined for the target procfs file, which
: causes regression of mmap on /proc/vmcore.
: 
: To address this issue, like get_unmapped_area(), call default
: current->mm->get_unmapped_area on MMU-present architectures if
: pde->proc_fops->get_unmapped_area, i.e.  the one in actual file operation
: in the procfs file, is not defined.

> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> Cc: HATAYAMA Daisuke <d.hatayama@jp.fujitsu.com>
> Cc: Alexey Dobriyan <adobriyan@gmail.com>
> Cc: David S. Miller <davem@davemloft.net> 

I tagged this with

	Cc: <stable@vger.kernel.org>	[3.12.x]

OK?

> +++ 3.13-rc1-proc-get-unmapped-area/fs/proc/inode.c
> @@ -292,16 +292,19 @@ proc_reg_get_unmapped_area(struct file *
>  {
>  	struct proc_dir_entry *pde = PDE(file_inode(file));
>  	unsigned long rv = -EIO;
> -	unsigned long (*get_area)(struct file *, unsigned long, unsigned long,
> -				  unsigned long, unsigned long) = NULL;
> +
>  	if (use_pde(pde)) {
> +		typeof(proc_reg_get_unmapped_area) *get_area
>  #ifdef CONFIG_MMU
> -		get_area = current->mm->get_unmapped_area;
> +			= pde->proc_fops->get_unmapped_area
> +			  ?: current->mm->get_unmapped_area;
> +#else
> +			= pde->proc_fops->get_unmapped_area;
>  #endif
> -		if (pde->proc_fops->get_unmapped_area)
> -			get_area = pde->proc_fops->get_unmapped_area;
> -		if (get_area)
> -			rv = get_area(file, orig_addr, len, pgoff, flags);
> +
> +		rv = get_area
> +		     ? get_area(file, orig_addr, len, pgoff, flags)
> +		     : orig_addr;
>  		unuse_pde(pde);
>  	}
>  	return rv;

That function has gone from bad to worse :(

How does this version look?

It would be acceptable and beneficial to create a new get_unmapped_area_t.

: static unsigned long
: proc_reg_get_unmapped_area(struct file *file, unsigned long orig_addr,
: 			   unsigned long len, unsigned long pgoff,
: 			   unsigned long flags)
: {
: 	struct proc_dir_entry *pde = PDE(file_inode(file));
: 	unsigned long rv = -EIO;
: 
: 	if (use_pde(pde)) {
: 		typeof(proc_reg_get_unmapped_area) *get_area;
: 
: 		get_area = pde->proc_fops->get_unmapped_area;
: #ifdef CONFIG_MMU
: 		if (!get_area)
: 			get_area = current->mm->get_unmapped_area;
: #endif
: 
: 		if (get_area)
: 			rv = get_area(file, orig_addr, len, pgoff, flags);
: 		else
: 			rv = orig_addr;
: 		unuse_pde(pde);
: 	}
: 	return rv;
: }

--- a/fs/proc/inode.c~procfs-also-fix-proc_reg_get_unmapped_area-for-mmu-case-fix
+++ a/fs/proc/inode.c
@@ -294,17 +294,18 @@ proc_reg_get_unmapped_area(struct file *
 	unsigned long rv = -EIO;
 
 	if (use_pde(pde)) {
-		typeof(proc_reg_get_unmapped_area) *get_area
+		typeof(proc_reg_get_unmapped_area) *get_area;
+
+		get_area = pde->proc_fops->get_unmapped_area;
 #ifdef CONFIG_MMU
-			= pde->proc_fops->get_unmapped_area
-			  ?: current->mm->get_unmapped_area;
-#else
-			= pde->proc_fops->get_unmapped_area;
+		if (!get_area)
+			get_area = current->mm->get_unmapped_area;
 #endif
 
-		rv = get_area
-		     ? get_area(file, orig_addr, len, pgoff, flags)
-		     : orig_addr;
+		if (get_area)
+			rv = get_area(file, orig_addr, len, pgoff, flags);
+		else
+			rv = orig_addr;
 		unuse_pde(pde);
 	}
 	return rv;
_