Re: [PATCH 4/4] PCI/sysfs: Pass the device name length in UTF-16 code units

["Krzysztof Wilczyński" <kwilczynski@kernel.org>]

Hello Rafael,

> Currently, dsm_label_utf16s_to_utf8s() passes the ACPI buffer
> length to utf16s_to_utf8s() unchanged.  The Device Name _DSM may
> return the device name as a buffer holding a UTF-16 string, and
> the ACPI length counts bytes while the converter counts wchar_t
> elements.  The converter therefore receives a count twice the
> number of code units the buffer holds, and that count is its only
> bound on the input.
> 
> A NUL code unit ends the conversion early, so a name that carries
> one is converted correctly and the error stays hidden.  ACPICA
> zeroes the entire result allocation, and the padding placed after
> the buffer supplies that NUL for most lengths.  Lengths that leave
> no padding, or a single byte of it, do not, and the conversion
> then runs up to its own length in bytes past the end of the
> allocation.  The bytes beyond the allocation are decoded into the
> "label" attribute, which is world readable.
> 
> Thus, divide the buffer length by the size of wchar_t so that the
> converter receives a count of code units.  The division truncates,
> so the converter is never told to read more bytes than the buffer
> holds, including for an odd length.
> 
> Reaching the out of bounds read needs firmware that returns the
> name as a buffer and omits the terminator.

[...]
>  	len = utf16s_to_utf8s((const wchar_t *)obj->buffer.pointer,
> -			      obj->buffer.length,
> +			      obj->buffer.length / sizeof(wchar_t),
>  			      UTF16_LITTLE_ENDIAN,
>  			      buf, PAGE_SIZE - 1);


While looking at this from PCI perspective, I also noticed something with
ACPI code base that we could also fix.

File drivers/acpi/device_sysfs.c, where the description_show() is adding
/sys/bus/acpi/devices/.../description sysfs attribute to each device where
this is supported/available using data from the ACPI _STR method, has the
following:

  result = utf16s_to_utf8s(
          (wchar_t *)str_obj->buffer.pointer,
          str_obj->buffer.length,
          UTF16_LITTLE_ENDIAN, buf,
          PAGE_SIZE - 1);

I believe, the above code has potentially the same issue as the one PCI
has, and which we are fixing here.  I can send a small patch to fix the
this, too, if you want.

Thank you!

        Krzysztof