netdev - Re: [PATCH net-next v11 1/4] net netlink: Add new type NLA_BITFIELD

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:   Fri, 28 Jul 2017 17:55:08 -0400
From:   Jamal Hadi Salim <jhs@...atatu.com>
To:     David Ahern <dsahern@...il.com>, davem@...emloft.net
Cc:     netdev@...r.kernel.org, jiri@...nulli.us, xiyou.wangcong@...il.com,
        eric.dumazet@...il.com, mrv@...atatu.com,
        simon.horman@...ronome.com, alex.aring@...il.com
Subject: Re: [PATCH net-next v11 1/4] net netlink: Add new type
 NLA_BITFIELD_32

On 17-07-28 11:13 AM, David Ahern wrote:
> On 7/28/17 9:04 AM, Jamal Hadi Salim wrote:
>>
>> Kernel side checking for device ifindex must know what a device
>> ifindex means.
>> That doesnt disqualify that the generic code checks that it
>> is of the same size as a signed 32b, etc. That is generic
>> stuff that can be factored out.
>>
>> In this case:
>> Checking for whether bits selected are in the allowed range
>> that the kernel understands, that the bit value are set in
>> the right bit position, that the bits set in the correct bit
>> value position are also selected in the transaction.
>> That is generic code (which the content validation does).
> 
> Create a helper function then. It's the validation of attribute content
> in 2 places that I object to. 1 attribute is validated in 1 place
> (generic infra for this bitfield attribute), the others are validated in
> line. Asymmetric validations is not a good design.
> 

What is not generic in what is being validated though?
Content validation is when you say "This bit means the sky is blue".
You have to know the meaning of the bit.
The generic validation is opaque; "this bit is not supposed to be here".
I would argue that "this is bit is not supposed to be here unless
this other bit is present" is had to generalize but would be infra
as well (why i provided the ops for it, but removed it so we could
move on).

cheers,
jamal