linux-kernel - Re: PCI: dwc: Warn only for non-prefetchable memory resource size >4GB

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 19 May 2020 19:33:04 -0700
From:   Alan Mikhak <alanmikhak@...il.com>
To:     gustavo.pimentel@...opsys.com
Cc:     alan.mikhak@...ive.com, amurray@...goodpenguin.co.uk,
        bhelgaas@...gle.com, helgaas@...nel.org, jingoohan1@...il.com,
        jonathanh@...dia.com, kthota@...dia.com,
        linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org,
        lorenzo.pieralisi@....com, mmaddireddy@...dia.com,
        sagar.tv@...il.com, thierry.reding@...il.com, vidyas@...dia.com,
        Alan Mikhak <amikhak@...elessfabric.com>
Subject: Re: PCI: dwc: Warn only for non-prefetchable memory resource size >4GB

Hi Lorenzo,

I came across this issue when implementing a Linux NVMe endpoint function
driver under the Linux PCI Endpoint Framework:
https://lwn.net/Articles/804369/

I needed to map up to 128GB of host memory using a single ATU window
from the endpoint side because NVMe PRPs can be scattered all over host
memory. In the process, I came across this 4GB limitation where the
maximum size of memory that can be mapped is limited by what a u32 value
can represent.

I submitted a separate patch to fix an undefined behavior that may also
happen in dw_pcie_prog_outbound_atu_unroll() under some circumstances
when the size of the memory being mapped is greater than what a u32 value
can represent.
https://patchwork.kernel.org/patch/11469701/

The above patch has been accepted. However, the variable pp->mem_size
in dw_pcie_host_init() is still a u32 whereas the value returned by
resource_size() is u64. If the resource size has non-zero upper 32-bits,
those upper 32-bits will be lost when assigning:
 pp->mem_size = resource_size(pp->mem).

Since current callers seem happy with the existing 4GB implementation
and fixing the u32 limit is beyond my available resources and has a long
high-impact tail, a warning seemed to be a good choice to highlight
this issue in case someone else decides to map a MEM region that is
greater than 4GB.

Removing the warning will avoid such discussions. Without this warning,
this limitation will go unnoticed and will only impact whoever has to
deal with it. It cost me time to figure it out when I had an application
that needed a region larger than 4GB. I figured the most I could do about
it is to raise the issue by adding a warning.

Regards,
Alan