netdev - Re: [PATCHv3 net-next] IPv6: add extack info for IPv6 address add/delete

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: <a9854c98-627f-a496-5c0a-1cbf3f050e4f@kernel.org>
Date: Tue, 25 Jul 2023 17:38:12 -0600
From: David Ahern <dsahern@...nel.org>
To: Hangbin Liu <liuhangbin@...il.com>
Cc: netdev@...r.kernel.org, "David S . Miller" <davem@...emloft.net>,
 Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>, Ido Schimmel <idosch@...sch.org>,
 Beniamino Galvani <bgalvani@...hat.com>
Subject: Re: [PATCHv3 net-next] IPv6: add extack info for IPv6 address
 add/delete

On 7/25/23 1:45 AM, Hangbin Liu wrote:
>>> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
>>> index 19eb4b3d26ea..53dea18a4a07 100644
>>> --- a/net/ipv6/addrconf.c
>>> +++ b/net/ipv6/addrconf.c
>>> @@ -1068,15 +1068,19 @@ ipv6_add_addr(struct inet6_dev *idev, struct ifa6_config *cfg,
>>>  	     !(cfg->ifa_flags & IFA_F_MCAUTOJOIN)) ||
>>>  	    (!(idev->dev->flags & IFF_LOOPBACK) &&
>>>  	     !netif_is_l3_master(idev->dev) &&
>>> -	     addr_type & IPV6_ADDR_LOOPBACK))
>>> +	     addr_type & IPV6_ADDR_LOOPBACK)) {
>>> +		NL_SET_ERR_MSG(extack, "Cannot assign requested address");
>>>  		return ERR_PTR(-EADDRNOTAVAIL);
>>> +	}
>>
>> It would be good to split the above checks into separate ones with more
>> specific messages.
> 
> How about this.
> 
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index 53dea18a4a07..e6c3fe413441 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -1063,13 +1063,17 @@ ipv6_add_addr(struct inet6_dev *idev, struct ifa6_config *cfg,
>         struct fib6_info *f6i = NULL;
>         int err = 0;
> 
> -       if (addr_type == IPV6_ADDR_ANY ||
> -           (addr_type & IPV6_ADDR_MULTICAST &&
> -            !(cfg->ifa_flags & IFA_F_MCAUTOJOIN)) ||
> -           (!(idev->dev->flags & IFF_LOOPBACK) &&
> -            !netif_is_l3_master(idev->dev) &&
> -            addr_type & IPV6_ADDR_LOOPBACK)) {
> -               NL_SET_ERR_MSG(extack, "Cannot assign requested address");
> +       if (addr_type == IPV6_ADDR_ANY) {
> +               NL_SET_ERR_MSG(extack, "IPv6: Cannot assign unspecified address");

Maybe just "IPv6: Invalid address".

Also, that reminds me that IPv6 should be using NL_SET_ERR_MSG_MOD which
will add the IPv6 part.



> +               return ERR_PTR(-EADDRNOTAVAIL);
> +       } else if (addr_type & IPV6_ADDR_MULTICAST &&
> +                  !(cfg->ifa_flags & IFA_F_MCAUTOJOIN)) {
> +               NL_SET_ERR_MSG(extack, "IPv6: Cannot assign multicast address without \"IFA_F_MCAUTOJOIN\" flag");
> +               return ERR_PTR(-EADDRNOTAVAIL);
> +       } else if (!(idev->dev->flags & IFF_LOOPBACK) &&
> +                  !netif_is_l3_master(idev->dev) &&
> +                  addr_type & IPV6_ADDR_LOOPBACK) {
> +               NL_SET_ERR_MSG(extack, "IPv6: Cannot assign loopback address on this device");
>                 return ERR_PTR(-EADDRNOTAVAIL);
>         }
> 
>> ...
>>
>>>  
>>>  	if (cfg->ifa_flags & IFA_F_MCAUTOJOIN) {
>>>  		int ret = ipv6_mc_config(net->ipv6.mc_autojoin_sk,
>>>  					 true, cfg->pfx, ifindex);
>>
>> and pass extack to this one for better message as well.
> 
> This one looks a little deep. We need pass extack to
> - ipv6_mc_config
>   - ipv6_sock_mc_join
>     - __ipv6_sock_mc_join
>       - __ipv6_dev_mc_inc	maybe also this one??
> 
> to get more detailed message. And all these are "Join multicast group failed".
> Do you still want to do this?

ok, staring at the errors for that stack, not much value I guess.