linux-kernel - Re: [patch 2/6] arch: mm: do not invoke OOM killer on kernel fault OOM

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20130726130721.GD17761@dhcp22.suse.cz>
Date:	Fri, 26 Jul 2013 15:07:21 +0200
From:	Michal Hocko <mhocko@...e.cz>
To:	Johannes Weiner <hannes@...xchg.org>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	David Rientjes <rientjes@...gle.com>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>,
	azurIt <azurit@...ox.sk>, linux-mm@...ck.org,
	cgroups@...r.kernel.org, x86@...nel.org,
	linux-arch@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [patch 2/6] arch: mm: do not invoke OOM killer on kernel fault
 OOM

On Thu 25-07-13 18:25:34, Johannes Weiner wrote:
> Kernel faults are expected to handle OOM conditions gracefully (gup,
> uaccess etc.), so they should never invoke the OOM killer.  Reserve
> this for faults triggered in user context when it is the only option.
> 
> Most architectures already do this, fix up the remaining few.
> 
> Signed-off-by: Johannes Weiner <hannes@...xchg.org>

I didn't go over all architectures to check whether something slipped
through but the converted ones look OK to me.

Reviewed-by: Michal Hocko <mhocko@...e.cz>

> ---
>  arch/arm/mm/fault.c       | 14 +++++++-------
>  arch/arm64/mm/fault.c     | 14 +++++++-------
>  arch/avr32/mm/fault.c     |  2 +-
>  arch/mips/mm/fault.c      |  2 ++
>  arch/um/kernel/trap.c     |  2 ++
>  arch/unicore32/mm/fault.c | 14 +++++++-------
>  6 files changed, 26 insertions(+), 22 deletions(-)
> 
> diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
> index c97f794..217bcbf 100644
> --- a/arch/arm/mm/fault.c
> +++ b/arch/arm/mm/fault.c
> @@ -349,6 +349,13 @@ retry:
>  	if (likely(!(fault & (VM_FAULT_ERROR | VM_FAULT_BADMAP | VM_FAULT_BADACCESS))))
>  		return 0;
>  
> +	/*
> +	 * If we are in kernel mode at this point, we
> +	 * have no context to handle this fault with.
> +	 */
> +	if (!user_mode(regs))
> +		goto no_context;
> +
>  	if (fault & VM_FAULT_OOM) {
>  		/*
>  		 * We ran out of memory, call the OOM killer, and return to
> @@ -359,13 +366,6 @@ retry:
>  		return 0;
>  	}
>  
> -	/*
> -	 * If we are in kernel mode at this point, we
> -	 * have no context to handle this fault with.
> -	 */
> -	if (!user_mode(regs))
> -		goto no_context;
> -
>  	if (fault & VM_FAULT_SIGBUS) {
>  		/*
>  		 * We had some memory, but were unable to
> diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
> index 0ecac89..dab1cfd 100644
> --- a/arch/arm64/mm/fault.c
> +++ b/arch/arm64/mm/fault.c
> @@ -294,6 +294,13 @@ retry:
>  			      VM_FAULT_BADACCESS))))
>  		return 0;
>  
> +	/*
> +	 * If we are in kernel mode at this point, we have no context to
> +	 * handle this fault with.
> +	 */
> +	if (!user_mode(regs))
> +		goto no_context;
> +
>  	if (fault & VM_FAULT_OOM) {
>  		/*
>  		 * We ran out of memory, call the OOM killer, and return to
> @@ -304,13 +311,6 @@ retry:
>  		return 0;
>  	}
>  
> -	/*
> -	 * If we are in kernel mode at this point, we have no context to
> -	 * handle this fault with.
> -	 */
> -	if (!user_mode(regs))
> -		goto no_context;
> -
>  	if (fault & VM_FAULT_SIGBUS) {
>  		/*
>  		 * We had some memory, but were unable to successfully fix up
> diff --git a/arch/avr32/mm/fault.c b/arch/avr32/mm/fault.c
> index b2f2d2d..2ca27b0 100644
> --- a/arch/avr32/mm/fault.c
> +++ b/arch/avr32/mm/fault.c
> @@ -228,9 +228,9 @@ no_context:
>  	 */
>  out_of_memory:
>  	up_read(&mm->mmap_sem);
> -	pagefault_out_of_memory();
>  	if (!user_mode(regs))
>  		goto no_context;
> +	pagefault_out_of_memory();
>  	return;
>  
>  do_sigbus:
> diff --git a/arch/mips/mm/fault.c b/arch/mips/mm/fault.c
> index 85df1cd..94d3a31 100644
> --- a/arch/mips/mm/fault.c
> +++ b/arch/mips/mm/fault.c
> @@ -241,6 +241,8 @@ out_of_memory:
>  	 * (which will retry the fault, or kill us if we got oom-killed).
>  	 */
>  	up_read(&mm->mmap_sem);
> +	if (!user_mode(regs))
> +		goto no_context;
>  	pagefault_out_of_memory();
>  	return;
>  
> diff --git a/arch/um/kernel/trap.c b/arch/um/kernel/trap.c
> index 089f398..b2f5adf 100644
> --- a/arch/um/kernel/trap.c
> +++ b/arch/um/kernel/trap.c
> @@ -124,6 +124,8 @@ out_of_memory:
>  	 * (which will retry the fault, or kill us if we got oom-killed).
>  	 */
>  	up_read(&mm->mmap_sem);
> +	if (!is_user)
> +		goto out_nosemaphore;
>  	pagefault_out_of_memory();
>  	return 0;
>  }
> diff --git a/arch/unicore32/mm/fault.c b/arch/unicore32/mm/fault.c
> index f9b5c10..8ed3c45 100644
> --- a/arch/unicore32/mm/fault.c
> +++ b/arch/unicore32/mm/fault.c
> @@ -278,6 +278,13 @@ retry:
>  	       (VM_FAULT_ERROR | VM_FAULT_BADMAP | VM_FAULT_BADACCESS))))
>  		return 0;
>  
> +	/*
> +	 * If we are in kernel mode at this point, we
> +	 * have no context to handle this fault with.
> +	 */
> +	if (!user_mode(regs))
> +		goto no_context;
> +
>  	if (fault & VM_FAULT_OOM) {
>  		/*
>  		 * We ran out of memory, call the OOM killer, and return to
> @@ -288,13 +295,6 @@ retry:
>  		return 0;
>  	}
>  
> -	/*
> -	 * If we are in kernel mode at this point, we
> -	 * have no context to handle this fault with.
> -	 */
> -	if (!user_mode(regs))
> -		goto no_context;
> -
>  	if (fault & VM_FAULT_SIGBUS) {
>  		/*
>  		 * We had some memory, but were unable to
> -- 
> 1.8.3.2
> 

-- 
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/