Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org E0EC8C433CA
From:   Kalle Valo <kvalo@codeaurora.org>
To:     Christian Lamparter <chunkeey@gmail.com>
Cc:     Jia-Ju Bai <baijiaju@tsinghua.edu.cn>, kuba@kernel.org,
        linux-wireless@vger.kernel.org
Subject: Re: [PATCH] p54: avoid accessing the data mapped to streaming DMA
References: <20200802132949.26788-1-baijiaju@tsinghua.edu.cn>
        <0b432282-8d52-3004-6648-d97c03cdae28@gmail.com>
Date:   Wed, 26 Aug 2020 19:02:43 +0300
In-Reply-To: <0b432282-8d52-3004-6648-d97c03cdae28@gmail.com> (Christian
        Lamparter's message of "Tue, 18 Aug 2020 19:27:10 +0200")
Message-ID: <873649cs4c.fsf@codeaurora.org>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

Christian Lamparter <chunkeey@gmail.com> writes:

> On 2020-08-02 15:29, Jia-Ju Bai wrote:
>> In p54p_tx(), skb->data is mapped to streaming DMA on line 337:
>>    mapping = pci_map_single(..., skb->data, ...);
>>
>> Then skb->data is accessed on line 349:
>>    desc->device_addr = ((struct p54_hdr *)skb->data)->req_id;
>>
>> This access may cause data inconsistency between CPU cache and hardware.
>>
>> To fix this problem, ((struct p54_hdr *)skb->data)->req_id is stored in
>> a local variable before DMA mapping, and then the driver accesses this
>> local variable instead of skb->data.
>
> Interesting. Please bear with me here. From my understanding, the
> streaming direction is set to PCI_DMA_TODEVICE. So is it really
> possible for the hardware to interfere with the data without the IOMMU
> catching this?

Also is there any documentation about this scenario? I would like to
understand this better.

> (That said, patch looks be fine. I'll need to dust off a old PCI PC to
> check this with real hardware, if requested.)

No need to test with real hardware for my sake, but a careful review is
very much appreciated.

-- 
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches