linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: David Hildenbrand <david@redhat.com>
To: Qi Zheng <zhengqi.arch@bytedance.com>
Cc: "Russell King (Oracle)" <linux@armlinux.org.uk>,
	Ezra Buehler <ezra@easyb.ch>,
	linux-mm@kvack.org, Andrew Morton <akpm@linux-foundation.org>,
	"Mike Rapoport (Microsoft)" <rppt@kernel.org>,
	Muchun Song <muchun.song@linux.dev>,
	Vlastimil Babka <vbabka@suse.cz>,
	Ryan Roberts <ryan.roberts@arm.com>,
	"Vishal Moola (Oracle)" <vishal.moola@gmail.com>,
	Hugh Dickins <hughd@google.com>,
	Matthew Wilcox <willy@infradead.org>,
	Peter Xu <peterx@redhat.com>,
	Nicolas Ferre <nicolas.ferre@microchip.com>,
	Alexandre Belloni <alexandre.belloni@bootlin.com>,
	Claudiu Beznea <claudiu.beznea@tuxon.dev>,
	open list <linux-kernel@vger.kernel.org>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [REGRESSION] NULL pointer dereference on ARM (AT91SAM9G25) during compaction
Date: Tue, 11 Feb 2025 10:37:54 +0100	[thread overview]
Message-ID: <2b0bb476-5bd6-489a-9b9e-7aa20964abfa@redhat.com> (raw)
In-Reply-To: <3f7babee-b232-4e6b-a896-947150dcd1ef@bytedance.com>

On 11.02.25 10:29, Qi Zheng wrote:
> 
> 
> On 2025/2/11 17:14, David Hildenbrand wrote:
>> On 11.02.25 04:45, Qi Zheng wrote:
>>> Hi Russell,
>>>
>>> On 2025/2/11 01:03, Russell King (Oracle) wrote:
>>>> On Mon, Feb 10, 2025 at 05:49:38PM +0100, Ezra Buehler wrote:
>>>>> When running vanilla Linux 6.13 or newer (6.14-rc2) on the
>>>>> AT91SAM9G25-based GARDENA smart Gateway, we are seeing a NULL pointer
>>>>> dereference resulting in a kernel panic. The culprit seems to be commit
>>>>> fc9c45b71f43 ("arm: adjust_pte() usepte_offset_map_rw_nolock()").
>>>>> Reverting the commit apparently fixes the issue.
>>>>
>>>> The blamed commit is buggy:
>>>>
>>>> arch/arm/include/asm/tlbflush.h:
>>>> #define update_mmu_cache(vma, addr, ptep) \
>>>>            update_mmu_cache_range(NULL, vma, addr, ptep, 1)
>>>>
>>>> So vmf can be NULL. This didn't used to matter before this commit,
>>>> because vmf was not used by ARM's update_mmu_cache_range(). However,
>>>> the commit introduced a dereference of it, which now causes a NULL
>>>> point dereference.
>>>>
>>>> Not sure what the correct solution is, but at a guess, both:
>>>>
>>>>      if (ptl != vmf->ptl)
>>>>
>>>> need to become:
>>>>
>>>>      if (!vmf || ptl != vmf->ptl)
>>>
>>> No, we can't do that, because without using split PTE locks, we would
>>> use shared mm->page_table_lock, which would create a deadlock.
>>
>> Maybe we can simply special-case on CONFIG_SPLIT_PTE_PTLOCKS ?
>>
>> if (IS_ENABLED(CONFIG_SPLIT_PTE_PTLOCKS)) {
> 
> In this case, if two vmas map the same PTE page, then the same PTE lock
> will be held repeatedly. Right?

Hmm, the comment says:

         /*
          * This is called while another page table is mapped, so we
          * must use the nested version.  This also means we need to
          * open-code the spin-locking.
          */

"another page table" implies that it cannot be the same. But maybe that 
comment was also wrong?


-- 
Cheers,

David / dhildenb



  reply	other threads:[~2025-02-11  9:38 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-10 16:49 Ezra Buehler
2025-02-10 17:03 ` Russell King (Oracle)
2025-02-11  3:45   ` Qi Zheng
2025-02-11  9:14     ` David Hildenbrand
2025-02-11  9:29       ` Qi Zheng
2025-02-11  9:37         ` David Hildenbrand [this message]
2025-02-11  9:43           ` Qi Zheng
2025-02-11 12:09             ` David Hildenbrand
2025-02-11 12:41               ` Qi Zheng
2025-02-12  6:40                 ` [PATCH] arm: pgtable: fix NULL pointer dereference issue Qi Zheng
2025-02-12  7:27                   ` Ezra Buehler
2025-02-12  7:32                     ` Qi Zheng
2025-02-12  8:20                   ` David Hildenbrand
2025-02-12  8:28                     ` Qi Zheng
2025-02-12  8:30                       ` David Hildenbrand

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=2b0bb476-5bd6-489a-9b9e-7aa20964abfa@redhat.com \
    --to=david@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=alexandre.belloni@bootlin.com \
    --cc=claudiu.beznea@tuxon.dev \
    --cc=ezra@easyb.ch \
    --cc=hughd@google.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux@armlinux.org.uk \
    --cc=muchun.song@linux.dev \
    --cc=nicolas.ferre@microchip.com \
    --cc=peterx@redhat.com \
    --cc=rppt@kernel.org \
    --cc=ryan.roberts@arm.com \
    --cc=vbabka@suse.cz \
    --cc=vishal.moola@gmail.com \
    --cc=willy@infradead.org \
    --cc=zhengqi.arch@bytedance.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