| 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
| ||
|
Date: Sat, 15 Jun 2019 06:58:36 -0400
From: Jeff Layton <jlayton@...nel.org>
To: "Yan, Zheng" <ukernel@...il.com>
Cc: Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
ceph-devel <ceph-devel@...r.kernel.org>,
Andrew Morton <akpm@...ux-foundation.org>,
Ilya Dryomov <idryomov@...il.com>, Zheng Yan <zyan@...hat.com>,
Sage Weil <sage@...hat.com>, agruenba@...hat.com
Subject: Re: [PATCH 1/3] lib/vsprintf: add snprintf_noterm
On Sat, 2019-06-15 at 10:41 +0800, Yan, Zheng wrote:
> On Fri, Jun 14, 2019 at 9:48 PM Jeff Layton <jlayton@...nel.org> wrote:
> > The getxattr interface returns a length after filling out the value
> > buffer, and the convention with xattrs is to not NULL terminate string
> > data.
> >
> > CephFS implements some virtual xattrs by using snprintf to fill the
> > buffer, but that always NULL terminates the string. If userland sends
> > down a buffer that is just the right length to hold the text without
> > termination then we end up truncating the value.
> >
> > Factor the formatting piece of vsnprintf into a separate helper
> > function, and have vsnprintf call that and then do the NULL termination
> > afterward. Then add a snprintf_noterm function that calls the new helper
> > to populate the string but skips the termination.
> >
> > Signed-off-by: Jeff Layton <jlayton@...nel.org>
> > ---
> > include/linux/kernel.h | 2 +
> > lib/vsprintf.c | 145 ++++++++++++++++++++++++++++-------------
> > 2 files changed, 103 insertions(+), 44 deletions(-)
> >
> > diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> > index 2d14e21c16c0..2f305a347482 100644
> > --- a/include/linux/kernel.h
> > +++ b/include/linux/kernel.h
> > @@ -462,6 +462,8 @@ extern int num_to_str(char *buf, int size,
> > extern __printf(2, 3) int sprintf(char *buf, const char * fmt, ...);
> > extern __printf(2, 0) int vsprintf(char *buf, const char *, va_list);
> > extern __printf(3, 4)
> > +int snprintf_noterm(char *buf, size_t size, const char *fmt, ...);
> > +extern __printf(3, 4)
> > int snprintf(char *buf, size_t size, const char *fmt, ...);
> > extern __printf(3, 0)
> > int vsnprintf(char *buf, size_t size, const char *fmt, va_list args);
> > diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> > index 791b6fa36905..ad5f4990eda3 100644
> > --- a/lib/vsprintf.c
> > +++ b/lib/vsprintf.c
> > @@ -2296,53 +2296,24 @@ set_precision(struct printf_spec *spec, int prec)
> > }
> >
> > /**
> > - * vsnprintf - Format a string and place it in a buffer
> > + * vsnprintf_noterm - Format a string and place it in a buffer without NULL
> > + * terminating it
> > * @buf: The buffer to place the result into
> > - * @size: The size of the buffer, including the trailing null space
> > + * @end: The end of the buffer
> > * @fmt: The format string to use
> > * @args: Arguments for the format string
> > *
> > - * This function generally follows C99 vsnprintf, but has some
> > - * extensions and a few limitations:
> > - *
> > - * - ``%n`` is unsupported
> > - * - ``%p*`` is handled by pointer()
> > - *
> > - * See pointer() or Documentation/core-api/printk-formats.rst for more
> > - * extensive description.
> > - *
> > - * **Please update the documentation in both places when making changes**
> > - *
> > - * The return value is the number of characters which would
> > - * be generated for the given input, excluding the trailing
> > - * '\0', as per ISO C99. If you want to have the exact
> > - * number of characters written into @buf as return value
> > - * (not including the trailing '\0'), use vscnprintf(). If the
> > - * return is greater than or equal to @size, the resulting
> > - * string is truncated.
> > - *
> > - * If you're not already dealing with a va_list consider using snprintf().
> > + * See the documentation over vsnprintf. This function does NOT add any NULL
> > + * termination to the buffer. The caller must do that if necessary.
> > */
> > -int vsnprintf(char *buf, size_t size, const char *fmt, va_list args)
> > +static int vsnprintf_noterm(char *buf, char *end, const char *fmt,
> > + va_list args)
> > {
> > unsigned long long num;
> > - char *str, *end;
> > + char *str;
> > struct printf_spec spec = {0};
> >
> > - /* Reject out-of-range values early. Large positive sizes are
> > - used for unknown buffer sizes. */
> > - if (WARN_ON_ONCE(size > INT_MAX))
> > - return 0;
> > -
> > str = buf;
> > - end = buf + size;
> > -
> > - /* Make sure end is always >= buf */
> > - if (end < buf) {
> > - end = ((void *)-1);
> > - size = end - buf;
> > - }
> > -
> > while (*fmt) {
> > const char *old_fmt = fmt;
> > int read = format_decode(fmt, &spec);
> > @@ -2462,18 +2433,69 @@ int vsnprintf(char *buf, size_t size, const char *fmt, va_list args)
> > str = number(str, end, num, spec);
> > }
> > }
> > -
> > out:
> > + /* the trailing null byte doesn't count towards the total */
> > + return str-buf;
> > +}
> > +EXPORT_SYMBOL(vsnprintf_noterm);
>
> export static function?
>
Good catch! I had originally had this function as an exported helper,
but made it static because there were no callers. Will fix.
> > +
> > +/**
> > + * vsnprintf - Format a string and place it in a buffer
> > + * @buf: The buffer to place the result into
> > + * @size: The size of the buffer, including the trailing null space
> > + * @fmt: The format string to use
> > + * @args: Arguments for the format string
> > + *
> > + * This function generally follows C99 vsnprintf, but has some
> > + * extensions and a few limitations:
> > + *
> > + * - ``%n`` is unsupported
> > + * - ``%p*`` is handled by pointer()
> > + *
> > + * See pointer() or Documentation/core-api/printk-formats.rst for more
> > + * extensive description.
> > + *
> > + * **Please update the documentation in both places when making changes**
> > + *
> > + * The return value is the number of characters which would
> > + * be generated for the given input, excluding the trailing
> > + * '\0', as per ISO C99. If you want to have the exact
> > + * number of characters written into @buf as return value
> > + * (not including the trailing '\0'), use vscnprintf(). If the
> > + * return is greater than or equal to @size, the resulting
> > + * string is truncated.
> > + *
> > + * If you're not already dealing with a va_list consider using snprintf().
> > + */
> > +int vsnprintf(char *buf, size_t size, const char *fmt, va_list args)
> > +{
> > + int ret;
> > + char *end;
> > +
> > + /* Reject out-of-range values early. Large positive sizes are
> > + used for unknown buffer sizes. */
> > + if (WARN_ON_ONCE(size > INT_MAX))
> > + return 0;
> > +
> > + end = buf + size;
> > +
> > + /* Make sure end is always >= buf */
> > + if (end < buf) {
> > + end = ((void *)-1);
> > + size = end - buf;
> > + }
> > +
> > + ret = vsnprintf_noterm(buf, end, fmt, args);
> > +
> > + /* NULL terminate the result */
> > if (size > 0) {
> > - if (str < end)
> > - *str = '\0';
> > + if (ret < size)
> > + buf[ret] = '\0';
> > else
> > - end[-1] = '\0';
> > + buf[size - 1] = '\0';
> > }
> >
> > - /* the trailing null byte doesn't count towards the total */
> > - return str-buf;
> > -
> > + return ret;
> > }
> > EXPORT_SYMBOL(vsnprintf);
> >
> > @@ -2506,6 +2528,41 @@ int vscnprintf(char *buf, size_t size, const char *fmt, va_list args)
> > }
> > EXPORT_SYMBOL(vscnprintf);
> >
> > +/**
> > + * snprintf_noterm - Format a string and place it in a buffer
> > + * @buf: The buffer to place the result into
> > + * @size: The size of the buffer, including the trailing null space
> > + * @fmt: The format string to use
> > + * @...: Arguments for the format string
> > + *
> > + * Same as snprintf, but don't NULL terminate the result.
> > + */
> > +int snprintf_noterm(char *buf, size_t size, const char *fmt, ...)
> > +{
> > + va_list args;
> > + int ret;
> > + char *end;
> > +
> > + /* Reject out-of-range values early. Large positive sizes are
> > + used for unknown buffer sizes. */
> > + if (WARN_ON_ONCE(size > INT_MAX))
> > + return 0;
> > +
> > + /* Make sure end is always >= buf */
> > + end = buf + size;
> > + if (end < buf) {
> > + end = ((void *)-1);
> > + size = end - buf;
> > + }
> > +
> > + va_start(args, fmt);
> > + ret = vsnprintf_noterm(buf, end, fmt, args);
> > + va_end(args);
> > +
> > + return ret;
> > +}
> > +EXPORT_SYMBOL(snprintf_noterm);
> > +
> > /**
> > * snprintf - Format a string and place it in a buffer
> > * @buf: The buffer to place the result into
> > --
> > 2.21.0
> >
--
Jeff Layton <jlayton@...nel.org>
Powered by blists - more mailing lists