netdev - Case for reusing netlink PADs WAS(Re: [PATCH net-next 1/1] net sched actions: dump more than TCA_ACT_MAX

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: <560eeb0b-7a34-484a-e4a1-c637b31eba59@mojatatu.com>
Date:   Tue, 18 Apr 2017 08:48:57 -0400
From:   Jamal Hadi Salim <jhs@...atatu.com>
To:     Eric Dumazet <eric.dumazet@...il.com>
Cc:     Jiri Pirko <jiri@...nulli.us>, davem@...emloft.net,
        netdev@...r.kernel.org, xiyou.wangcong@...il.com
Subject: Case for reusing netlink PADs WAS(Re: [PATCH net-next 1/1] net sched
 actions: dump more than TCA_ACT_MAX_PRIO actions per batch

On 17-04-17 01:08 PM, Eric Dumazet wrote:
> On Mon, 2017-04-17 at 12:46 -0400, Jamal Hadi Salim wrote:
>
>> Of course it is trivial to add this as attributes and 32 bits
>> for this case is not a big deal because it is done once. I want to talk
>> about the pads instead ;-> What do you suggest we do with pads?
>
> We do nothing with pads. Just leave them.
>
> Or, you need something to make sure to not break old user applications.
>
> Something like a version.
>
>

Dave is not fond of version fields - they tend to be abused
although looking at the idiag and tpacket stuff there is a case
to be made for versioning ;->

For the patches I posted, I will work on getting an attribute based
variant of the patches out - but i wanted to have this discussion a
little more if you bear with me.

Netlink is a wire protocol. When a protocol is defined with rules such
as alignment (which lead to explicit padding) then those are equivalent
to "reserved bits" in standard wire protocols. Good practise is:
all sender zero those bits(MBZ); and all receivers must ignore them
unless they wish to interpret them. Not everyone follows these rules
(I remember the havoc ECN caused when TCP/IP started using the different
reserved fields).

For our case it is _very sad_ that someone actually explicitly defined
pads - in my opinion for no other purpose other than reuse and then
we say we cant use them after.

I believe you understand what I was saying, but to clarify, here is
what i meant:
---
struct tcamsg {
         unsigned char   tca_family;
         union {
                 unsigned char   tca__pad1;
                 unsigned char   tca_flags;
         };
         union {
                 unsigned short  tca__pad2;
                 unsigned char   tca_foobar;
         };
};
---

This should work with old binaries and kernels.
It will work with someone referencing sizeof tcamsg.
It will work with unchanged source that memsets the struct.
It will work with things that explicitly set the pad1 and pad2.

What is the downside?

cheers,
jamal