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  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]
Date:   Wed, 21 Sep 2022 00:22:54 +0000
From:   Vladimir Oltean <vladimir.oltean@....com>
To:     Andrew Lunn <andrew@...n.ch>
CC:     "mattias.forsblad@...il.com" <mattias.forsblad@...il.com>,
        netdev <netdev@...r.kernel.org>,
        Florian Fainelli <f.fainelli@...il.com>,
        Christian Marangi <ansuelsmth@...il.com>
Subject: Re: [PATCH rfc v0 2/9] net: dsa: qca8k: Move completion into DSA core

On Wed, Sep 21, 2022 at 02:19:54AM +0200, Andrew Lunn wrote:
> I've been thinking about this a bit. And i think the bug is in the old
> code.
> 
> As soon as we call reinit_completion(), a late arriving packet can
> trigger the completion. Note that the sequence number has not been
> incremented yet. So that late arriving packet can pass the sequence
> number test, and the results will be copied and complete() called.
> 
> qca8k_read_eth() can continue, increment the sequence number, call
> wait_for_completion_timeout() and immediately exit, returning the
> contents of the late arriving reply.
> 
> To make this safe:
> 
> 1) The sequence number needs to be incremented before
>    reinit_completion(). That closes one race
> 
> 2) If the sequence numbers don't match, silently drop the packet, it
>    is either later, or bogus. Hopefully the correct reply packet will
>    come along soon and trigger the completion.
> 
> I've also got some of this wrong in my code.

Wouldn't the programming be more obvious if we didn't try to reuse the
same dsa_inband structure for every request/response, but simply
allocate/free on demand and use only once?

Powered by blists - more mailing lists