Re: [PATCH net-next v3 1/3] rust: core abstractions for network PHY drivers

[Benno Lossin <benno.lossin@proton.me>]

On 13.10.23 07:45, FUJITA Tomonori wrote:
> On Thu, 12 Oct 2023 21:17:14 -0700
> Boqun Feng <boqun.feng@gmail.com> wrote:
> 
>> After re-read my email exchange with Tomo, I realised I need to explain
>> this a little bit. The minimal requirement of a Rust binding is
>> soundness: it means if one only uses safe APIs, one cannot introduce
>> memory/type safety issue (i.e. cannot have an object in an invalid
>> state), this is a tall task, because you can have zero assumption of the
>> API users, you can only encode the usage requirement in the type system.
>>
>> Of course the type system doesn't always work, hence we have unsafe API,
>> but still the soundness of Rust bindings means using safe APIs +
>> *correctly* using unsafe APIs cannot introduce memory/type safety
>> issues.
>>
>> Tomo, this is why we gave you a hard time here ;-) Unsafe Rust APIs must
>> be very clear on the correct usage and safe Rust APIs must not assume
>> how users would call it. Hope this help explain a little bit, we are not
>> poking random things here, soundness is the team effort from everyone
>> ;-)
> 
> Understood, so let me know if you still want to improve something in
> v4 patchset :) I tried to addressed all the review comments.
> 
> btw, what's the purpose of using Rust in linux kernel? Creating sound
> Rust abstractions? Making linux kernel more reliable, or something
> else?  For me, making linux kernel more reliable is the whole
> point. Thus I still can't understand the slogan that Rust abstractions
> can't trust subsystems.

For me it is making the Linux kernel more reliable. The Rust abstractions
are just a tool for that goal: we offload the difficult task of handling
the C <-> Rust interactions and other `unsafe` features into those
abstractions. Then driver authors do not need to concern themselves with
that and can freely write drivers in safe Rust. Since there will be a lot
more drivers than abstractions, this will pay off in the end, since we will
have a lot less `unsafe` code than safe code.

Concentrating the difficult/`unsafe` code in the abstractions make it
easier to review (compared to `unsafe` code in every driver) and easier to
maintain, if we find a soundness issue, we only have to fix it in the
abstractions.

> Rust abstractions always must check the validity of values that
> subsysmtes give because subsysmtes might give an invalid value. Like
> the enum state issue, if PHYLIB has a bug then give a random value, so
> the abstraction have to prevent the invalid value in Rust with
> validity checking. But with such critical bug, likely the system
> cannot continue to run anyway. Preventing the invalid state in Rust
> aren't useful much for system reliability.

It's not that we do not trust the subsystems, for example when we register
a callback `foo` and the C side documents that it is ok to sleep within
`foo`, then we will assume so. If we would not trust the C side, then we
would have to disallow sleeping there, since sleeping while holding a
spinlock is UB (and the C side could accidentally be holding a spinlock).

But there are certain things where we do not trust the subsystems, these
are mainly things where we can afford it from a performance and usability
perspective (in the example above we could not afford it from a usability
perspective).

In the enum case it would also be incredibly simple for the C side to just
make a slight mistake and set the integer to a value outside of the
specified range. This strengthens the case for checking validity here.
When an invalid value is given to Rust we have immediate UB. In Rust UB
always means that anything can happen so we must avoid it at all costs.
In this case having a check would not really hurt performance and in terms
of usability it also seems reasonable. If it would be bad for performance,
let us know.

-- 
Cheers,
Benno