Re: [linuxsh-dev] Re: bad alignment fixup

[Philipp R. <pr...@pa...>]

On Mon, Nov 20, 2000 at 02:11:26PM +0000, David Woodhouse wrote:
> 
> prumpf said:
> > I think the patch leaves a security hole: if you put_user to an unaligned
> > invalid address, the kernel will oops.
> 
> Yep. I've seen that happen.

that's still a bug _somewhere_ though.  either we're passing an invalid pointer
to a syscall or we're using {get,put}_user when we shouldn't.

> > The fix is something like (optimized for code size not speed)
> >
> > static inline void unaligned_memcpy(void *dst, void *src, int count,
> >                                     struct pt_regs *regs)
> > {
> >     int failed;
> >     while(count--) {
> >         asm volatile("
> >                 1:      mov.b   @(%2),%0
> >                         bra     4f
> >                 2:      mov.b   %0,@(%3)
> >                 3:      mov     #1,%1
> >                 4:
> >                         .section        __ex_table,"a"
> >                         .long   1b,2b
> >                 .previous" : "=r" (tmp), "=r" (failed) : 
> >		"r" (dst++), "r" (src++), "0" (0), "1" (0));
> >
> >         if (failed) {
> >             die_if_no_fixup("invalid unaligned access", regs, ???);
> >             return;
> >         }
> >     }
> > }
> 
> 
> 
> Isn't that fairly much identical to __copy_user()? So a repeated 
> application of the following ought to suffice:
> 
> -               memcpy(dst, src, count);
> +               if (__copy_user(dst, src, count)) {
> +                       die_if_no_fixup("Fault in unaligned fixup", regs, 0);
> +                       return;
> +               }

agreed.

> > An alternative fix, and one which I think I like better, is to try the
> > search_exception_table before doing any unaligned handling - unaligned
> > userspace pointers aren`t the kernel`s job to deal with.
> 
> You mean to search the exception table before attempting the fixup, and to
> refuse to do the fixup if there's an entry for $pc? I don't like that very
> much.

I don't like that very much either, especially since we can test for userspace
accesses trivially (dst&src&0x80000000)==0 for userspace accesses.

> I think we _should_ be fixing up unaligned userspace accesses. Not
> only when they're accessed from the kernel, but when they happen entirely in
> userspace.

that'd be MAC, TAS, FPU insns, and probably quite a lot of sh5 insns we don't
use in the kernel but have to emulate anyway.

> At the moment, I'm more concerned about what happens when the offending 
> insn is in a branch delay slot. It looks like regs->pc will be pointing 
> to the branch insn iff it was taken, so in that case, we have to fix up the 
> insn at $pc+2 and then also emulate the branch insn follow the branch.

yikes, looks like it.  Do we need -mno-mem-access-in-delay-slots for gcc ?