Re: [PATCH] iommu/amd: fix over-read of ACPI UID from IVRS table

[Joerg Roedel]

On Mon, May 11, 2020 at 10:23:52AM +0000, Alexander Monakov wrote:
> IVRS parsing code always tries to read 255 bytes from memory when
> retrieving ACPI device path, and makes an assumption that firmware
> provides a zero-terminated string. Both of those are bugs: the entry
> is likely to be shorter than 255 bytes, and zero-termination is not
> guaranteed.
> 
> With Acer SF314-42 firmware these issues manifest visibly in dmesg:
> 
> AMD-Vi: ivrs, add hid:AMDI0020, uid:\_SB.FUR0\xf0\xa5, rdevid:160
> AMD-Vi: ivrs, add hid:AMDI0020, uid:\_SB.FUR1\xf0\xa5, rdevid:160
> AMD-Vi: ivrs, add hid:AMDI0020, uid:\_SB.FUR2\xf0\xa5, rdevid:160
> AMD-Vi: ivrs, add hid:AMDI0020, uid:\_SB.FUR3>\x83e\x8d\x9a\xd1...
> 
> The first three lines show how the code over-reads adjacent table
> entries into the UID, and in the last line it even reads garbage data
> beyond the end of the IVRS table itself.
> 
> Since each entry has the length of the UID (uidl member of ivhd_entry
> struct), use that for memcpy, and manually add a zero terminator.
> 
> Avoid zero-filling hid and uid arrays up front, and instead ensure
> the uid array is always zero-terminated. No change needed for the hid
> array, as it was already properly zero-terminated.
> 
> Fixes: 2a0cb4e2d423c ("iommu/amd: Add new map for storing IVHD dev entry type HID")
> 
> Signed-off-by: Alexander Monakov <amonakov@xxxxxxxxx>
> Cc: Joerg Roedel <joro@xxxxxxxxxx>
> Cc: iommu@xxxxxxxxxxxxxxxxxxxxxxxxxx
> ---
>  drivers/iommu/amd_iommu_init.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)

Applied for v5.7, thanks.