linux-kernel - Re: [PATCH] kernel/fork: stop playing lockless games for exe

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: <CAGudoHHHyZJXkNfGjGyxqHDay3yAzCm97r6SOdiSLYao7q2Z7A@mail.gmail.com>
Date:   Mon, 14 Aug 2023 10:21:21 +0200
From:   Mateusz Guzik <mjguzik@...il.com>
To:     David Hildenbrand <david@...hat.com>
Cc:     linux-kernel@...r.kernel.org, torvalds@...ux-foundation.org,
        brauner@...nel.org, ebiederm@...ssion.com,
        akpm@...ux-foundation.org, linux-mm@...ck.org, koct9i@...il.com,
        oleg@...hat.com, dave@...olabs.net
Subject: Re: [PATCH] kernel/fork: stop playing lockless games for exe_file replacement

On 8/14/23, David Hildenbrand <david@...hat.com> wrote:
> On 13.08.23 14:33, Mateusz Guzik wrote:
>> xchg originated in 6e399cd144d8 ("prctl: avoid using mmap_sem for
>> exe_file serialization"). While the commit message does not explain
>> *why* the change, clearly the intent was to use mmap_sem less in this
>> codepath. I found the original submission [1] which ultimately claims it
>> cleans things up.
>
> More details are apparently in v1 of that patch:
>
> https://lore.kernel.org/lkml/1424979417.10344.14.camel@stgolabs.net/
>
> Regarding your patch: adding more mmap_write_lock() where avoidable, I'm
> not so sure.
>

But exe_file access is already synchronized with the semaphore and
your own commit added a mmap_read_lock/mmap_read_unlock cycle after
the xchg in question to accomodate this requirement.

Then mmap_write_lock around the replacement is the obvious thing to do.

> Your patch doesn't look (to me) like it is removing a lot of complexity.
>

The code in the current form makes the reader ask what prompts xchg +
read-lock instead of mere write-locking.

This is not a hot path either and afaics it can only cause contention
if userspace is trying to abuse the interface to break the kernel,
messing with a processs hard at work (which would be an extra argument
to not play games on kernel side).

That said, no, it does not remove "a lot of complexity". It does
remove some though at no real downside that I can see.

But then again, it is on people who insist on xchg to justify it.

-- 
Mateusz Guzik <mjguzik gmail.com>