Skip site navigation (1)Skip section navigation (2) 
Date:      Sat, 14 Jun 2025 07:12:17 -0600
From:      Warner Losh <imp@bsdimp.com>
To:        John Baldwin <jhb@freebsd.org>
Cc:        Warner Losh <imp@freebsd.org>, src-committers@freebsd.org,  dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org
Subject:   Re: git: aae67a2c2b66 - main - mfiutil: Fix unsafe assumptions of snprintf(3) return value
Message-ID:  <CANCZdfpCTbOvSxFk6DaVemA_aBRMm-ueo%2BsFbuvHDPfsBfmvfQ@mail.gmail.com>
In-Reply-To: <50830604-3bd8-47d6-920c-fd099a96a08e@FreeBSD.org>
References:  <202506130121.55D1LhXF086456@gitrepo.freebsd.org> <50830604-3bd8-47d6-920c-fd099a96a08e@FreeBSD.org>
index | next in thread | previous in thread | raw e-mail 

On Sat, Jun 14, 2025 at 6:42 AM John Baldwin <jhb@freebsd.org> wrote:
>
> On 6/12/25 21:21, Warner Losh wrote:
> > The branch main has been updated by imp:
> >
> > URL: https://cgit.FreeBSD.org/src/commit/?id=aae67a2c2b663a6bce8fbc087ff8490336b8618f
> >
> > commit aae67a2c2b663a6bce8fbc087ff8490336b8618f
> > Author:     WHR <whr@rivoreo.one>
> > AuthorDate: 2024-09-03 10:19:04 +0000
> > Commit:     Warner Losh <imp@FreeBSD.org>
> > CommitDate: 2025-06-13 01:21:44 +0000
> >
> >      mfiutil: Fix unsafe assumptions of snprintf(3) return value
> >
> >      PR: 281160
> >      Reviewed by: imp
> >      Pull Request: https://github.com/freebsd/freebsd-src/pull/1405
> >      Closes: https://github.com/freebsd/freebsd-src/pull/1405
> > ---
> >   usr.sbin/mfiutil/mfi_bbu.c | 17 +++++++++++++++--
> >   1 file changed, 15 insertions(+), 2 deletions(-)
> >
> > diff --git a/usr.sbin/mfiutil/mfi_bbu.c b/usr.sbin/mfiutil/mfi_bbu.c
> > index 9075c4d0ddd0..e97227d47c70 100644
> > --- a/usr.sbin/mfiutil/mfi_bbu.c
> > +++ b/usr.sbin/mfiutil/mfi_bbu.c
> > @@ -50,10 +50,23 @@ mfi_autolearn_period(uint32_t period, char *buf, size_t sz)
> >
> >       tmp = buf;
> >       if (d != 0) {
> > -             tmp += snprintf(buf, sz, "%u day%s", d, d == 1 ? "" : "s");
> > +             int fmt_len;
> > +             fmt_len = snprintf(buf, sz, "%u day%s", d, d == 1 ? "" : "s");
> > +             if (fmt_len < 0) {
> > +                     *buf = 0;
> > +                     return;
> > +             }
> > +             if ((size_t)fmt_len >= sz) {
> > +                     return;
> > +             }
> > +             tmp += fmt_len;
> >               sz -= tmp - buf;
> >               if (h != 0) {
> > -                     tmp += snprintf(tmp, sz, ", ");
> > +                     fmt_len = snprintf(tmp, sz, ", ");
> > +                     if (fmt_len < 0 || (size_t)fmt_len >= sz) {
> > +                             return;
> > +                     }
> > +                     tmp += fmt_len;
> >                       sz -= 2;
> >               }
> >       }
>
> It seems like using a string builder like fmemopen() or sbuf() would be
> better here than fragile dances with snprintf().

True. This is better than what was there, but either of those would be better.

Warner
help

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CANCZdfpCTbOvSxFk6DaVemA_aBRMm-ueo%2BsFbuvHDPfsBfmvfQ>