netdev - Re: [PATCH v2 net-next 13/15] ipv6: anycast: Unify two error paths in ipv6_sock_ac

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: <CAAVpQUBWepaZYZvPjNMzSYiERWuvM15wtBJE+y5iWavO+saCqA@mail.gmail.com>
Date: Thu, 26 Jun 2025 18:02:22 -0700
From: Kuniyuki Iwashima <kuniyu@...gle.com>
To: Eric Dumazet <edumazet@...gle.com>
Cc: Kuniyuki Iwashima <kuni1840@...il.com>, "David S. Miller" <davem@...emloft.net>, 
	David Ahern <dsahern@...nel.org>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, 
	Simon Horman <horms@...nel.org>, netdev@...r.kernel.org
Subject: Re: [PATCH v2 net-next 13/15] ipv6: anycast: Unify two error paths in ipv6_sock_ac_join().

On Thu, Jun 26, 2025 at 7:52 AM Eric Dumazet <edumazet@...gle.com> wrote:
>
> On Tue, Jun 24, 2025 at 1:26 PM Kuniyuki Iwashima <kuni1840@...il.com> wrote:
> >
> > From: Kuniyuki Iwashima <kuniyu@...gle.com>
> >
> > The next patch will replace __dev_get_by_index() and __dev_get_by_flags()
> > to RCU + refcount version.
> >
> > Then, we will need to call dev_put() in some error paths.
> >
> > Let's unify two error paths to make the next patch cleaner.
> >
> > Signed-off-by: Kuniyuki Iwashima <kuniyu@...gle.com>
> > ---
> >  net/ipv6/anycast.c | 22 ++++++++++++++--------
> >  1 file changed, 14 insertions(+), 8 deletions(-)
> >
> > diff --git a/net/ipv6/anycast.c b/net/ipv6/anycast.c
> > index 8440e7b27f6d..e0a1f9d7622c 100644
> > --- a/net/ipv6/anycast.c
> > +++ b/net/ipv6/anycast.c
> > @@ -67,12 +67,11 @@ static u32 inet6_acaddr_hash(const struct net *net,
> >  int ipv6_sock_ac_join(struct sock *sk, int ifindex, const struct in6_addr *addr)
> >  {
> >         struct ipv6_pinfo *np = inet6_sk(sk);
> > +       struct ipv6_ac_socklist *pac = NULL;
> > +       struct net *net = sock_net(sk);
> >         struct net_device *dev = NULL;
> >         struct inet6_dev *idev;
> > -       struct ipv6_ac_socklist *pac;
> > -       struct net *net = sock_net(sk);
> > -       int     ishost = !net->ipv6.devconf_all->forwarding;
> > -       int     err = 0;
> > +       int err = 0, ishost;
> >
> >         ASSERT_RTNL();
> >
> > @@ -84,15 +83,22 @@ int ipv6_sock_ac_join(struct sock *sk, int ifindex, const struct in6_addr *addr)
> >         if (ifindex)
> >                 dev = __dev_get_by_index(net, ifindex);
> >
> > -       if (ipv6_chk_addr_and_flags(net, addr, dev, true, 0, IFA_F_TENTATIVE))
> > -               return -EINVAL;
> > +       if (ipv6_chk_addr_and_flags(net, addr, dev, true, 0, IFA_F_TENTATIVE)) {
> > +               err = -EINVAL;
> > +               goto error;
> > +       }
> >
> >         pac = sock_kmalloc(sk, sizeof(struct ipv6_ac_socklist), GFP_KERNEL);
> > -       if (!pac)
> > -               return -ENOMEM;
> > +       if (!pac) {
> > +               err = -ENOMEM;
> > +               goto error;
> > +       }
> > +
> >         pac->acl_next = NULL;
> >         pac->acl_addr = *addr;
> >
> > +       ishost = !net->ipv6.devconf_all->forwarding;
>
> RTNL will no longer protect this read, you should add a READ_ONCE()

Ah exactly.  Will use it.

Thank you!


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