netdev - Re: [PATCH net-next] net: Avoid spurious rx_dropped increases with tap and rx

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <20200525.180024.261815334954697599.davem@davemloft.net>
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.