Re: [PATCH net-next v2 5/6] rust: Add read_poll_timeout function

[Boqun Feng]

On Tue, Oct 08, 2024 at 07:16:42PM +0200, Andrew Lunn wrote:
> On Tue, Oct 08, 2024 at 03:14:05PM +0200, Miguel Ojeda wrote:
> > On Tue, Oct 8, 2024 at 2:13 PM Andrew Lunn <andrew@xxxxxxx> wrote:
> > >
> > > As far as i see, might_sleep() will cause UAF where there is going to
> > > be a UAF anyway. If you are using it correctly, it does not cause UAF.
> > 
> > This already implies that it is an unsafe function (in general, i.e.
> > modulo klint, or a way to force the user to have to write `unsafe`
> > somewhere else, or what I call ASHes -- "acknowledged soundness
> > holes").
> > 
> > If we consider as safe functions that, if used correctly, do not cause
> > UB, then all functions would be safe.
> 
> From what i hear, klint is still WIP. So we have to accept there will
> be bad code out there, which will UAF. We want to find such bad code,

If you don't believe in klint, then we need to mark might_sleep() as
unsafe, as I already explain a million times, might_sleep() is unsafe
without the klint compile time check. You have to accept that an unsafe
function should really be marked as unsafe. And yes, in this way, all
sleep functions would be marked as unsafe as well (or we could mark all
preemption disable function as unsafe), but still an unsafe function is
unsafe.

Again, as Miguel mentioned, we can only mark might_sleep() because sleep
in atomic context is an ASH, not because it's really safe.

> and the easiest way to find it at the moment is to make it UAF as
> fast as possible. might_sleep() does that, __might_sleep() does not,
> and using neither is the slowest way.
> 

might_sleep() is useful because it checks preemption count and task
state, which is provided by __might_sleep() as well. I don't think
causing UAF helps we detect atomic context violation faster than what
__might_sleep() already have. Again, could you provide an example that
help me understand your reasoning here?

Regards,
Boqun

> 	Andrew