Re: [PATCH] driver: tg3: fix potential UAF in tigon3_dma_hwbug_workaround()

[David Miller]

From: Gen Zhang <blackgod016574@xxxxxxxxx>
Date: Thu, 16 Jan 2020 11:30:44 +0800

> In tigon3_dma_hwbug_workaround(), pskb is first stored in skb. And this
> function is to store new_skb into pskb at the end. However, in the error
> paths when new_skb is freed by dev_kfree_skb_any(), stroing new_skb to pskb
> should be prevented.
> 
> And freeing skb with dev_consume_skb_any() should be executed after storing
> new_skb to pskb, because freeing skb will free pskb (alias).
> 
> Signed-off-by: Gen Zhang <blackgod016574@xxxxxxxxx>

There are no bugs here.

The caller never references "*pskb" when an error is returned.  So it is
safe to store any value whatsoever into that pointer.

'skb' never changes it's value even if we store something into *pskb
because we've loaded it into a local variable.  So it is always safe to
call dev_consume_skb_any() on 'skb' in any order with respect to that
assignment.

I'm not applying this until you can show a real bug resulting from
the current code, and if so you'll need to add that explanation to
your commit message.

Thanks.