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] [day] [month] [year] [list]
Date:   Sun, 19 Jan 2020 11:00:31 +0100 (CET)
From:   David Miller <davem@...emloft.net>
To:     blackgod016574@...il.com
Cc:     siva.kallam@...adcom.com, prashant@...adcom.com,
        mchan@...adcom.com, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] driver: tg3: fix potential UAF in
 tigon3_dma_hwbug_workaround()

From: Gen Zhang <blackgod016574@...il.com>
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@...il.com>

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.

Powered by blists - more mailing lists