linux-kernel - Re: [PATCH 2/3] arm64: signal: sigreturn() and rt_sigreturn() sometime returns the wrong signals

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: <m1y2cztuiw.fsf@fess.ebiederm.org>
Date:   Fri, 30 Apr 2021 14:57:43 -0500
From:   ebiederm@...ssion.com (Eric W. Biederman)
To:     Liam Howlett <liam.howlett@...cle.com>
Cc:     Will Deacon <will@...nel.org>,
        Catalin Marinas <catalin.marinas@....com>,
        Julien Grall <julien.grall@....com>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>,
        "linux-arm-kernel\@lists.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-kernel\@vger.kernel.org" <linux-kernel@...r.kernel.org>,
        "netdev\@vger.kernel.org" <netdev@...r.kernel.org>,
        "bpf\@vger.kernel.org" <bpf@...r.kernel.org>
Subject: Re: [PATCH 2/3] arm64: signal: sigreturn() and rt_sigreturn() sometime returns the wrong signals

Liam Howlett <liam.howlett@...cle.com> writes:

> This is way out of scope for what I'm doing.  I'm trying to fix a call
> to the wrong mm API.  I was trying to clean up any obvious errors in
> calling functions which were exposed by fixing that error.  If you want
> this fixed differently, then please go ahead and tackle the problems you
> see.

I was asked by the arm maintainers to describe what the code should be
doing here.  I hope I have done that.

What is very interesting is that the code in __do_page_fault does not
use find_vma_intersection it uses find_vma.  Which suggests that
find_vma_intersection may not be the proper mm api.

The logic is:

>From __do_page_fault:
	struct vm_area_struct *vma = find_vma(mm, addr);

	if (unlikely(!vma))
		return VM_FAULT_BADMAP;

	/*
	 * Ok, we have a good vm_area for this memory access, so we can handle
	 * it.
	 */
	if (unlikely(vma->vm_start > addr)) {
		if (!(vma->vm_flags & VM_GROWSDOWN))
			return VM_FAULT_BADMAP;
		if (expand_stack(vma, addr))
			return VM_FAULT_BADMAP;
	}

	/*
	 * Check that the permissions on the VMA allow for the fault which
	 * occurred.
	 */
	if (!(vma->vm_flags & vm_flags))
		return VM_FAULT_BADACCESS;

>From do_page_fault:

	arm64_force_sig_fault(SIGSEGV,
			      fault == VM_FAULT_BADACCESS ? SEGV_ACCERR : SEGV_MAPERR,
			      far, inf->name);


Hmm.  If the expand_stack step is skipped. Does is the logic equivalent
to find_vma_intersection?

	static inline struct vm_area_struct *find_vma_intersection(
        	struct mm_struct * mm,
                unsigned long start_addr,
                unsigned long end_addr)
	{
		struct vm_area_struct * vma = find_vma(mm,start_addr);
	
		if (vma && end_addr <= vma->vm_start)
			vma = NULL;
		return vma;
	}

Yes. It does look that way.  VM_FAULT_BADMAP is returned when a vma
covering the specified address is not found.  And VM_FAULT_BADACCESS is
returned when there is a vma and there is a permission problem.

There are also two SIGBUS cases that arm64_notify_segfault does not
handle.

So it appears changing arm64_notify_segfault to use
find_vma_intersection instead of find_vma would be a correct but
incomplete fix.

I don't see a point in changing sigerturn or rt_sigreturn.

Eric