netdev - Re: [PATCH net-next v3 3/4] net: use skb->priority for overloading skb->dropcount and skb->reserved

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: <20150223205633.7fbd08d4@halley>
Date:	Mon, 23 Feb 2015 20:56:33 +0200
From:	Shmulik Ladkani <shmulik.ladkani@...il.com>
To:	Eyal Birger <eyal.birger@...il.com>, davem@...emloft.net
Cc:	edumazet@...gle.com, netdev@...r.kernel.org
Subject: Re: [PATCH net-next v3 3/4] net: use skb->priority for overloading
 skb->dropcount and skb->reserved_tailroom instead of skb->mark

Hi,

On Mon, 23 Feb 2015 19:52:03 +0200 Eyal Birger <eyal.birger@...il.com> wrote:
> Commit 977750076d98 ("af_packet: add interframe drop cmsg (v6)")
> unionized skb->mark and skb->dropcount in order to allow recording
> of the socket drop count while maintaining struct sk_buff size.
> 
> In order to allow for the skb->mark to be fetched by user-space code
> it needs to be detached from this union; skb->priority is used
> instead.
> 
> The purpose of overloading skb->priority is solely for retaining
> struct sk_buff size; skb->priority is not used after the skb is
> queued to the socket.
> 
> Use of aliased fields, skb->dropcount and skb->reserved_tailroom,
> is either done after the skb is cloned (e.g. in packet_rcv()) or
> before skb->priority is used (e.g. in IGMPv3/MLD/TCP).

Preserving a small skb size is an important cause.

In this case however, it seems that aliasing 'priority' sacrifices
maintainability: 'priority' does not seem "naturally orthogonal" to
'drop_count' or 'reserved_tailroom'.

One might arm 'priority' in various code flows (future and existing),
which may accidentally clash with the aliases. It is unexpected, hard to
remember (especially since 'priority' wasn't formerly aliased), and
sometimes it's simply not trivial to assure there's no clash.

May I suggest to unalias 'mark' out of the union?

Regards,
Shmulik
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html