linux-kernel - Re: [PATCH RESEND] media: omap3isp: Use struct

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: <YfJKPCXr4RjPL6lc@paasikivi.fi.intel.com>
Date:   Thu, 27 Jan 2022 09:31:08 +0200
From:   Sakari Ailus <sakari.ailus@...ux.intel.com>
To:     Mauro Carvalho Chehab <mchehab@...nel.org>
Cc:     Kees Cook <keescook@...omium.org>,
        Laurent Pinchart <laurent.pinchart@...asonboard.com>,
        Arnd Bergmann <arnd@...db.de>, linux-media@...r.kernel.org,
        stable@...r.kernel.org,
        "Gustavo A . R . Silva" <gustavoars@...nel.org>,
        linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org
Subject: Re: [PATCH RESEND] media: omap3isp: Use struct_group() for memcpy()
 region

Hi Mauro,

On Tue, Jan 25, 2022 at 09:24:26AM +0100, Mauro Carvalho Chehab wrote:
> Em Mon, 24 Jan 2022 09:29:52 -0800
> Kees Cook <keescook@...omium.org> escreveu:
> 
> > In preparation for FORTIFY_SOURCE performing compile-time and run-time
> > field bounds checking for memcpy(), memmove(), and memset(), avoid
> > intentionally writing across neighboring fields. Wrap the target region
> > in struct_group(). This additionally fixes a theoretical misalignment
> > of the copy (since the size of "buf" changes between 64-bit and 32-bit,
> > but this is likely never built for 64-bit).
> 
> 
> > FWIW, I think this code is totally broken on 64-bit (which appears to
> > not be a "real" build configuration): it would either always fail (with
> > an uninitialized data->buf_size) or would cause corruption in userspace
> > due to the copy_to_user() in the call path against an uninitialized
> > data->buf value:
> 
> It doesn't matter. This driver is specific for TI OMAP3 SoC, which
> is Cortex-A8 (32-bits). It only builds on 64 bit due to COMPILE_TEST.

I agree that "it doesn't matter" in any real configuration. But if it's
this easy to address omap3isp driver behaving nicely with compile test,
then this is definitely worth merging.

I'll pick the patch to my tree.

-- 
Kind regards,

Sakari Ailus