linux-kernel - Re: [PATCH 01/29] Revert "userfaultfd: don't fail on unrecognized features"

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: <cf4595d5-8b6c-efee-a721-c908bb3e3a5d@redhat.com>
Date:   Mon, 3 Apr 2023 09:48:31 +0200
From:   David Hildenbrand <david@...hat.com>
To:     Axel Rasmussen <axelrasmussen@...gle.com>,
        Peter Xu <peterx@...hat.com>
Cc:     linux-mm@...ck.org, linux-kernel@...r.kernel.org,
        Mike Kravetz <mike.kravetz@...cle.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Andrea Arcangeli <aarcange@...hat.com>,
        Mike Rapoport <rppt@...ux.vnet.ibm.com>,
        Nadav Amit <nadav.amit@...il.com>,
        Leonardo Bras Soares Passos <lsoaresp@...hat.com>,
        linux-stable <stable@...r.kernel.org>
Subject: Re: [PATCH 01/29] Revert "userfaultfd: don't fail on unrecognized
 features"

On 30.03.23 21:04, Axel Rasmussen wrote:
> On Thu, Mar 30, 2023 at 8:57 AM Peter Xu <peterx@...hat.com> wrote:
>>
>> This is a proposal to revert commit 914eedcb9ba0ff53c33808.
>>
>> I found this when writting a simple UFFDIO_API test to be the first unit
>> test in this set.  Two things breaks with the commit:
>>
>>    - UFFDIO_API check was lost and missing.  According to man page, the
>>    kernel should reject ioctl(UFFDIO_API) if uffdio_api.api != 0xaa.  This
>>    check is needed if the api version will be extended in the future, or
>>    user app won't be able to identify which is a new kernel.
> 
> 100% agreed, this was a mistake.
> 
>>
>>    - Feature flags checks were removed, which means UFFDIO_API with a
>>    feature that does not exist will also succeed.  According to the man
>>    page, we should (and it makes sense) to reject ioctl(UFFDIO_API) if
>>    unknown features passed in.
> 
> I still strongly disagree with reverting this part, my feeling is
> still that doing so makes things more complicated for no reason.
> 
> Re: David's point, it's clearly wrong to change semantics so a thing
> that used to work now fails. But this instead makes it more permissive
> - existing userspace programs continue to work as-is, but *also* one
> can achieve the same thing more simply (combine probing +
> configuration into one step). I don't see any problem with that,
> generally.
> 
> But, if David and others don't find my argument convincing, it isn't
> the end of the world. It just means I have to go update my userspace
> code to be a bit more complicated. :)


I'd probably find it more convincing if we'd started out with that 
approach ;) . User space would have to deal with the behavior of old 
kernels either way already? IOW, old kernels would reject the new flags, 
new kernels would not reject them but mask them out. So changing that 
behavior after the effects is somewhat suboptimal IMHO ... and rather 
makes things more complicated.

-- 
Thanks,

David / dhildenb