linux-kernel - Re: [PATCH] i2c: ismt: fix wrong device address when unmap the data buffer

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:   Mon, 12 Jun 2017 12:38:13 +0300
From:   Andy Shevchenko <andy.shevchenko@...il.com>
To:     Peter Rosin <peda@...ntia.se>
Cc:     Song liwei <liwei.song@...driver.com>,
        Wolfram Sang <wsa@...-dreams.de>,
        Seth Heasley <seth.heasley@...el.com>,
        Neil Horman <nhorman@...driver.com>,
        linux-i2c <linux-i2c@...r.kernel.org>,
        linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] i2c: ismt: fix wrong device address when unmap the data buffer

On Mon, Jun 12, 2017 at 12:28 PM, Peter Rosin <peda@...ntia.se> wrote:
> On 2017-06-12 11:11, Andy Shevchenko wrote:
>> On Mon, Jun 12, 2017 at 6:42 AM, Song liwei <liwei.song@...driver.com> wrote:
>>> From: Liwei Song <liwei.song@...driver.com>

>>> After finished I2C block read/write, when unmap the data buffer,
>>> a wrong device address was pass to dma_unmap_single(),

>>> the right
>>> device address should be "dev" not "&adap->dev", the relation is
>>> *(&adap->dev) == dev.
>>
>> This is confusing. You are telling that there are two copies of struct
>> device here?
>
> Yes, there are two copies.

No, there is not. See below.

> There's the local dev variable that is like
> this:
>         struct device *dev = &priv->pci_dev->dev;
>
> And then there's the adapter device in adap->dev (inlined in adap, so
> the explanation that the relations is "*(&adap->dev) == dev" is doubly
> wrong).

I got the idea, but your explanation is not a penny better.

There are two struct devices, one is a real PCI device, which
represents actual device what *does* DMA.
This struct should be used according to DMA API.

Another struct device which is wrongly used is an artificial one that
represents I2C adapter in terms of Linux kernel.

The relation ship (if designed correctly) *should be* adap->dev.parent
== &pci_dev->dev.

> The bug is that the first argument to dma_unmap_single is not the
> same as the first argument to dma_map_single that appears a few lines
> up.

I understand that.

> I cannot tell if that argument should be "dev" or "&adap->dev"
> though, but the two calls must refer to the same struct device *.

See above. It's a simple choice.

-- 
With Best Regards,
Andy Shevchenko