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: <b9140772-0370-a858-578c-af503a06d8e9@deltatee.com>
Date:   Fri, 29 May 2020 15:21:04 -0600
From:   Logan Gunthorpe <logang@...tatee.com>
To:     Marek Szyprowski <m.szyprowski@...sung.com>,
        Christoph Hellwig <hch@...radead.org>
Cc:     Tom Murphy <murphyt7@....ie>, iommu@...ts.linux-foundation.org,
        kvm@...r.kernel.org, David Airlie <airlied@...ux.ie>,
        dri-devel@...ts.freedesktop.org,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Matthias Brugger <matthias.bgg@...il.com>,
        Julien Grall <julien.grall@....com>,
        Thierry Reding <thierry.reding@...il.com>,
        Will Deacon <will@...nel.org>,
        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>,
        Gerald Schaefer <gerald.schaefer@...ibm.com>,
        linux-s390@...r.kernel.org, linux-arm-msm@...r.kernel.org,
        intel-gfx@...ts.freedesktop.org,
        Alex Williamson <alex.williamson@...hat.com>,
        linux-mediatek@...ts.infradead.org,
        Rodrigo Vivi <rodrigo.vivi@...el.com>,
        linux-tegra@...r.kernel.org, Thomas Gleixner <tglx@...utronix.de>,
        virtualization@...ts.linux-foundation.org,
        linux-arm-kernel@...ts.infradead.org,
        Robin Murphy <robin.murphy@....com>,
        Cornelia Huck <cohuck@...hat.com>,
        linux-kernel@...r.kernel.org, Kukjin Kim <kgene@...nel.org>,
        David Woodhouse <dwmw2@...radead.org>,
        Jani Nikula <jani.nikula@...ux.intel.com>,
        Joonas Lahtinen <joonas.lahtinen@...ux.intel.com>
Subject: Re: [PATCH 0/8] Convert the intel iommu driver to the dma-iommu api



On 2020-05-29 3:11 p.m., Marek Szyprowski wrote:
> Patches are pending:
> https://lore.kernel.org/linux-iommu/20200513132114.6046-1-m.szyprowski@samsung.com/T/

Cool, nice! Though, I still don't think that fixes the issue in
i915_scatterlist.h given it still ignores sg_dma_len() and strictly
relies on sg_next()/sg_is_last() to stop iterating -- and I suspect this
is the bug that got in Tom's way.

>> However, as Robin pointed out, there are other ugly tricks like stopping
>> iterating through the SGL when sg_dma_len() is zero. For example, the
>> AMD driver appears to use drm_prime_sg_to_page_addr_arrays() which does
>> this trick and thus likely isn't buggy (otherwise, I'd expect someone to
>> have complained by now seeing AMD has already switched to IOMMU-DMA.
> 
> I'm not sure that this is a trick. Stopping at zero sg_dma_len() was 
> somewhere documented.

Well whatever you want to call it, it is ugly to have some drivers doing
one thing with the returned value and others assuming there's an extra
zero at the end. It just causes confusion for people reading/copying the
code. It would be better if they are all consistent. However, I concede
stopping at zero should not be broken, presently.

Logan