netdev - Re: [PATCH net-next v2 3/3] genetlink: fit NLMSG

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: Thu, 21 Mar 2024 18:26:31 +0100
From: Eric Dumazet <edumazet@...gle.com>
To: Ido Schimmel <idosch@...sch.org>
Cc: Gal Pressman <gal.pressman@...ux.dev>, Jakub Kicinski <kuba@...nel.org>, 
	Stefano Brivio <sbrivio@...hat.com>, davem@...emloft.net, netdev@...r.kernel.org, 
	pabeni@...hat.com, jiri@...nulli.us, johannes@...solutions.net, fw@...len.de, 
	pablo@...filter.org, Martin Pitt <mpitt@...hat.com>, 
	Paul Holzinger <pholzing@...hat.com>, David Gibson <david@...son.dropbear.id.au>
Subject: Re: [PATCH net-next v2 3/3] genetlink: fit NLMSG_DONE into same
 read() as families

On Thu, Mar 21, 2024 at 2:51 PM Ido Schimmel <idosch@...sch.org> wrote:
>
> On Thu, Mar 21, 2024 at 02:56:41PM +0200, Gal Pressman wrote:
> > We've encountered a new issue recently which I believe is related to
> > this discussion.
> >
> > Following Eric's patch:
> > 9cc4cc329d30 ("ipv6: use xa_array iterator to implement inet6_dump_addr()")
> >
> > Setting the interface mtu to < 1280 results in 'ip addr show eth2'
> > returning an error, because the ipv6 dump fails. This is a degradation
> > from the user's perspective.
> >
> > # ip addr show eth2
> > 4: eth2: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN group
> > default qlen 1000
> >     link/ether 24:42:53:21:52:44 brd ff:ff:ff:ff:ff:ff
> >     altname enp6s0f0np0
> > # ip link set dev eth2 mtu 1000
> > # ip addr show eth2
> > RTNETLINK answers: No such device
> > Dump terminated
>
> I don't think it's the same issue. Original issue was about user space
> not knowing how to handle NLMSG_DONE being sent together with dump
> responses. The issue you reported seems to be related to an
> unintentional change in the return code when IPv6 is disabled on an
> interface. Can you please test the following patch?
>
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index 247bd4d8ee45..92db9b474f2b 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -5416,10 +5416,11 @@ static int inet6_dump_addr(struct sk_buff *skb, struct netlink_callback *cb,
>
>                 err = 0;
>                 if (fillargs.ifindex) {
> -                       err = -ENODEV;
>                         dev = dev_get_by_index_rcu(tgt_net, fillargs.ifindex);
> -                       if (!dev)
> +                       if (!dev) {
> +                               err = -ENODEV;
>                                 goto done;
> +                       }
>                         idev = __in6_dev_get(dev);
>                         if (idev)
>                                 err = in6_dump_addrs(idev, skb, cb,

Good catch, thanks !

Not sure why ip command is so picky about the error code here.

There is no IPv6 on this device after all.

The following seems to react quite differently :

# ip addr  show lo
1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN
group default qlen 1000
    link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
    inet 127.0.0.1/8 scope host lo
       valid_lft forever preferred_lft forever
    inet6 ::1/128 scope host proto kernel_lo
       valid_lft forever preferred_lft forever
# ip link set dev lo mtu 1000
# ip addr  show lo
1: lo: <LOOPBACK,UP,LOWER_UP> mtu 1000 qdisc noqueue state UNKNOWN
group default qlen 1000
    link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
    inet 127.0.0.1/8 scope host lo
       valid_lft forever preferred_lft forever



Reviewed-by: Eric Dumazet <edumazet@...gle.com>