linux-kernel - Re: [PATCH net-next v4 4/4] net: tun: track dropped skb via kfree_skb

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20220301185021.7cba195d@kicinski-fedora-PC1C0HJN.hsd1.ca.comcast.net>
Date:   Tue, 1 Mar 2022 18:50:21 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     Dongli Zhang <dongli.zhang@...cle.com>, dsahern@...il.com
Cc:     netdev@...r.kernel.org, bpf@...r.kernel.org,
        linux-kernel@...r.kernel.org, davem@...emloft.net,
        rostedt@...dmis.org, mingo@...hat.com, ast@...nel.org,
        daniel@...earbox.net, andrii@...nel.org, imagedong@...cent.com,
        joao.m.martins@...cle.com, joe.jin@...cle.com, edumazet@...gle.com
Subject: Re: [PATCH net-next v4 4/4] net: tun: track dropped skb via
 kfree_skb_reason()

On Sat, 26 Feb 2022 00:49:29 -0800 Dongli Zhang wrote:
> +	SKB_DROP_REASON_SKB_PULL,	/* failed to pull sk_buff data */
> +	SKB_DROP_REASON_SKB_TRIM,	/* failed to trim sk_buff data */

IDK if these are not too low level and therefore lacking meaning.

What are your thoughts David?

Would it be better to up level the names a little bit and call SKB_PULL
something like "HDR_TRUNC" or "HDR_INV" or "HDR_ERR" etc or maybe
"L2_HDR_ERR" since in this case we seem to be pulling off ETH_HLEN?

For SKB_TRIM the error comes from allocation failures, there may be
a whole bunch of skb helpers which will fail only under mem pressure,
would it be better to identify them and return some ENOMEM related
reason, since, most likely, those will be noise to whoever is tracking
real errors?

>  	SKB_DROP_REASON_DEV_HDR,	/* there is something wrong with
>  					 * device driver specific header
>  					 */
> +	SKB_DROP_REASON_DEV_READY,	/* device is not ready */

What is ready? link is not up? peer not connected? can we expand?