linux-kernel - Re: [PATCH 11/20] amifb: get rid of pointless access

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 14 May 2020 18:41:31 +0100
From:   Al Viro <viro@...iv.linux.org.uk>
To:     Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>
Cc:     linux-kernel@...r.kernel.org,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH 11/20] amifb: get rid of pointless access_ok() calls

On Thu, May 14, 2020 at 04:25:35PM +0200, Bartlomiej Zolnierkiewicz wrote:
> Thank you for in-detail explanations, for this patch:
> 
> Acked-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>
> 
> Could you also please take care of adding missing checks for {get,put}_user()
> failures later?

Umm...  OK; put_user() side is trivial -  the interesting part is what to do
about get_user() failures halfway through.  Right now it treats them as
"we'd read zeroes".  On anything else I would say "screw it, memdup_user()
the damn thing on the way in and copy from there", but... Amiga has how
much RAM, again?

OTOH, from my reading of that code it does appear to be limited to
4Kb of data to copy, so it's probably OK...  Hell knows - I'm really
confused by those #ifdef __mc68000__ in there; the driver *is*
amiga-only:
obj-$(CONFIG_FB_AMIGA)            += amifb.o c2p_planar.o
config FB_AMIGA
        tristate "Amiga native chipset support"
        depends on FB && AMIGA
and AMIGA is defined only in arch/m68k/Kconfig.machine.  So how the
hell can it *not* be true?  OTOH, it looks like hand-optimized
asm equivalents of C they have in #else, so that #else might be
meant to document what's going on...

I've no idea how to test any changes to that thing - the only
m68k emulator I'm reasonably familiar with is aranym, and
that's Atari, not Amiga.  Never got around to setting up UAE...
So I can do a patch more or less blindly (memdup_user() after
it has checked the limits on height/width, then dereferencing
from copy instead of get_user()), but I won't be able to test
it.