netdev - Re: [PATCH 0/5] Rust abstractions for network device 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]

Message-ID: <c1b23f21-d161-6241-26fb-7a2cbc4c059c@ryhl.io>
Date: Sat, 17 Jun 2023 12:08:26 +0200
From: Alice Ryhl <alice@...l.io>
To: Andrew Lunn <andrew@...n.ch>
Cc: Jakub Kicinski <kuba@...nel.org>,
 FUJITA Tomonori <fujita.tomonori@...il.com>, netdev@...r.kernel.org,
 rust-for-linux@...r.kernel.org, aliceryhl@...gle.com,
 miguel.ojeda.sandonis@...il.com
Subject: Re: [PATCH 0/5] Rust abstractions for network device drivers

On 6/16/23 22:04, Andrew Lunn wrote:
>> Yes, you can certainly put a WARN_ON in the destructor.
>>
>> Another possibility is to use a scope to clean up. I don't know anything
>> about these skb objects are used, but you could have the user define a
>> "process this socket" function that you pass a pointer to the skb, then make
>> the return value be something that explains what should be done with the
>> packet. Since you must return a value of the right type, this forces you to
>> choose.
>>
>> Of course, this requires that the processing of packets can be expressed as
>> a function call, where it only inspects the packet for the duration of that
>> function call. (Lifetimes can ensure that the skb pointer does not escape
>> the function.)
>>
>> Would something like that work?
> 
> I don't think so, at least not in the contest of an Rust Ethernet
> driver.
> 
> There are two main flows.
> 
> A packet is received. An skb is allocated and the received packet is
> placed into the skb. The Ethernet driver then hands the packet over to
> the network stack. The network stack is free to do whatever it wants
> with the packet. Things can go wrong within the driver, so at times it
> needs to free the skb rather than pass it to the network stack, which
> would be a drop.
> 
> The second flow is that the network stack has a packet it wants sent
> out an Ethernet port, in the form of an skb. The skb gets passed to
> the Ethernet driver. The driver will do whatever it needs to do to
> pass the contents of the skb to the hardware. Once the hardware has
> it, the driver frees the skb. Again, things can go wrong and it needs
> to free the skb without sending it, which is a drop.
> 
> So the lifetime is not a simple function call.
> 
> The drop reason indicates why the packet was dropped. It should give
> some indication of what problem occurred which caused the drop. So
> ideally we don't want an anonymous drop. The C code does not enforce
> that, but it would be nice if the rust wrapper to dispose of an skb
> did enforce it.

It sounds like a destructor with WARN_ON is the best approach right now.

Unfortunately, I don't think we can enforce that the destructor is not 
used today. That said, in the future it may be possible to implement a 
linter that detects it - I know that there have already been experiments 
with other custom lints for the kernel (e.g., enforcing that you don't 
sleep while holding a spinlock).

> I would also say that this dummy driver and the C dummy driver is
> actually wrong in 'dropping' the frame. Its whole purpose in life is to
> be a black hole. It should only drop the packet if for some reason it
> cannot throw the packet into the black hole.

Ah, I suppose that we would also need a "by value" cleanup method for 
that case.

Alice