netdev - Re: [PATCH net-next v5 1/2] 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]

Date:   Mon, 24 Apr 2017 18:18:42 -0400
From:   Jamal Hadi Salim <jhs@...atatu.com>
To:     David Miller <davem@...emloft.net>
Cc:     simon.horman@...ronome.com, jiri@...nulli.us,
        xiyou.wangcong@...il.com, eric.dumazet@...il.com,
        netdev@...r.kernel.org, tom@...bertland.com, pablo@...filter.org
Subject: Re: [PATCH net-next v5 1/2] net sched actions: dump more than
 TCA_ACT_MAX_PRIO actions per batch

On 17-04-24 04:30 PM, David Miller wrote:

> Which is fine.  But two things:
>
> 1) Again, bits you aren't using now, make sure userspace doesn't
>    set them.  And if it does, reject.
>

I meet those goals on the bit checks but i went a slightly different
path with a patch I posted[1]

With the posted patch: unknow bits set will result in a kernel rejection
unless the user space explicitly ask the kernel to ignore flags it
doesnt understand and just handles what it knows. This reduces the
amount of work in tc.

If this ok I will resend tomorrow.

> 2) If you are worried about performance, we're talking about a TLV in
>    the request here not the dump response itself so performance isn't
>    a real issue as Pablo mentioned.

doesnt make much of a difference for a simple request, true; i was more
worried about how we pack similar things for dumps or for large
set requests in general. And note it makes no difference if i make the
bitmask u32 or u16 - the TLV still eats 32 + 32 bits. So using a u32
is sensible.

cheers,
jamal

[1]
This is because i worry about making large changes to
the behavior of user space apps like tc. If I reject I will need to
change tc to detect this rejection and retry (and I dont think
extended ACKs in their current shape are ready to provide any
meaningful detail).