linux-kernel - Re: [PATCH 0/8] Convert the intel iommu driver to the dma-iommu api

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]

Message-ID: <465815ae-9292-f37a-59b9-03949cb68460@deltatee.com>
Date:   Thu, 28 May 2020 18:00:44 -0600
From:   Logan Gunthorpe <logang@...tatee.com>
To:     Tom Murphy <murphyt7@....ie>, iommu@...ts.linux-foundation.org
Cc:     kvm@...r.kernel.org, David Airlie <airlied@...ux.ie>,
        dri-devel@...ts.freedesktop.org,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        linux-tegra@...r.kernel.org, Julien Grall <julien.grall@....com>,
        Thierry Reding <thierry.reding@...il.com>,
        Will Deacon <will@...nel.org>,
        Marek Szyprowski <m.szyprowski@...sung.com>,
        Jean-Philippe Brucker <jean-philippe@...aro.org>,
        linux-samsung-soc@...r.kernel.org, Marc Zyngier <maz@...nel.org>,
        Krzysztof Kozlowski <krzk@...nel.org>,
        Jonathan Hunter <jonathanh@...dia.com>,
        linux-rockchip@...ts.infradead.org, Andy Gross <agross@...nel.org>,
        linux-arm-kernel@...ts.infradead.org, linux-s390@...r.kernel.org,
        linux-arm-msm@...r.kernel.org, intel-gfx@...ts.freedesktop.org,
        Eric Auger <eric.auger@...hat.com>,
        Alex Williamson <alex.williamson@...hat.com>,
        linux-mediatek@...ts.infradead.org,
        Rodrigo Vivi <rodrigo.vivi@...el.com>,
        Matthias Brugger <matthias.bgg@...il.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        virtualization@...ts.linux-foundation.org,
        Gerald Schaefer <gerald.schaefer@...ibm.com>,
        David Woodhouse <dwmw2@...radead.org>,
        Cornelia Huck <cohuck@...hat.com>,
        linux-kernel@...r.kernel.org, Kukjin Kim <kgene@...nel.org>,
        Robin Murphy <robin.murphy@....com>,
        Lu Baolu <baolu.lu@...ux.intel.com>
Subject: Re: [PATCH 0/8] Convert the intel iommu driver to the dma-iommu api

Hi Tom,

On 2019-12-21 8:03 a.m., Tom Murphy wrote:
> This patchset converts the intel iommu driver to the dma-iommu api.

Just wanted to note that I've rebased your series on recent kernels and
have done some testing on my old Sandybridge machine (without the DO NOT
MERGE patch) and have found no issues. I hope this can make progress
soon and get merged soon. If you like you can add:

Tested-By: Logan Gunthorpe <logang@...tatee.com>

> While converting the driver I exposed a bug in the intel i915 driver which causes a huge amount of artifacts on the screen of my laptop. You can see a picture of it here:
> https://github.com/pippy360/kernelPatches/blob/master/IMG_20191219_225922.jpg
> 
> This issue is most likely in the i915 driver and is most likely caused by the driver not respecting the return value of the dma_map_ops::map_sg function. You can see the driver ignoring the return value here:
> https://github.com/torvalds/linux/blob/7e0165b2f1a912a06e381e91f0f4e495f4ac3736/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c#L51
> 
> Previously this didn’t cause issues because the intel map_sg always returned the same number of elements as the input scatter gather list but with the change to this dma-iommu api this is no longer the case. I wasn’t able to track the bug down to a specific line of code unfortunately.  

I did some digging into this myself and while I don't have full patch, I
think I traced it closer to the problem.

Sadly, ignoring the number of nents returned by map_sg() is endemic to
dma-buf users, but AMD's GPU driver seems to do the same thing,
presumably without issues.

Digging a bit further, I found that the i915 has an "innovative" way of
iterating through SGLs, see [1]. I suspect if __sgt_iter is changed to
increment with sg_dma_len() and return NULL when there is no length
left, it may fix the issue.

But, sorry, I don't really have the means or time to fix and test this
myself.

Thanks,

Logan

[1]
https://elixir.bootlin.com/linux/v5.7-rc7/source/drivers/gpu/drm/i915/i915_scatterlist.h#L76