lkml.org 
[lkml]   [2020]   [May]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.6 014/126] iommu/amd: Fix over-read of ACPI UID from IVRS table
    Date
    From: Alexander Monakov <amonakov@ispras.ru>

    [ Upstream commit e461b8c991b9202b007ea2059d953e264240b0c9 ]

    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@ispras.ru>
    Cc: Joerg Roedel <joro@8bytes.org>
    Cc: iommu@lists.linux-foundation.org
    Link: https://lore.kernel.org/r/20200511102352.1831-1-amonakov@ispras.ru
    Signed-off-by: Joerg Roedel <jroedel@suse.de>
    Signed-off-by: Sasha Levin <sashal@kernel.org>
    ---
    drivers/iommu/amd_iommu_init.c | 9 +++++----
    1 file changed, 5 insertions(+), 4 deletions(-)

    diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
    index 2b9a67ecc6ac..5b81fd16f5fa 100644
    --- a/drivers/iommu/amd_iommu_init.c
    +++ b/drivers/iommu/amd_iommu_init.c
    @@ -1329,8 +1329,8 @@ static int __init init_iommu_from_acpi(struct amd_iommu *iommu,
    }
    case IVHD_DEV_ACPI_HID: {
    u16 devid;
    - u8 hid[ACPIHID_HID_LEN] = {0};
    - u8 uid[ACPIHID_UID_LEN] = {0};
    + u8 hid[ACPIHID_HID_LEN];
    + u8 uid[ACPIHID_UID_LEN];
    int ret;

    if (h->type != 0x40) {
    @@ -1347,6 +1347,7 @@ static int __init init_iommu_from_acpi(struct amd_iommu *iommu,
    break;
    }

    + uid[0] = '\0';
    switch (e->uidf) {
    case UID_NOT_PRESENT:

    @@ -1361,8 +1362,8 @@ static int __init init_iommu_from_acpi(struct amd_iommu *iommu,
    break;
    case UID_IS_CHARACTER:

    - memcpy(uid, (u8 *)(&e->uid), ACPIHID_UID_LEN - 1);
    - uid[ACPIHID_UID_LEN - 1] = '\0';
    + memcpy(uid, &e->uid, e->uidl);
    + uid[e->uidl] = '\0';

    break;
    default:
    --
    2.25.1


    \
     
     \ /
      Last update: 2020-05-26 21:22    [W:6.399 / U:0.024 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site