netdev - Re: [PATCH v2 net-next] udp_diag: fix udp_dump_one() vs SO

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: <1452038884.8255.154.camel@edumazet-glaptop2.roam.corp.google.com>
Date:	Tue, 05 Jan 2016 16:08:04 -0800
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	Craig Gallek <kraigatgoog@...il.com>
Cc:	David Miller <davem@...emloft.net>, netdev@...r.kernel.org
Subject: Re: [PATCH v2 net-next] udp_diag: fix udp_dump_one() vs SO_REUSEPORT

On Tue, 2016-01-05 at 17:56 -0500, Craig Gallek wrote:
> On Tue, Jan 5, 2016 at 5:33 PM, Eric Dumazet <eric.dumazet@...il.com> wrote:
> > From: Eric Dumazet <edumazet@...gle.com>
> > +       unsigned short hnum = ntohs(req->id.idiag_dport);
> > +       unsigned int slot = udp_hashfn(net, hnum, tbl->mask);
> > +       struct udp_hslot *hslot = &tbl->hash[slot];
> > +       struct hlist_nulls_node *node;
> > +       int err = -ENOENT;
> > +
> > +       spin_lock_bh(&hslot->lock);
> > +       sk_nulls_for_each(aux, node, &hslot->head) {
> > +               if (net_eq(sock_net(aux), net) &&
> > +                   !sock_diag_check_cookie(aux, req->id.idiag_cookie) &&
> > +                   (req->sdiag_family == AF_UNSPEC ||
> > +                    req->sdiag_family == aux->sk_family)) {
> > +                       sk = aux;
> > +                       sock_hold(sk);
> > +                       break;
> > +               }
> > +       }
> > +       spin_unlock_bh(&hslot->lock);
> >         if (!sk)
> >                 goto out_nosk;
> >
> > -       err = sock_diag_check_cookie(sk, req->id.idiag_cookie);
> > -       if (err)
> > -               goto out;
> > -
> sock_diag_check_cookie will return successfully if the cookie in the
> request is INET_DIAG_NOCOOKIE before even considering the socket.  I
> think this could cause this loop to prematurely terminate.

Hmmm...

I never considered passing a INET_DIAG_NOCOOKIE there.

(Actually I am not sure if this interface is used. It is not in
iproute2)


> 
> Also, previously this would return ESTALE on a cookie error, now it
> returns ENOENT.  Not sure if this is a big deal or not, though...

Probably not a big deal.

I'll send a V3, thanks.


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html