linux-kernel - Re: arch/superh: Suspicious coherent DMA allocations for CEU video buffers

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] [day] [month] [year] [list]

Message-ID: <20230703152336.43c5d129@meshulam.tesarici.cz>
Date:   Mon, 3 Jul 2023 15:23:36 +0200
From:   Petr Tesařík <petr@...arici.cz>
To:     Jacopo Mondi <jacopo.mondi@...asonboard.com>
Cc:     Yoshinori Sato <ysato@...rs.sourceforge.jp>,
        Rich Felker <dalias@...c.org>,
        John Paul Adrian Glaubitz <glaubitz@...sik.fu-berlin.de>,
        "open list:SUPERH" <linux-sh@...r.kernel.org>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: arch/superh: Suspicious coherent DMA allocations for CEU video
 buffers

Hi Jacopo,

On Mon, 3 Jul 2023 00:32:54 +0200
Jacopo Mondi <jacopo.mondi@...asonboard.com> wrote:

> Hi Petr,
> 
> On Thu, Jun 29, 2023 at 03:11:24PM +0200, Petr Tesařík wrote:
> > Hi Jacopo,
> >
> > I've just noticed a few calls to dma_declare_coherent_memory() which
> > look suspicious to me, all authored by you:
> >
> > - arch/sh/boards/mach-ap325rxa/setup.c
> > - arch/sh/boards/mach-ecovec24/setup.c
> > - arch/sh/boards/mach-kfr2r09/setup.c
> > - arch/sh/boards/mach-migor/setup.c
> > - arch/sh/boards/mach-se/7724/setup.c
> >
> > In these files, the last argument to dma_declare_coherent_memory()
> > looks like the last address to be used, but the expected value should
> > be the size of the reserved region, e.g.:
> >
> > 	dma_declare_coherent_memory(&ap325rxa_ceu_device.dev,
> > 			ceu_dma_membase, ceu_dma_membase,
> > 			ceu_dma_membase + CEU_BUFFER_MEMORY_SIZE - 1);
> >
> > Do you (or anyone else on Cc) agree that this is a braino, and all these
> > boards should actually use CEU_BUFFER_MEMORY_SIZE as the size of their
> > DMA pools rather than membase + CEU_BUFFER_MEMORY_SIZE - 1?
> >  
> 
> Thanks for spotting this.. I tried to track down where this comes from
> digging out the CEU and the platform file from 4.16, but it seems this
> simply is a brain fart from my side.
> 
> I presume this is very much dead code, as the commit message of
> 39fb993038e1 ("media: arch: sh: ap325rxa: Use new renesas-ceu camera
> driver") says: "Compile tested only." and nobody has ever reported
> bugs.
> 
> Feel free to send patches and cc me, as long as this code is around
> it's nice to have it correct at least.

Thank you for confirmation. I'll do that.

> Thank you!
>    j
> 
> PS: if you ask me, as nobody clearly run this code in the last 5
> years, I would simply drop all these platform files. But that's
> maintainers' call.

Since the beginning of the coherent region is declared correctly, it is
possible that it just happens to work, merely unreliably, frustrating
users with occasional freezes/crashes which may not be easily linked
with the CEU camera.

OTOH it is much more likely that this code is simply dead.

Petr T