netdev - Re: [PATCH v3 bpf-next 2/6] bpf: udp: Make sure iter->batch always contains a full bucket snapshot

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: <20250418001530.45213-1-kuniyu@amazon.com>
Date: Thu, 17 Apr 2025 17:15:08 -0700
From: Kuniyuki Iwashima <kuniyu@...zon.com>
To: <jordan@...fe.io>
CC: <aditi.ghag@...valent.com>, <bpf@...r.kernel.org>, <daniel@...earbox.net>,
	<kuniyu@...zon.com>, <martin.lau@...ux.dev>, <netdev@...r.kernel.org>,
	<willemdebruijn.kernel@...il.com>
Subject: Re: [PATCH v3 bpf-next 2/6] bpf: udp: Make sure iter->batch always contains a full bucket snapshot

From: Jordan Rife <jordan@...fe.io>
Date: Thu, 17 Apr 2025 16:57:11 -0700
> > > @@ -3454,15 +3460,26 @@ static struct sock *bpf_iter_udp_batch(struct seq_file *seq)
> > >  				batch_sks++;
> > >  			}
> > >  		}
> > > -		spin_unlock_bh(&hslot2->lock);
> > >  
> > >  		if (iter->end_sk)
> > >  			break;
> > > +next_bucket:
> > > +		/* Somehow the bucket was emptied or all matching sockets were
> > > +		 * removed while we held onto its lock. This should not happen.
> > > +		 */
> > > +		if (WARN_ON_ONCE(!resizes))
> > > +			/* Best effort; reset the resize budget and move on. */
> > > +			resizes = MAX_REALLOC_ATTEMPTS;
> > > +		if (lock)
> > > +			spin_unlock_bh(lock);
> > > +		lock = NULL;
> > >  	}
> > >  
> > >  	/* All done: no batch made. */
> > >  	if (!iter->end_sk)
> > > -		return NULL;
> > > +		goto done;
> > 
> > If we jump here when no UDP socket exists, uninitialised sk is returned.
> > Maybe move this condition down below the sk initialisation.
> 
> In this case, we'd want to return NULL just like it did before, since
> there's no socket in the batch. Do you want me to make this more
> explicit by setting sk = NULL here?

Sounds good to me