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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:   Mon, 25 May 2020 18:00:24 -0700 (PDT)
From:   David Miller <davem@...emloft.net>
To:     bpoirier@...ulusnetworks.com
Cc:     netdev@...r.kernel.org, nikolay@...ulusnetworks.com,
        edumazet@...gle.com, jiri@...nulli.us
Subject: Re: [PATCH net-next] net: Avoid spurious rx_dropped increases with
 tap and rx_handler

From: Benjamin Poirier <bpoirier@...ulusnetworks.com>
Date: Mon, 25 May 2020 14:01:37 +0900

> Consider an skb which doesn't match a ptype_base/ptype_specific handler. If
> this skb is delivered to a ptype_all handler, it does not count as a drop.
> However, if the skb is also processed by an rx_handler which returns
> RX_HANDLER_PASS, the frame is now counted as a drop because pt_prev was
> reset. An example of this situation is an LLDP frame received on a bridge
> port while lldpd is listening on a packet socket with ETH_P_ALL (ex. by
> specifying `lldpd -c`).
> 
> Fix by adding an extra condition variable to record if the skb was
> delivered to a packet tap before running an rx_handler.
> 
> The situation is similar for RX_HANDLER_EXACT frames so their accounting is
> also changed. OTOH, the behavior is unchanged for RX_HANDLER_ANOTHER frames
> - they are accounted according to what happens with the new skb->dev.
> 
> Fixes: caf586e5f23c ("net: add a core netdev->rx_dropped counter")
> Message-Id: <20200522011420.263574-1-bpoirier@...ulusnetworks.com>
> Signed-off-by: Benjamin Poirier <bpoirier@...ulusnetworks.com>

You can state over and over about the semantics of PASS and other
rx_handler return values, but as Eric pointed out we free this SKB
so the counter should be bumped.

I'm sorry if it is confusing, but the counter is counting the event
accurately.

If you can make that kfree_skb() not happen, then maybe it shouldn't
be incremented.  But until then...

I'm not applying this, sorry.

Powered by blists - more mailing lists