linux-kernel - Re: [PATCH v2] x86: Ensure input to pfn_to

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CF5E01C9-3EC6-422C-BAD2-B8732E6FCE77@zytor.com>
Date:   Thu, 16 Nov 2023 00:50:46 -0500
From:   "H. Peter Anvin" <hpa@...or.com>
To:     Michael Roth <michael.roth@....com>,
        Dave Hansen <dave.hansen@...el.com>
CC:     x86@...nel.org, Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Tom Lendacky <thomas.lendacky@....com>,
        Joerg Roedel <jroedel@...e.de>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] x86: Ensure input to pfn_to_kaddr() is treated as a 64-bit type

On November 15, 2023 5:42:31 PM EST, Michael Roth <michael.roth@....com> wrote:
>On Wed, Nov 15, 2023 at 12:48:58PM -0800, Dave Hansen wrote:
>> On 11/15/23 12:14, Michael Roth wrote:
>> > While it might be argued that the issue is on the caller side, other
>> > archs/macros have taken similar approaches to deal with instances like
>> > this, such as commit e48866647b48 ("ARM: 8396/1: use phys_addr_t in
>> > pfn_to_kaddr()").
>> 
>> Gah, I really hope nobody is arguing that for real, or is even thinking
>> about this as a valid argument.
>
>Not that I'm aware, but I did have my own doubts initially, which is
>why I thought it warranted a note in the commit just in case it came up
>from someone else.
>
>> 
>> The helper should, well, help the caller.  It makes zero sense to me
>> that every single call site would need to know if the argument's type
>> was big enough to hold the _return_ value.  This nonsense can only even
>> happen with macros.  Type promotion would just do the right thing for
>> any sanely declared actual helper function.
>
>My thought was that it is easier to expect developers to know the pitfalls
>of bit-field types, since it is universally applicable to all C code,
>whereas expecting developers to anticipate such issues when writing similar
>macros is potentially harder to enforce/audit and could lead to similar
>issues popping up as things are refactored over time and new macros get
>added that don't take such usages into account.
>
>But neither argument seems to hold up in reality. Experienced developers
>obviously do fall victim to the subtleties of of bit-field types, and
>kernel devs obviously do tend to address these instances in more robust
>ways based on the various pfn-related macros I looked through.
>
>-Mike

Now, if you are doing a cast, you are making the macro unusable for assembly anyway; any reason not to make it an inline function at that point?