linux-kernel - Re: [PATCH] dma-buf: add struct dma_buf_attach

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:   Fri, 3 May 2019 14:05:47 +0200
From:   Christian König <ckoenig.leichtzumerken@...il.com>
To:     Russell King - ARM Linux admin <linux@...linux.org.uk>
Cc:     xen-devel@...ts.xenproject.org, devel@...verdev.osuosl.org,
        linux-tegra@...r.kernel.org, intel-gfx@...ts.freedesktop.org,
        linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org,
        linux-media@...r.kernel.org, sstabellini@...nel.org,
        jgross@...e.com, boris.ostrovsky@...cle.com, digetx@...il.com,
        gregkh@...uxfoundation.org, arnd@...db.de, mchehab@...nel.org,
        kyungmin.park@...sung.com, m.szyprowski@...sung.com,
        pawel@...iak.com, jonathanh@...dia.com, thierry.reding@...il.com,
        tomi.valkeinen@...com, rodrigo.vivi@...el.com,
        joonas.lahtinen@...ux.intel.com, jani.nikula@...ux.intel.com,
        sean@...rly.run, maxime.ripard@...tlin.com,
        maarten.lankhorst@...ux.intel.com, sumit.semwal@...aro.org
Subject: Re: [PATCH] dma-buf: add struct dma_buf_attach_info v2

Am 30.04.19 um 19:31 schrieb Russell King - ARM Linux admin:
> On Tue, Apr 30, 2019 at 01:10:02PM +0200, Christian König wrote:
>> Add a structure for the parameters of dma_buf_attach, this makes it much easier
>> to add new parameters later on.
> I don't understand this reasoning.  What are the "new parameters" that
> are being proposed, and why do we need to put them into memory to pass
> them across this interface?
>
> If the intention is to make it easier to change the interface, passing
> parameters in this manner mean that it's easy for the interface to
> change and drivers not to notice the changes, since the compiler will
> not warn (unless some member of the structure that the driver is using
> gets removed, in which case it will error.)
>
> Additions to the structure will go unnoticed by drivers - what if the
> caller is expecting some different kind of behaviour, and the driver
> ignores that new addition?

Well, exactly that's the intention here: That the drivers using this 
interface should be able to ignore the new additions for now as long as 
they are not going to use them.

The background is that we have multiple interface changes in the 
pipeline, and each step requires new optional parameters.

> This doesn't seem to me like a good idea.

Well, the obvious alternatives are:

a) Change all drivers to explicitly provide NULL/0 for the new parameters.

b) Use a wrapper, so that the function signature of dma_buf_attach stays 
the same.

Key point here is that I have an invalidation callback change, a P2P 
patch set and some locking changes which all require adding new 
parameters or flags. And at each step I would then start to change all 
drivers, adding some more NULL pointers or flags with 0 default value.

I'm actually perfectly fine going down any route, but this just seemed 
to me simplest and with the least risk of breaking anything. Opinions?

Thanks,
Christian.