netdev - Re: [PATCH net-next 4/4] inetpeer: do not get a refcount in inet

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CANn89iJgOA3SUfT_vEujxSpHjbGkr1yAmHYKw6dbnjfZ0ZuJow@mail.gmail.com>
Date: Sun, 15 Dec 2024 18:42:15 +0100
From: Eric Dumazet <edumazet@...gle.com>
To: Ido Schimmel <idosch@...sch.org>
Cc: "David S . Miller" <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, 
	Paolo Abeni <pabeni@...hat.com>, netdev@...r.kernel.org, Simon Horman <horms@...nel.org>, 
	David Ahern <dsahern@...nel.org>, Kuniyuki Iwashima <kuniyu@...zon.com>, eric.dumazet@...il.com
Subject: Re: [PATCH net-next 4/4] inetpeer: do not get a refcount in inet_getpeer()

On Sun, Dec 15, 2024 at 4:48 PM Ido Schimmel <idosch@...sch.org> wrote:
>
> On Fri, Dec 13, 2024 at 01:02:12PM +0000, Eric Dumazet wrote:
> > diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
> > index 5eeb9f569a706cf2766d74bcf1a667c8930804f2..7a1b1af2edcae0b0648ef3c3411b4ef36e6d9b14 100644
> > --- a/net/ipv4/icmp.c
> > +++ b/net/ipv4/icmp.c
> > @@ -322,11 +322,11 @@ static bool icmpv4_xrlim_allow(struct net *net, struct rtable *rt,
> >               goto out;
> >
> >       vif = l3mdev_master_ifindex(dst->dev);
> > +     rcu_read_lock();
> >       peer = inet_getpeer_v4(net->ipv4.peers, fl4->daddr, vif);
> >       rc = inet_peer_xrlim_allow(peer,
> >                                  READ_ONCE(net->ipv4.sysctl_icmp_ratelimit));
> > -     if (peer)
> > -             inet_putpeer(peer);
> > +     rcu_read_unlock();
> >  out:
> >       if (!rc)
> >               __ICMP_INC_STATS(net, ICMP_MIB_RATELIMITHOST);
>
> Maybe convert l3mdev_master_ifindex() to l3mdev_master_ifindex_rcu() and
> move it into the RCU critical section?
>
> diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
> index 7a1b1af2edca..094084b61bff 100644
> --- a/net/ipv4/icmp.c
> +++ b/net/ipv4/icmp.c
> @@ -312,7 +312,6 @@ static bool icmpv4_xrlim_allow(struct net *net, struct rtable *rt,
>         struct dst_entry *dst = &rt->dst;
>         struct inet_peer *peer;
>         bool rc = true;
> -       int vif;
>
>         if (!apply_ratelimit)
>                 return true;
> @@ -321,9 +320,9 @@ static bool icmpv4_xrlim_allow(struct net *net, struct rtable *rt,
>         if (dst->dev && (dst->dev->flags&IFF_LOOPBACK))
>                 goto out;
>
> -       vif = l3mdev_master_ifindex(dst->dev);
>         rcu_read_lock();
> -       peer = inet_getpeer_v4(net->ipv4.peers, fl4->daddr, vif);
> +       peer = inet_getpeer_v4(net->ipv4.peers, fl4->daddr,
> +                              l3mdev_master_ifindex_rcu(dst->dev));
>         rc = inet_peer_xrlim_allow(peer,
>                                    READ_ONCE(net->ipv4.sysctl_icmp_ratelimit));
>         rcu_read_unlock();
>
> [...]
>
> > @@ -975,9 +975,9 @@ static int ip_error(struct sk_buff *skb)
> >               break;
> >       }
> >
> > +     rcu_read_lock();
> >       peer = inet_getpeer_v4(net->ipv4.peers, ip_hdr(skb)->saddr,
> >                              l3mdev_master_ifindex(skb->dev));
> > -
> >       send = true;
> >       if (peer) {
> >               now = jiffies;
>
> And here?
>
> diff --git a/net/ipv4/route.c b/net/ipv4/route.c
> index d2086648dcf1..9f9d4e6ea1b9 100644
> --- a/net/ipv4/route.c
> +++ b/net/ipv4/route.c
> @@ -977,7 +977,7 @@ static int ip_error(struct sk_buff *skb)
>
>         rcu_read_lock();
>         peer = inet_getpeer_v4(net->ipv4.peers, ip_hdr(skb)->saddr,
> -                              l3mdev_master_ifindex(skb->dev));
> +                              l3mdev_master_ifindex_rcu(skb->dev));
>         send = true;
>         if (peer) {
>                 now = jiffies;

Good ideas, I will add this to V2, thanks !