On Tue, 17 Mar 2026 12:15:07 -0400 Steven Rostedt <[email protected]> wrote:
> On Tue, 17 Mar 2026 16:55:49 +0900 > Masami Hiramatsu (Google) <[email protected]> wrote: > > > > --- a/lib/bootconfig.c > > > +++ b/lib/bootconfig.c > > > @@ -319,10 +319,10 @@ int __init xbc_node_compose_key_after(struct > > > xbc_node *root, > > > depth ? "." : ""); > > > if (ret < 0) > > > return ret; > > > - if (ret >= size) { > > > + if (ret >= (int)size) { > > > > nit: > > > > if ((size_t)ret >= size) { > > > > because sizeof(size_t) > sizeof(int). > > I don't think we need to worry about this. But this does bring up an issue. > ret comes from: > > ret = snprintf(buf, size, "%s%s", xbc_node_get_data(node), > depth ? "." : ""); > > Where size is of type size_t > > snprintf() takes size_t but returns int. > > snprintf() calls vsnprintf() which has: > > size_t len, pos; > > Where pos is incremented based on fmt, and vsnprintf() returns: > > return pos; > > Which can overflow. I think that is vsnprintf() (maybe POSIX) design issue. I believe we're simply using the size_t to represent size of memory out of convention. > > Now, honestly, we should never have a 2Gig string as that would likely > cause other horrible things. Does size really need to be size_t? Even if so, it should be done in vsnprintf() instead of this. This function just believes that the caller gives collect size and enough amount of memory. Or, we need to check "INT_MAX > size" in everywhere. > > Perhaps we should have: > > if (WARN_ON_ONCE(size > MAX_INT)) > return -EINVAL; I think this is an over engineering effort especially in caller side. This overflow should be checked in vsnprintf() and should return -EINVAL. (and the caller checks the return value.) Thank you, > > ? > > -- Steve > -- Masami Hiramatsu (Google) <[email protected]>
