mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "David Hildenbrand (Arm)" <david@kernel.org>
To: Lance Yang <lance.yang@linux.dev>, akpm@linux-foundation.org
Cc: neganhat@gmail.com, jgg@ziepe.ca, jhubbard@nvidia.com,
	peterx@redhat.com, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] mm/gup: fix NULL pointer dereference in fixup_user_fault()
Date: Mon, 5 Oct 2026 12:27:50 +0200	[thread overview]
Message-ID: <ae9989b0-3797-4246-a2d2-985d7053d222@kernel.org> (raw)
In-Reply-To: <20261005021957.5254-1-lance.yang@linux.dev>

On 10/5/26 04:19, Lance Yang wrote:
> 
> On Sun, Oct 04, 2026 at 12:56:01PM -0700, Andrew Morton wrote:
>> On Sun,  4 Oct 2026 02:48:46 +0700 Nguyen Duy Nhat Anh <neganhat@gmail.com> wrote:
>>
>>> In fixup_user_fault(), the 'unlocked' parameter is checked for NULL
>>> early on, allowing callers to pass NULL if they do not track whether the
>>> mmap lock was dropped.
>>>
>>> However, if handle_mm_fault() returns VM_FAULT_COMPLETED, line 1597
>>> dereferences 'unlocked' directly (*unlocked = true) without checking
>>> if it is NULL. Callers like s390's pci_mmio.c pass NULL for 'unlocked',
>>> leading to a kernel NULL pointer dereference when VM_FAULT_COMPLETED
>>> occurs.
>>>
>>> Fix this by checking if 'unlocked' is non-NULL before assigning to it.
>>
>> This code is too subtle so you aren't the first to attempt to "fix" it.
>>
>> The key hint is in the kerneldoc:
>>
>> * @unlocked:	did we unlock the mmap_lock while retrying, maybe NULL if caller
>> *		does not allow retry. If NULL, the caller must guarantee
>> *		that fault_flags does not contain FAULT_FLAG_ALLOW_RETRY.
>>
>> Trace through the
>> FAULT_FLAG_ALLOW_RETRY/VM_FAULT_COMPLETED/VM_FAULT_RETRY logic
>> to confirm that the null deref is a cant-happen.
> 
> IIUC, that's indeed a can't-happen :)

https://lore.kernel.org/linux-mm/c44e5924-091f-43ed-8cff-34e87c3d9763@kernel.org/

People should stop trusting tool output and use their brain ;)

If you did mmap_read_lock() and wouldn't have a way to indicate that to the user
something would be seriously messed up.

-- 
Cheers,

David

  reply	other threads:[~2026-10-05 10:27 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-03 19:48 Nguyen Duy Nhat Anh
2026-10-04 19:56 ` Andrew Morton
2026-10-05  2:19   ` Lance Yang
2026-10-05 10:27     ` David Hildenbrand (Arm) [this message]
2026-10-05  9:37   ` [PATCH v2] mm/gup: document unlocked invariant in fixup_user_fault Nguyen Duy Nhat Anh
2026-10-05 10:29     ` David Hildenbrand (Arm)
2026-10-05 11:22       ` Nhật Anh Nguyễn Duy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ae9989b0-3797-4246-a2d2-985d7053d222@kernel.org \
    --to=david@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=jgg@ziepe.ca \
    --cc=jhubbard@nvidia.com \
    --cc=lance.yang@linux.dev \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=neganhat@gmail.com \
    --cc=peterx@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®