Re: [PATCH v2 5/9] seq_buf: Add seq_buf_strlen()

[David Laight]

On Sun, 20 Sep 2026 06:34:32 +0100
Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:

> On Sat, Sep 19, 2026 at 02:15:58PM -0700, Kees Cook wrote:
> > On Sat, Sep 19, 2026 at 08:38:37AM +0100, Greg KH wrote:  
> > > On Fri, Sep 18, 2026 at 05:27:03PM -0700, Kees Cook wrote:  
> > > > +static inline size_t seq_buf_strlen(struct seq_buf *s)
> > > > +{
> > > > +	if (WARN_ON(s->size == 0))
> > > > +		return 0;  
> > > 
> > > Why WARN_ON()?  Are you wanting to just mint new CVEs with this code
> > > path, do we not give out enough already?  :)
> > > 
> > > I can see returning 0, if it's empty, but isn't that a valid check for
> > > people to wish to know at times?  Why crash the box?  (remember about
> > > panic-on-warn being enabled in a few billion Linux instances...)  
> > 
> > We have to figure out a line somewhere. :P Making a seq_buf with size 0
> > is a nonsense construction, but seq_buf_init is non-allocating, so
> > there's no feedback about setting it to size 0. We could move the WARN
> > to the init? I was just following the existing style here.  
> 
> WARN on the init makes more sense, but even then it feels odd as if we
> wanted to make a seq_buf with data from a device or userspace, we would
> have to verify the size is non-zero _before_ creating the seq_buf or we
> would crash.  So someone has to check the "untrusted" data somewhere,
> right?
> 
> And why can't we have buffers of 0 size work just fine?  What prevents
> that?  People have "empty" strings for lots of things.

If someone passes 0 to an allocate you might be able to use a global
char[1] buffer (that always contains 0) just to keep everything happy.
In particular you can return a '\0' terminated string without adding
conditionals anywhere else.

David

> 
> thanks,
> 
> greg k-h
>