netdev - Re: [PATCH net-next v7 1/5] rust: core abstractions for network PHY drivers

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]

Date: Mon, 30 Oct 2023 08:37:33 +0000
From: Benno Lossin <benno.lossin@...ton.me>
To: Andrew Lunn <andrew@...n.ch>, FUJITA Tomonori <fujita.tomonori@...il.com>
Cc: boqun.feng@...il.com, netdev@...r.kernel.org, rust-for-linux@...r.kernel.org, tmgross@...ch.edu, miguel.ojeda.sandonis@...il.com, wedsonaf@...il.com
Subject: Re: [PATCH net-next v7 1/5] rust: core abstractions for network PHY drivers

On 29.10.23 18:32, Andrew Lunn wrote:
>> The current code is fine from Rust perspective because the current
>> code copies phy_driver on stack and makes a reference to the copy, if
>> I undertand correctly.
>>
>> It's not nice to create an 500-bytes object on stack. It turned out
>> that it's not so simple to avoid it.
> 
> Does it also copy the stack version over the 'real' version before
> exiting? If not, it is broken, since modifying state in phy_device is
> often why the driver is called. But copying the stack version is also
> broken, since another thread taking the phydev->lock is going to get
> lost from the linked list of waiters.

It does not copy the stack version over the original. Since we only read
the `link` field in the discussed function and we hold the lock, it should
not get modified, right? So from a functional viewpoint there is no issue.
(Aside from increased stack size and the data race issue etc.)

> Taking a copy of the C structure does seem very odd, to me as a C
> programmer.

It is also odd in Rust.

-- 
Cheers,
Benno