Bug 207037 - ixv driver uses uninitialized offset variable and writes into arbitrary pci config register
Summary: ixv driver uses uninitialized offset variable and writes into arbitrary pci c...
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Only Me
Assignee: Kevin Bowling
URL:
Keywords: IntelNetworking, patch
Depends on:
Blocks:
 
Reported: 2016-02-08 23:13 UTC by Jeremiah
Modified: 2024-10-30 01:04 UTC (History)
3 users (show)

See Also:


Attachments
Patch to illustrate the problem (805 bytes, patch)
2016-02-08 23:13 UTC, Jeremiah
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jeremiah 2016-02-08 23:13:03 UTC
Created attachment 166768 [details]
Patch to illustrate the problem

In the QEMU workaround code in if_ixv.c, the ixv driver calls pci_find_cap(dev, PCIY_MSIX, &rid). It is not checking the return code from that function and the function appears to always be failing. This then causes the driver to use the rid variable uninitialized, which will mean setting a bit at an arbitrary offset in pci config space. For now, this seems to have no adverse impact, but it could easily cause very subtle problems. Also the QEMU workaround is probably non-functional because of this.

I've attached a patch for a partial solution that checks the error code and skips PCI write if it fails. This avoid the erroneous PCI accesses, but it would be better if we could figure out why finding the capability is failing (I have not debugged it that far).
Comment 1 Piotr Kubaj freebsd_committer freebsd_triage 2023-02-03 15:43:02 UTC
Seems to be still relevant.
Comment 2 commit-hook freebsd_committer freebsd_triage 2024-10-27 07:22:51 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=b87b3696c973ef0a9df70143cd89f6b488531e93

commit b87b3696c973ef0a9df70143cd89f6b488531e93
Author:     Jeremiah Lott <jlott@averesystems.com>
AuthorDate: 2024-10-27 07:18:54 +0000
Commit:     Kevin Bowling <kbowling@FreeBSD.org>
CommitDate: 2024-10-27 07:18:54 +0000

    ixv: Check cap return before MSI-X enable write

    In the QEMU workaround code in if_ixv.c, the ixv driver calls
    pci_find_cap(dev, PCIY_MSIX, &rid). It is not checking the return code
    from that function and the function appears to always be failing. This
    then causes the driver to use the rid variable uninitialized, which
    will mean setting a bit at an arbitrary offset in pci config space. For
    now, this seems to have no adverse impact, but it could easily cause
    very subtle problems.

    PR:             207037
    MFC after:      3 days
    Sponsored by:   BBOX.io

 sys/dev/ixgbe/if_ixv.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)
Comment 3 commit-hook freebsd_committer freebsd_triage 2024-10-30 01:02:47 UTC
A commit in branch stable/14 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=86efff54916bba7b7543699fe5922d8363f16c11

commit 86efff54916bba7b7543699fe5922d8363f16c11
Author:     Jeremiah Lott <jlott@averesystems.com>
AuthorDate: 2024-10-27 07:18:54 +0000
Commit:     Kevin Bowling <kbowling@FreeBSD.org>
CommitDate: 2024-10-30 01:01:04 +0000

    ixv: Check cap return before MSI-X enable write

    In the QEMU workaround code in if_ixv.c, the ixv driver calls
    pci_find_cap(dev, PCIY_MSIX, &rid). It is not checking the return code
    from that function and the function appears to always be failing. This
    then causes the driver to use the rid variable uninitialized, which
    will mean setting a bit at an arbitrary offset in pci config space. For
    now, this seems to have no adverse impact, but it could easily cause
    very subtle problems.

    PR:             207037
    Sponsored by:   BBOX.io

    (cherry picked from commit b87b3696c973ef0a9df70143cd89f6b488531e93)

 sys/dev/ixgbe/if_ixv.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)
Comment 4 commit-hook freebsd_committer freebsd_triage 2024-10-30 01:03:56 UTC
A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=453f85caa93a431911b66b2cd781595e4ebd59ec

commit 453f85caa93a431911b66b2cd781595e4ebd59ec
Author:     Jeremiah Lott <jlott@averesystems.com>
AuthorDate: 2024-10-27 07:18:54 +0000
Commit:     Kevin Bowling <kbowling@FreeBSD.org>
CommitDate: 2024-10-30 01:02:46 +0000

    ixv: Check cap return before MSI-X enable write

    In the QEMU workaround code in if_ixv.c, the ixv driver calls
    pci_find_cap(dev, PCIY_MSIX, &rid). It is not checking the return code
    from that function and the function appears to always be failing. This
    then causes the driver to use the rid variable uninitialized, which
    will mean setting a bit at an arbitrary offset in pci config space. For
    now, this seems to have no adverse impact, but it could easily cause
    very subtle problems.

    PR:             207037
    Sponsored by:   BBOX.io

    (cherry picked from commit b87b3696c973ef0a9df70143cd89f6b488531e93)

 sys/dev/ixgbe/if_ixv.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)