Re: [RESEND 2][PATCH v4] hexdump: fix for non-aligned buffers

[Andrew Morton <akpm@linux-foundation.org>]

On Thu, 9 Jul 2015 03:36:02 +0200 "H. Mijail" <hmijail@gmail.com> wrote:

> 
> > On 09 Jul 2015, at 02:03, Andrew Morton <akpm@linux-foundation.org> wrote:
> > 
> > On Thu, 9 Jul 2015 01:44:18 +0200 Horacio Mijail Ant__n Quiles <hmijail@gmail.com> wrote:
> > 
> >> An hexdump with a buf not aligned to the groupsize causes
> >> non-naturally-aligned memory accesses. This was causing a kernel panic on
> >> the processor BlackFin BF527, when such an unaligned buffer was fed by the
> >> function ubifs_scanned_corruption in fs/ubifs/scan.c .
> >> 
> >> To fix this, if the buffer is not aligned to groupsize in a platform which
> >> does not define CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS, then change the
> >> groupsize to 1, so alignment is no longer a problem.
> >> This behavior is coherent with the way the function currently deals with
> >> inappropriate parameter combinations, which is to fall back to safe
> >> "defaults", even if that means changing the output format and the implicit
> >> access patterns that could have been expected.
> > 
> > CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS seems inappropriate for this. 
> > Having this unset means "can do unaligned accesses, but they're
> > inefficient".  It doesn't mean "unaligned accesses go oops".
> > 
> > But I can't the appropriate config knob.  There's a
> > CONFIG_CPU_HAS_NO_UNALIGNED, but that's an m68k-private thing.
> 
> I'm only a newbie, but I will argue that the lesser evil is checking
> CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS - until a new configure variable
> is defined.
> 
> In Documentation/unaligned-memory-access.txt, an undefined
> CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS is taken as if to mean 'the 
> hardware isn't able to access memory on arbitrary boundaries'.

hm, yes, OK, CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS is a poor name.

> The other alternative in Documentation/unaligned-memory-access.txt is the
> macro get_unaligned() from asm/unaligned.h. However, using get_unaligned()
> would mean a much more intrusive patch, since each case of the groupsize 
> would be changed, and anyway we would still need to check 
> CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS to avoid penalising everyone.

Actually, I think using get_unaligned() would be a better solution. 
For architectures which have CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS=y,
get_unaligned() should be fast - just one instruction.

This way we avoid having different-appearing output on different
architectures.  

> >> Resent on 8 Jul because of no answers.
> >> 
> >> Resent on 29 Jun because of no answers.
> > 
> > During the merge window.  So I have everything sitting there in my
> > patches-to-process pile.  The backlog is excessive this time (700+
> > emails) so I'm thinking I'll change things so I'll henceforth be
> > processing patches-for-the-next-cycle during this-cycle, while keeping
> > patches-for-next-cycle out of linux-next.
> 
> No problem for me - if I should squelch the next version of this patch
> for some time, please let me know.

The merge window has ended ;)

> > 
> > But as mentioned above, that's different from "architectures which do
> > not support them efficently"!  Maybe we need a new config variable.
> > 
> > Or maybe blackfin should be handling the unaligned access trap and
> > transparently handling it, like sparc?
> > 
> 
> I'll wait for anyone else to weight in'

Possibly blackfin *could* emulate unaligned accesses.  But according to
the documentation, hex_dump_to_buffer() needs to be altered anyway
because we cannot rely on the architecture handling such accesses.