linux-kernel - Re: [PATCH v4 02/13] rust: sync: introduce `Lock` and `Guard`

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: <5628890a-a975-9283-7ce7-969d1501de1f@ryhl.io>
Date:   Fri, 14 Apr 2023 14:02:49 +0200
From:   Alice Ryhl <alice@...l.io>
To:     Wedson Almeida Filho <wedsonaf@...il.com>,
        Gary Guo <gary@...yguo.net>, rust-for-linux@...r.kernel.org
Cc:     Miguel Ojeda <ojeda@...nel.org>,
        Alex Gaynor <alex.gaynor@...il.com>,
        Boqun Feng <boqun.feng@...il.com>,
        Björn Roy Baron <bjorn3_gh@...tonmail.com>,
        linux-kernel@...r.kernel.org,
        Wedson Almeida Filho <walmeida@...rosoft.com>,
        Martin Rodriguez Reboredo <yakoyoku@...il.com>
Subject: Re: [PATCH v4 02/13] rust: sync: introduce `Lock` and `Guard`

On 4/12/23 13:38, Wedson Almeida Filho wrote:
> On Tue, 11 Apr 2023 at 17:42, Gary Guo <gary@...yguo.net> wrote:
>>
>> On Tue, 11 Apr 2023 02:45:32 -0300
>> Wedson Almeida Filho <wedsonaf@...il.com> wrote:
>>
>>> From: Wedson Almeida Filho <walmeida@...rosoft.com>
>>>
>>> They are generic Rust implementations of a lock and a lock guard that
>>> contain code that is common to all locks. Different backends will be
>>> introduced in subsequent commits.
>>>
>>> Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@...il.com>
>>> Suggested-by: Gary Guo <gary@...yguo.net>
>>> Signed-off-by: Wedson Almeida Filho <walmeida@...rosoft.com>
>>> --- >>
>> There is not drop implementation on `Lock`, which implies all locks can
>> be just forgotten?
> 
> Yes, all locks can be forgotten.

Are the locks not pinned? Pinning comes with the guarantee that the 
value remains valid until the destructor runs, so the only kind of 
forgetting you can do to a lock is the one where the memory containing 
it is leaked, meaning that its not a UAF to use it after forgetting it.

My understanding is that the issue had to do with forgetting the guard 
types, since those were not pinned.

>> I believe we discussed a case where this is can lead to UAF when a lock
>> is dropped while it is locked (e.g. because the guard is forgotten).
> 
> Yes, this is the issue brought up by Boqun:
> https://github.com/Rust-for-Linux/linux/issues/862
> 
> The issue arises when a mutex guard is forgotten and the task that
> owns it exits. Then another task trying to acquire the mutex will lead
> to a UAF. A drop implementation on the lock doesn't solve this.
> 
> One solution is to increment the refcount on the current task when we
> acquire the mutex and decrement it when we release, but if we do that,
> the cost of acquiring/releasing a mutex gets much worse in Rust than
> it is in C.
> 
> Another solution might be to force disable CONFIG_MUTEX_SPIN_ON_OWNER
> when Rust is enabled, which is undesirable because it affects the
> performance of C code as well.
> 
> Even a closure-based lock (which I believe you suggested at the time)
> doesn't solve this completely because the thread may exit during the
> closure execution and leave a dangling pointer in the mutex.
> 
> So we don't have a good solution for this yet.