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

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

On Wed, Nov 22, 2000 at 05:31:41PM +0000, David Howells wrote:
> 
> I've attached a patch to do fixups on all instructions that can incur an
> unaligned access fault, barring "multiply from postinc memory operands". It
> also handles faults in jump, branch and return delay slots.

and adds unaligned userspace access emulation, which I still think is a
terribly bad idea;  it also adds at least several security holes:
1. accesses to [0x80000000,0xffffffff] generate address errors while in
userspace.
2. if the faulting insn is just before 0x80000000, we must not access memory
after 0x80000000 to fetch data.
3. unaligned accesses to [0x80000000,0xffffffff] get emulated

> +/*
> + * try and fix up kernelspace address errors
> + * - userspace errors just cause EFAULT to be returned, resulting in SEGV
> + * - kernel/userspace interfaces cause a jump to an appropriate handler
> + * - other kernel errors are bad
> + * - return 0 if fixed-up, -EFAULT if non-fatal (to the kernel) fault
> + */
> +static int die_if_no_fixup(const char * str, struct pt_regs * regs, long err)
>  {
>  	if (!user_mode(regs))
>  	{
> @@ -85,27 +96,39 @@
>  		fixup = search_exception_table(regs->pc);
>  		if (fixup) {
>  			regs->pc = fixup;
> -			return;
> +			return 0;
>  		}
>  		die(str, regs, err);
>  	}
> -}
> +	return -EFAULT;
> +} /* end die_if_no_fixup() */

what's the rationale for the -EFAULT return value here ?  All the
information is already there, and we don't know enough about the context
to decide whether we're in a -EFAULT situation at all.

> -static void handle_unaligned(u16 instruction, struct pt_regs *regs)
> +/*****************************************************************************/
> +/*
> + * handle an instruction that does an unaligned memory access by emulating the
> + * desired behaviour
> + * - note that PC _may not_ point to the faulting instruction (if that instruction
> + *   is in a branch delay slot
> + * - return 0 if emulation okay, -EFAULT on existential error

same here ..

> + */
> +static int handle_unaligned_ins(u16 instruction, struct pt_regs *regs)
>  {

userspace accesses to [0x80000000,0xffffffff] also get here, and must
generate a SEGV rather than a memory access (which doesn't get caught
by __copy_user either).

> +		if (__copy_user(&instruction, (u16 *)(regs->pc), 2)) {
> +			/* Argh. Fault on the instruction itself.
> +			   This should never happen non-SMP
> +			*/
> +			goto uspace_segv;

Isn't this what happens when you jsr to an address in the top two GB ?

> -	} else {
> -		u16 instruction;
> -		
> -		if(regs->pc & 1)
> +	}
> +	else {

yuck, see CodingStyle ;)