netdev - Re: [PATCH net-next v4 04/12] net: netlink: add NLM_F

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] [day] [month] [year] [list]

Message-ID: <99b9a532-5feb-fe00-3d4e-29d560d34dc0@6wind.com>
Date:   Wed, 21 Sep 2022 08:43:37 +0200
From:   Nicolas Dichtel <nicolas.dichtel@...nd.com>
To:     Nikolay Aleksandrov <razor@...ckwall.org>, netdev@...r.kernel.org
Cc:     dsahern@...nel.org, roopa@...dia.com, idosch@...sch.org,
        kuba@...nel.org, davem@...emloft.net,
        bridge@...ts.linux-foundation.org
Subject: Re: [PATCH net-next v4 04/12] net: netlink: add NLM_F_BULK delete
 request modifier


Le 20/09/2022 à 11:05, Nikolay Aleksandrov a écrit :
> On 20/09/2022 10:49, Nicolas Dichtel wrote:
>>
>> Le 13/04/2022 à 12:51, Nikolay Aleksandrov a écrit :
>>> Add a new delete request modifier called NLM_F_BULK which, when
>>> supported, would cause the request to delete multiple objects. The flag
>>> is a convenient way to signal that a multiple delete operation is
>>> requested which can be gradually added to different delete requests. In
>>> order to make sure older kernels will error out if the operation is not
>>> supported instead of doing something unintended we have to break a
>>> required condition when implementing support for this flag, f.e. for
>>> neighbors we will omit the mandatory mac address attribute.
>>> Initially it will be used to add flush with filtering support for bridge
>>> fdbs, but it also opens the door to add similar support to others.
>>>
>>> Signed-off-by: Nikolay Aleksandrov <razor@...ckwall.org>
>>> ---
>>>  include/uapi/linux/netlink.h | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/include/uapi/linux/netlink.h b/include/uapi/linux/netlink.h
>>> index 4c0cde075c27..855dffb4c1c3 100644
>>> --- a/include/uapi/linux/netlink.h
>>> +++ b/include/uapi/linux/netlink.h
>>> @@ -72,6 +72,7 @@ struct nlmsghdr {
>>>  
>>>  /* Modifiers to DELETE request */
>>>  #define NLM_F_NONREC	0x100	/* Do not delete recursively	*/
>>> +#define NLM_F_BULK	0x200	/* Delete multiple objects	*/
>> Sorry to reply to an old patch, but FWIW, this patch broke the uAPI.
>> One of our applications was using NLM_F_EXCL with RTM_DELTFILTER. This is
>> conceptually wrong but it was working. After this patch, the kernel returns an
>> error (EOPNOTSUPP).
>>
>> Here is the patch series:
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/log/?h=92716869375b
>>
>> We probably can't do anything now, but to avoid this in the future, I see only
>> two options:
>>  - enforce flags validation depending on the operation (but this may break some
>>    existing apps)
>>  - stop adding new flags that overlap between NEW and DEL operations (by adding
>>    a comment or defining dummy flags).
>>
>> Any thoughts?
>>
> 
> Personally I'd prefer to enforce validation so we don't lose the flags because of buggy user-space
> applications, but we can break someone (who arguably should fix their app though). We already had
> that discussion while the set was under review[1] and just to be a bit more confident I also
Thanks for the link. Finally, someone has (almost) complained :D

> tried searching for open-source buggy users, but didn't find any.
The trend seems to let someone else add another specific flag if needed. Thus,
it seems that checking flags is the way to go.
The pro is that if someone complains, the patch could be reverted, which is not
the case for a new feature like this bulk for example.


Regards,
Nicolas