Re: [PATCH net v4 2/2] mlx5: fix possible ptp queue fifo use-after-free

[Jakub Kicinski <kuba@kernel.org>]

On Wed, 1 Feb 2023 15:40:00 -0800 Saeed Mahameed wrote:
> >And again the only concern here is that we don't have any checks 
> >whether FIFO has anything to pop before actually poping the value. 
> >That can easily bring use-after-free in the futuee, especially because 
> >the function is not ptp specific and is already used for other fifos. 
> >I can add unlikely() for this check if it helps, but I would like to 
> >have this check in the code.
> 
> you shouldn't access the fifo if by design it's guaranteed nothing is there.
> We don't build for a future/fool proof code, the fifo is only accessed
> when we know there's something there by design, this is not a general
> purpose fifo, it's a fifo used by mlx5 ordered cqs.. 

The check for fifo being empty seems 100% sane to me. You can put 
a WARN_ON_ONCE() on it if you believe it can never happen. But the
cost of dealing with random double frees is much higher than a single
conditional on not-so-fast path.

> According to your logic, kfree should also check for double free.. ? :) 

I reckon we'd happily make kfree check for double free if there was
an efficient way of doing that. Various large companies build their
production kernels with KFENCE enabled, AFAIK.