linux-kernel - Re: [PATCH for-5.3] drm/omap: ensure we have a valid dma

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: <20190809080750.GA21874@lst.de>
Date:   Fri, 9 Aug 2019 10:07:50 +0200
From:   Christoph Hellwig <hch@....de>
To:     Tomi Valkeinen <tomi.valkeinen@...com>
Cc:     Christoph Hellwig <hch@....de>, airlied@...ux.ie, daniel@...ll.ch,
        dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
        "H. Nikolaus Schaller" <hns@...delico.com>,
        Tony Lindgren <tony@...mide.com>,
        Peter Ujfalusi <peter.ujfalusi@...com>
Subject: Re: [PATCH for-5.3] drm/omap: ensure we have a valid dma_mask

On Fri, Aug 09, 2019 at 09:40:32AM +0300, Tomi Valkeinen wrote:
> We do call dma_set_coherent_mask() in omapdrm's probe() (in omap_drv.c), 
> but apparently that's not enough anymore. Changing that call to 
> dma_coerce_mask_and_coherent() removes the WARN. I can create a patch for 
> that, or Christoph can respin this one.

Oh, yes - that actually is the right thing to do here.  If you already
have it please just send it out.

>
> I am not too familiar with the dma mask handling, so maybe someone can 
> educate:
>
> dma_coerce_mask_and_coherent() overwrites dev->dma_mask. Isn't that a bad 
> thing? What if the platform has set dev->dma_mask, and the driver 
> overwrites it with its value? Or who is supposed to set dev->dma_mask?

->dma_mask is a complete mess.  It is a pointer when it really should
just be a u64, and that means every driver layer has to allocate space
for it.  We don't really do that for platform_devices, as that breaks
horribly assumptions in the usb code.  That is why
dma_coerce_mask_and_coherent exists as a nasty workaround that sets
the dma_mask to the coherent_dma_mask for devices that don't have
space for ->dma_mask allocated, which works as long as the device
doesn't have differnet addressing requirements for both.

I'm actually working to fix that mess up at the moment, but it is going
to take a few cycles until everything falls into place.