linux-kernel - Re: [RFC RESEND v2 02/13] powerpc: mm: Fix kfence page fault reporting

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: <660a2cf7-24f9-4558-87df-5e4c13362380@csgroup.eu>
Date: Tue, 15 Oct 2024 08:42:36 +0200
From: Christophe Leroy <christophe.leroy@...roup.eu>
To: "Ritesh Harjani (IBM)" <ritesh.list@...il.com>,
 linuxppc-dev@...ts.ozlabs.org
Cc: kasan-dev@...glegroups.com, linux-mm@...ck.org,
 Marco Elver <elver@...gle.com>, Alexander Potapenko <glider@...gle.com>,
 Heiko Carstens <hca@...ux.ibm.com>, Michael Ellerman <mpe@...erman.id.au>,
 Nicholas Piggin <npiggin@...il.com>,
 Madhavan Srinivasan <maddy@...ux.ibm.com>,
 Hari Bathini <hbathini@...ux.ibm.com>,
 "Aneesh Kumar K . V" <aneesh.kumar@...nel.org>,
 Donet Tom <donettom@...ux.vnet.ibm.com>,
 Pavithra Prakash <pavrampu@...ux.vnet.ibm.com>,
 LKML <linux-kernel@...r.kernel.org>, Disha Goel <disgoel@...ux.ibm.com>
Subject: Re: [RFC RESEND v2 02/13] powerpc: mm: Fix kfence page fault
 reporting



Le 15/10/2024 à 03:33, Ritesh Harjani (IBM) a écrit :
> copy_from_kernel_nofault() can be called when doing read of /proc/kcore.
> /proc/kcore can have some unmapped kfence objects which when read via
> copy_from_kernel_nofault() can cause page faults. Since *_nofault()
> functions define their own fixup table for handling fault, use that
> instead of asking kfence to handle such faults.
> 
> Hence we search the exception tables for the nip which generated the
> fault. If there is an entry then we let the fixup table handler handle the
> page fault by returning an error from within ___do_page_fault().
> 
> This can be easily triggered if someone tries to do dd from /proc/kcore.
> dd if=/proc/kcore of=/dev/null bs=1M
> 
> <some example false negatives>
> ===============================
> BUG: KFENCE: invalid read in copy_from_kernel_nofault+0xb0/0x1c8
> Invalid read at 0x000000004f749d2e:
>   copy_from_kernel_nofault+0xb0/0x1c8
>   0xc0000000057f7950
>   read_kcore_iter+0x41c/0x9ac
>   proc_reg_read_iter+0xe4/0x16c
>   vfs_read+0x2e4/0x3b0
>   ksys_read+0x88/0x154
>   system_call_exception+0x124/0x340
>   system_call_common+0x160/0x2c4
> 
> BUG: KFENCE: use-after-free read in copy_from_kernel_nofault+0xb0/0x1c8
> Use-after-free read at 0x000000008fbb08ad (in kfence-#0):
>   copy_from_kernel_nofault+0xb0/0x1c8
>   0xc0000000057f7950
>   read_kcore_iter+0x41c/0x9ac
>   proc_reg_read_iter+0xe4/0x16c
>   vfs_read+0x2e4/0x3b0
>   ksys_read+0x88/0x154
>   system_call_exception+0x124/0x340
>   system_call_common+0x160/0x2c4
> 
> Guessing the fix should go back to when we first got kfence on PPC32.
> 
> Fixes: 90cbac0e995d ("powerpc: Enable KFENCE for PPC32")
> Reported-by: Disha Goel <disgoel@...ux.ibm.com>
> Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@...il.com>
> ---
>   arch/powerpc/mm/fault.c | 10 +++++++++-
>   1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
> index 81c77ddce2e3..fa825198f29f 100644
> --- a/arch/powerpc/mm/fault.c
> +++ b/arch/powerpc/mm/fault.c
> @@ -439,9 +439,17 @@ static int ___do_page_fault(struct pt_regs *regs, unsigned long address,
>   	/*
>   	 * The kernel should never take an execute fault nor should it
>   	 * take a page fault to a kernel address or a page fault to a user
> -	 * address outside of dedicated places
> +	 * address outside of dedicated places.
> +	 *
> +	 * Rather than kfence reporting false negatives, let the fixup table
> +	 * handler handle the page fault by returning SIGSEGV, if the fault
> +	 * has come from functions like copy_from_kernel_nofault().
>   	 */
>   	if (unlikely(!is_user && bad_kernel_fault(regs, error_code, address, is_write))) {
> +
> +		if (search_exception_tables(instruction_pointer(regs)))
> +			return SIGSEGV;

This is a heavy operation. It should at least be done only when KFENCE 
is built-in.

kfence_handle_page_fault() bails out immediately when 
is_kfence_address() returns false, and is_kfence_address() returns 
always false when KFENCE is not built-in.

So you could check that before calling the heavy weight 
search_exception_tables().

		if (is_kfence_address(address) &&
		    !search_exception_tables(instruction_pointer(regs)) &&
		    kfence_handle_page_fault(address, is_write, regs))
			return 0;



 > +			return SIGSEGV;

> +
>   		if (kfence_handle_page_fault(address, is_write, regs))
>   			return 0;
>