netdev - Re: [PATCH net-next v6] net: sched: Introduce act

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]

Date:   Wed, 12 Jun 2019 20:56:10 +0200
From:   Johannes Berg <johannes@...solutions.net>
To:     Jakub Kicinski <jakub.kicinski@...ronome.com>,
        Marcelo Ricardo Leitner <marcelo.leitner@...il.com>
Cc:     Kevin 'ldir' Darbyshire-Bryant <ldir@...byshire-bryant.me.uk>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Paul Blakey <paulb@...lanox.com>,
        John Hurley <john.hurley@...ronome.com>,
        Toke Høiland-Jørgensen <toke@...hat.com>,
        Michal Kubecek <mkubecek@...e.cz>,
        "dcaratti@...hat.com" <dcaratti@...hat.com>,
        David Ahern <dsahern@...il.com>
Subject: Re: [PATCH net-next v6] net: sched: Introduce act_ctinfo action

(switching to my personal email)

> > I can't add these actions with current net-next and iproute-next:
> > # ~/iproute2/tc/tc action add action ctinfo dscp 0xfc000000 0x01000000
> > Error: NLA_F_NESTED is missing.
> > We have an error talking to the kernel
> > 
> > This also happens with the current post of act_ct and should also
> > happen with the act_mpls post (thus why Cc'ing John as well).
> > 
> > I'm not sure how we should fix this. In theory the kernel can't get
> > stricter with userspace here, as that breaks user applications as
> > above, so older actions can't use the more stricter parser. Should we
> > have some actions behaving one way, and newer ones in a different way?
> > That seems bad.

I think you could just fix all of the actions in userspace, since the
older kernel would allow both with and without the flag, and then from a
userspace POV it all behaves the same, just the kernel accepts some
things without the flag for compatibility with older iproute2?

> > Or maybe all actions should just use nla_parse_nested_deprecated()?
> > I'm thinking this last. Yet, then the _deprecated suffix may not make
> > much sense here. WDYT?
> 
> Surely for new actions we can require strict validation, there is
> no existing user space to speak of..  

That was the original idea.

> Perhaps act_ctinfo and act_ct
> got slightly confused with the race you described, but in principle
> there is nothing stopping new actions from implementing the user space
> correctly, right?

There's one potential thing where you have a new command in netlink
(which thus will use strict validation), but you use existing code in
userspace to build the netlink message or parts thereof?

But then again you can just fix that while you test it, and the current
and older kernel will accept the stricter version for the existing use
of the existing code too, right?

johannes