linux-kernel - Re: [PATCH v3 07/22] checkpatch: Warn on page table access without accessors

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: <818ba76e-0553-459d-b956-520f23762034@sifive.com>
Date: Thu, 11 Dec 2025 09:29:28 +0900
From: Samuel Holland <samuel.holland@...ive.com>
To: "David Hildenbrand (Red Hat)" <david@...nel.org>,
 Joe Perches <joe@...ches.com>, Palmer Dabbelt <palmer@...belt.com>,
 Paul Walmsley <pjw@...nel.org>, linux-riscv@...ts.infradead.org,
 Andrew Morton <akpm@...ux-foundation.org>, linux-mm@...ck.org
Cc: devicetree@...r.kernel.org, Suren Baghdasaryan <surenb@...gle.com>,
 linux-kernel@...r.kernel.org, Mike Rapoport <rppt@...nel.org>,
 Michal Hocko <mhocko@...e.com>, Conor Dooley <conor@...nel.org>,
 Lorenzo Stoakes <lorenzo.stoakes@...cle.com>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Alexandre Ghiti <alex@...ti.fr>,
 Emil Renner Berthing <kernel@...il.dk>, Rob Herring <robh+dt@...nel.org>,
 Vlastimil Babka <vbabka@...e.cz>, "Liam R . Howlett"
 <Liam.Howlett@...cle.com>, Andy Whitcroft <apw@...onical.com>,
 Dwaipayan Ray <dwaipayanray1@...il.com>,
 Lukas Bulwahn <lukas.bulwahn@...il.com>
Subject: Re: [PATCH v3 07/22] checkpatch: Warn on page table access without
 accessors

On 2025-11-14 4:17 AM, David Hildenbrand (Red Hat) wrote:
> On 13.11.25 03:36, Samuel Holland wrote:
>> On 2025-11-12 8:21 PM, Joe Perches wrote:
>>> On Wed, 2025-11-12 at 17:45 -0800, Samuel Holland wrote:
>>>> Architectures may have special rules for accessing the hardware page
>>>> tables (for example, atomicity/ordering requirements), so the generic MM
>>>> code provides the pXXp_get() and set_pXX() hooks for architectures to
>>>> implement. These accessor functions are often omitted where a raw
>>>> pointer dereference is believed to be safe (i.e. race-free). However,
>>>> RISC-V needs to use these hooks to rewrite the page table values at
>>>> read/write time on some platforms. A raw pointer dereference will no
>>>> longer produce the correct value on those platforms, so the generic code
>>>> must always use the accessor functions.
>>> []
>>>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>>> []
>>>> @@ -7721,6 +7721,13 @@ sub process {
>>>>                   ERROR("MISSING_SENTINEL", "missing sentinel in ID
>>>> array\n" . "$here\n$stat\n");
>>>>               }
>>>>           }
>>>> +
>>>> +# check for raw dereferences of hardware page table pointers
>>>> +        if ($realfile !~ m@...ch/@ &&
>>>> +            $line =~ /(?<!pte_t |p[mu4g]d_t |izeof\()\*\(?(vmf(\.|->))?
>>>> (pte|p[mu4g]d)p?\b/) {
>>>> +            WARN("PAGE_TABLE_ACCESSORS",
>>>> +                 "Use $3p_get()/set_$3() instead of dereferencing page
>>>> table pointers\n" . $herecurr);
>>>> +        }
>>>>       }
>>>
>>> Seems like a lot of matches
>>>
>>> $ git grep -P '(?<!pte_t |p[mu4g]d_t |izeof\()\*\(?(vmf(\.|->))?(pte|
>>> p[mu4g]d)p?\b' | \
>>>    grep -v '^arch/' | wc -l
>>> 766
> 
> That is indeed concerning.
> 
> I recall that we discussed an alternative approach with Ryan in the past: I
> don't remember all the details, but essentially it was about using separate
> types, such that dereferencing would not get you the type the other functions
> would be expecting. Such that the compiler will bark when you try to dereference.

Even if some functions a new incompatible pointer type, don't we still have the
problem that neither type would be safe to dereference?

A similar option to a new type would be to add a sparse annotation to the
pointers that reference hardware page tables, similar to __user. I have
prototyped a coccinelle script to add this annotation, and the sparse checking
works. But I don't have the coccinelle expertise to automate the whole thing, so
there's a lot of manual cleanup required. And this requires touching all
architectures at once to avoid introducing erroneous sparse warnings. So I did
not include this for v3 because it is quite a lot of churn. Is this something I
should try to fully implement for v4?

Regards,
Samuel