Re: [PATCH 11/20] amifb: get rid of pointless access_ok() calls

From: Geert Uytterhoeven
Date: Thu May 14 2020 - 16:22:01 EST


Hi Al,

On Thu, May 14, 2020 at 7:43 PM Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:
> 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@xxxxxxxxxxx>
> >
> > 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?

In theory, up to 3.5 GiB ;-)
In practice, 16 MiB is already a lot (mine has 12).

> 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...

These #ifdefs are relics from APUS (Amiga Power-Up System), which
added a PPC board. APUS support was killed off a long time ago,
when arch/ppc/ was still king, but these #ifdefs were missed, because
they didn't test for CONFIG_APUS.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds