netdev - Re: [BUG iproute2] Netkit unusable in batch mode

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: <1cfae3f3-d1cf-413e-8659-a6bd72b03a71@iogearbox.net>
Date: Wed, 11 Jun 2025 16:07:01 +0200
From: Daniel Borkmann <daniel@...earbox.net>
To: Alexandre Ferrieux <alexandre.ferrieux@...il.com>
Cc: Linux Kernel Network Developers <netdev@...r.kernel.org>,
 Nicolas Dichtel <nicolas.dichtel@...nd.com>,
 Nikolay Aleksandrov <razor@...ckwall.org>
Subject: Re: [BUG iproute2] Netkit unusable in batch mode

Hi Alexandre,

On 6/11/25 2:08 PM, Alexandre Ferrieux wrote:
[...]
> Playing around with netkit to circumvent veth performance issues, I stumbled
> upon a strange thing in iplink_netkit.c : the presence of three static variables
> which tend to wreak havoc in case of multiple netlink commands in batch mode (ip
> -b) : "seen_mode", "seen_peer", and "data".

Hah, never used batch mode :/

> As a consequence, the following simple batch sequence systematically fails:
> 
>      # ip -b - <<EOF
>      link add a1 type netkit peer a2
>      link add b1 type netkit peer b2
>      EOF
>      *    Error: duplicate "peer": "b2" is the second value.*
> 
> While the patch below solves the problem, I wonder: why in the first place are
> these three locals declared static ? Is there a scenario where
> netkit_parse_opt() is called several times in a single command, but in a
> stateful manner ?
> 
> Thanks for any clarification
> 
> -Alex
> 
> diff --git a/ip/iplink_netkit.c b/ip/iplink_netkit.c
> index 818da119..de1681b9 100644
> --- a/ip/iplink_netkit.c
> +++ b/ip/iplink_netkit.c
> @@ -48,8 +48,8 @@ static int netkit_parse_opt(struct link_util *lu, int argc,
> char **argv,
>   {
>          __u32 ifi_flags, ifi_change, ifi_index;
>          struct ifinfomsg *ifm, *peer_ifm;
> -       static bool seen_mode, seen_peer;
> -       static struct rtattr *data;
> +       bool seen_mode=false, seen_peer=false;
> +       struct rtattr *data=NULL;
>          int err;
> 
>          ifm = NLMSG_DATA(n);

This was basically because we call into iplink_parse() after "peer" to gather settings
for the peer similar to link_veth.c modulo that netkit has mode/policy/scrub settings
and we only want them to be present once for a single 'link add' call. Might be better
to just reset the above before the 'goto out_ok' after parsing peer. Could you cook a
patch for that instead?

Thanks,
Daniel