Re: [PATCH 06/14] lib/vsprintf.c: warn about too large precisions and field widths

[Andy Shevchenko]

On Mon, Nov 23, 2015 at 11:29 PM, Rasmus Villemoes
<linux@xxxxxxxxxxxxxxxxxx> wrote:
> The field width is overloaded to pass some extra information for
> some %p extensions (e.g. #bits for %pb). But we might silently
> truncate the passed value when we stash it in struct printf_spec (see
> e.g. "lib/vsprintf.c: expand field_width to 24 bits").  Hopefully 23
> value bits should now be enough for everybody, but if not, let's make
> some noise.
>
> Do the same for the precision. In both cases, clamping seems more
> sensible than truncating. While, according to POSIX, "A negative
> precision is taken as if the precision were omitted.", the kernel's
> printf has always treated that case as if the precision was 0, so we
> use that as lower bound. For the field width, the smallest
> representable value is actually -(1<<23), but a negative field width
> means 'set the LEFT flag and use the absolute value', so we want the
> absolute value to fit.
>

Do we need to do the same for bstr_printf() ?

> Signed-off-by: Rasmus Villemoes <linux@xxxxxxxxxxxxxxxxxx>
> ---
>  lib/vsprintf.c | 24 ++++++++++++++++++++++--
>  1 file changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index d7e27c54fa00..8af5535fd738 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -386,6 +386,8 @@ struct printf_spec {
>         unsigned int    base:8;         /* number base, 8, 10 or 16 only */
>         signed int      precision:16;   /* # of digits/chars */
>  } __packed;
> +#define FIELD_WIDTH_MAX ((1 << 23) - 1)
> +#define PRECISION_MAX ((1 << 15) - 1)
>  extern char __check_printf_spec[1-2*(sizeof(struct printf_spec) != 8)];
>
>  static noinline_for_stack
> @@ -1815,6 +1817,24 @@ qualifier:
>         return ++fmt - start;
>  }
>
> +static inline void
> +set_field_width(struct printf_spec *spec, int width)
> +{
> +       spec->field_width = width;
> +       if (WARN_ONCE(spec->field_width != width, "field width %d too large", width)) {
> +               spec->field_width = clamp(width, -FIELD_WIDTH_MAX, FIELD_WIDTH_MAX);
> +       }
> +}
> +
> +static inline void
> +set_precision(struct printf_spec *spec, int prec)
> +{
> +       spec->precision = prec;
> +       if (WARN_ONCE(spec->precision != prec, "precision %d too large", prec)) {
> +               spec->precision = clamp(prec, 0, PRECISION_MAX);
> +       }
> +}
> +
>  /**
>   * vsnprintf - Format a string and place it in a buffer
>   * @buf: The buffer to place the result into
> @@ -1882,11 +1902,11 @@ int vsnprintf(char *buf, size_t size, const char *fmt, va_list args)
>                 }
>
>                 case FORMAT_TYPE_WIDTH:
> -                       spec.field_width = va_arg(args, int);
> +                       set_field_width(&spec, va_arg(args, int));
>                         break;
>
>                 case FORMAT_TYPE_PRECISION:
> -                       spec.precision = va_arg(args, int);
> +                       set_precision(&spec, va_arg(args, int));
>                         break;
>
>                 case FORMAT_TYPE_CHAR: {
> --
> 2.6.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/



-- 
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/