netdev - Re: [PATCH v2 net-next] wireguard: allowedips: Add WGALLOWEDIP_F_REMOVE

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]

Message-ID: <20241130095624.1c34a12c@kernel.org>
Date: Sat, 30 Nov 2024 09:56:24 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: Jordan Rife <jrife@...gle.com>
Cc: Jason@...c4.com, davem@...emloft.net, edumazet@...gle.com,
 linux-kselftest@...r.kernel.org, netdev@...r.kernel.org, pabeni@...hat.com,
 shuah@...nel.org, wireguard@...ts.zx2c4.com
Subject: Re: [PATCH v2 net-next] wireguard: allowedips: Add
 WGALLOWEDIP_F_REMOVE_ME flag

On Wed, 27 Nov 2024 23:21:33 +0000 Jordan Rife wrote:
> The second command fails with "Invalid argument" (EINVAL) on the
> unpatched kernel. This simplifies things, as there's no need for
> clients to explicitly probe to see if this attribute is supported. I
> will do the following:
> 
> 1. Revert WG_GENL_VERSION back to 1.
> 2. Add a check for new flags similar to the one you mentioned for
>    WGPEER_A_FLAGS.
> 
>         if (attrs[WGPEER_A_FLAGS])
>                 flags = nla_get_u32(attrs[WGPEER_A_FLAGS]);
>         ret = -EOPNOTSUPP;
>         if (flags & ~__WGPEER_F_ALL)
>                 goto out;
> 
> This should be sufficient. We might want to consider how best to bubble
> this error up to users. In the case of wg, "Invalid argument" may not be
> very helpful in determining where you went wrong. We could always detect
> when EINVAL is returned in response to an operation that sets
> WGALLOWEDIP_A_FLAGS and print something more helpful like "Operation not
> supported on this kernel". However, these are details that can be worked
> out.

Better still use NLA_POLICY_MASK() so that nla_parse_nested() can
perform the validation and attach a machine readable info about
the failure.