Re: [PATCH RFC 0/4] net: add bpfilter

[Phil Sutter <phil@nwl.cc>]

Hi Michal,

On Tue, Feb 20, 2018 at 10:35:41AM +0100, Michal Kubecek wrote:
> On Mon, Feb 19, 2018 at 06:09:39PM +0100, Phil Sutter wrote:
> > What puzzles me about your argumentation is that you seem to propose for
> > the kernel to cover up flaws in userspace. Spinning this concept further
> > would mean that if there would be an old bug in iproute2 we should think
> > of adding a workaround to rtnetlink interface in kernel because
> > containers will keep the old iproute2 binary? Or am I (hopefully) just
> > missing your point?
> 
> Actually, that's what we already do. This is from rtnl_dump_ifinfo():
> 
> 	/* A hack to preserve kernel<->userspace interface.
> 	 * The correct header is ifinfomsg. It is consistent with rtnl_getlink.
> 	 * However, before Linux v3.9 the code here assumed rtgenmsg and that's
> 	 * what iproute2 < v3.9.0 used.
> 	 * We can detect the old iproute2. Even including the IFLA_EXT_MASK
> 	 * attribute, its netlink message is shorter than struct ifinfomsg.
> 	 */

The reason why this is in place (and should be IMHO) is that commit
88c5b5ce5cb57 ("rtnetlink: Call nlmsg_parse() with correct header
length") incompatibly changed uAPI.

I have a different example which reflects what I have in mind, namely
iproute2 commit 33f6dd23a51c4 ("ip fou: pass family attribute as u8")
which basically does:

| -       addattr16(n, 1024, FOU_ATTR_AF, family);
| +       addattr8(n, 1024, FOU_ATTR_AF, family);

If kernel cares about those userspace bugs, shouldn't the better fix be
to make it expect u16 in FOU_ATTR_AF and check whether the high or low
byte contains the expected value?

Cheers, Phil