On Sat, Jun 14, 2025 at 6:42 AM John Baldwin <j...@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 <w...@rivoreo.one> > > AuthorDate: 2024-09-03 10:19:04 +0000 > > Commit: Warner Losh <i...@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
