Re: [PATCH] platform/x86: think-lmi: Return EINVAL when kbdlang gets set to a 0 length string

[Andy Shevchenko <andy.shevchenko@gmail.com>]

On Mon, Jun 21, 2021 at 5:13 PM Hans de Goede <hdegoede@redhat.com> wrote:
> On 6/21/21 3:58 PM, Andy Shevchenko wrote:
> > On Mon, Jun 21, 2021 at 4:24 PM Hans de Goede <hdegoede@redhat.com> wrote:
> >>
> >> Commit 0ddcf3a6b442 ("platform/x86: think-lmi: Avoid potential read before
> >> start of the buffer") moved the lengt == 0 up to before stripping the '\n'
> >
> > length
>
> Ack, will fix.
>
> >> which typically gets added when users echo a value to a sysfs-attribute
> >> from the shell.
> >>
> >> This avoids a potential buffer-underrun, but it also causes a behavioral
> >> change, prior to this change "echo > kbdlang", iow writing just a single
> >> '\n' would result in an EINVAL error, but after the change this gets
> >> accepted setting kbdlang to an empty string.
> >
> > And why is it a problem?
>
> Because there are only a couple of valid string like "de", "fr", "es"
> and "us". We don't know the exact set of valid strings for a certain
> BIOS, but we do not for sure that an empty string is not valid.

Since we call strlen() on the buf it means we are expecting
NUL-terminated string no matter what.
In this case the

  p = skip_spaces(buf);
  length = strchrnul(buf, '\n') - p;
  if (!length || length >= ...)
    return ...

seems the best, no?

-- 
With Best Regards,
Andy Shevchenko