Improper handling of unaligned user address access?

Improper handling of unaligned user address access?

[Steven J. Hill]

Greetings.

I am having some issues using 'copy_from_user' in a driver. The issue
is that instead of returning a EFAULT for a bad address, it throws a
kernel panic and then proceeds to segfault the application. I am doing
a test on the module where I pass in an invalid user source address:

    copy_from_user(&dst, src, sizeof(dst));

where 'src' is equal to '0xa'. Now for the interesting part. When it
goes to do the copy, in 'arch/mips/lib/memcpy.S' it correctly jumps
to 'src_unaligned_dst_aligned' and then to 'cleanup_src_unaligned'
and we have the following code:

    8025f004 <cleanup_src_unaligned>:
    8025f004:       10c00017        beqz    a2,8025f064 <done>
    8025f008:       30d80003        andi    t8,a2,0x3
    8025f00c:       13060009        beq     t8,a2,8025f034 <copy_bytes>
    8025f010:       88a80000        lwl     t0,0(a1)

The instruction at 8025f00c is the offending instruction, however, the
kernel oops that kills the process shows:

    Unable to handle kernel paging request at virtual address 0000000a,
    epc == 8025f00c, ra == 8011c3c8
    Oops in fault.c:do_page_fault, line 199:
    $0 : 00000000 00000012 0000001a 0000001a 87887f10 0000000a 00000008 
00000001
    $8 : 00000000 00000000 00000000 00001116 802ec2f0 fffffffe ffffffff 
00000010
    $16: 0000000a 7fff7d68 87887f10 00000000 004009b4 00000000 00000000 
00000000
    $24: 00000000 87887e18                   87886000 87887f00 7fff7d30 
8011c3c8
    Hi : 00000000
    Lo : 00000000
    epc  : 8025f00c    Not tainted
    Status: 3000fc03
    Cause : 90000008

I am using the last version of the 2.4.18 Linux/MIPS kernel. It looks
like there was a possible fix for this in 'arch/mips/kernel/unaligned.c'
by Ralf, but it did not seem to work. Any thoughts on this?

-Steve

Re: Improper handling of unaligned user address access?

[Geert Uytterhoeven]

On Mon, 3 Mar 2003, Steven J. Hill wrote:
> I am having some issues using 'copy_from_user' in a driver. The issue
> is that instead of returning a EFAULT for a bad address, it throws a
> kernel panic and then proceeds to segfault the application. I am doing
> a test on the module where I pass in an invalid user source address:
> 
>     copy_from_user(&dst, src, sizeof(dst));
> 
> where 'src' is equal to '0xa'. Now for the interesting part. When it
> goes to do the copy, in 'arch/mips/lib/memcpy.S' it correctly jumps
> to 'src_unaligned_dst_aligned' and then to 'cleanup_src_unaligned'
> and we have the following code:
> 
>     8025f004 <cleanup_src_unaligned>:
>     8025f004:       10c00017        beqz    a2,8025f064 <done>
>     8025f008:       30d80003        andi    t8,a2,0x3
>     8025f00c:       13060009        beq     t8,a2,8025f034 <copy_bytes>
>     8025f010:       88a80000        lwl     t0,0(a1)
> 
> The instruction at 8025f00c is the offending instruction, however, the
                     ^^^^^^^^
Don't you mean 8025f010?

> kernel oops that kills the process shows:
> 
>     Unable to handle kernel paging request at virtual address 0000000a,
>     epc == 8025f00c, ra == 8011c3c8
>     Oops in fault.c:do_page_fault, line 199:
>     $0 : 00000000 00000012 0000001a 0000001a 87887f10 0000000a 00000008 
> 00000001
>     $8 : 00000000 00000000 00000000 00001116 802ec2f0 fffffffe ffffffff 
> 00000010
>     $16: 0000000a 7fff7d68 87887f10 00000000 004009b4 00000000 00000000 
> 00000000
>     $24: 00000000 87887e18                   87886000 87887f00 7fff7d30 
> 8011c3c8
>     Hi : 00000000
>     Lo : 00000000
>     epc  : 8025f00c    Not tainted
>     Status: 3000fc03
>     Cause : 90000008
> 
> I am using the last version of the 2.4.18 Linux/MIPS kernel. It looks
> like there was a possible fix for this in 'arch/mips/kernel/unaligned.c'
> by Ralf, but it did not seem to work. Any thoughts on this?

This looks like the unaligned access in a branch delay slot problem I
experienced a while ago, where the CPU doesn't set the BD flag if the branch is
not taken. Can you please try the patch I posted?

BTW, what kind of CPU is this? A VR41xx?

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

Re: Improper handling of unaligned user address access?

[Mike Uhler]

<snip>

> > 
> >     8025f004 <cleanup_src_unaligned>:
> >     8025f004:       10c00017        beqz    a2,8025f064 <done>
> >     8025f008:       30d80003        andi    t8,a2,0x3
> >     8025f00c:       13060009        beq     t8,a2,8025f034 <copy_bytes>
> >     8025f010:       88a80000        lwl     t0,0(a1)
> > 
> > The instruction at 8025f00c is the offending instruction, however, the
>                      ^^^^^^^^
> Don't you mean 8025f010?
> 

<snip>

> >     epc == 8025f00c, ra == 8011c3c8
> >     epc  : 8025f00c    Not tainted
> >     Status: 3000fc03
> >     Cause : 90000008
> > 
> > I am using the last version of the 2.4.18 Linux/MIPS kernel. It looks
> > like there was a possible fix for this in 'arch/mips/kernel/unaligned.c'
> > by Ralf, but it did not seem to work. Any thoughts on this?
> 
> This looks like the unaligned access in a branch delay slot problem I
> experienced a while ago, where the CPU doesn't set the BD flag if the branch is
> not taken. Can you please try the patch I posted?

In this particular case, it would appear that it's not the delay slot problem.
According to the Cause value above, BD is set, and EPC has been rolled
back to point at the branch.  That all looks consistent to me.

Note that the lwl will not take an unaligned exception, and the Cause code
value indicates a TLB miss.  I don't have the full context of the problem,
but is 0xA (i.e., virtual page zero) actually a valid address?  If not,
that's the cause of the problem.

By the way, having the oops message put out the BadVAddr and PRId CP0 registers
would be very helpful.

/gmu
-- 

  =*=*=*=*=*=*=*=*=*=*=*=*=*=*=*=*=*=*=*=*=*=*=*=*=*=*=*=*=*=*=*=*=*=*=*=*=
  Michael Uhler, VP, Systems, Architecture, and Software Products 
  MIPS Technologies, Inc.   Email: uhler@mips.com   Pager: uhler_p@mips.com
  1225 Charleston Road      Voice:  (650)567-5025   FAX:   (650)567-5225
  Mountain View, CA 94043   Mobile: (650)868-6870   Admin: (650)567-5085

Re: Improper handling of unaligned user address access?

[Mike Uhler]

<snip>

> > 
> >     8025f004 <cleanup_src_unaligned>:
> >     8025f004:       10c00017        beqz    a2,8025f064 <done>
> >     8025f008:       30d80003        andi    t8,a2,0x3
> >     8025f00c:       13060009        beq     t8,a2,8025f034 <copy_bytes>
> >     8025f010:       88a80000        lwl     t0,0(a1)
> > 
> > The instruction at 8025f00c is the offending instruction, however, the
>                      ^^^^^^^^
> Don't you mean 8025f010?
> 

<snip>

> >     epc == 8025f00c, ra == 8011c3c8
> >     epc  : 8025f00c    Not tainted
> >     Status: 3000fc03
> >     Cause : 90000008
> > 
> > I am using the last version of the 2.4.18 Linux/MIPS kernel. It looks
> > like there was a possible fix for this in 'arch/mips/kernel/unaligned.c'
> > by Ralf, but it did not seem to work. Any thoughts on this?
> 
> This looks like the unaligned access in a branch delay slot problem I
> experienced a while ago, where the CPU doesn't set the BD flag if the branch is
> not taken. Can you please try the patch I posted?

In this particular case, it would appear that it's not the delay slot problem.
According to the Cause value above, BD is set, and EPC has been rolled
back to point at the branch.  That all looks consistent to me.

Note that the lwl will not take an unaligned exception, and the Cause code
value indicates a TLB miss.  I don't have the full context of the problem,
but is 0xA (i.e., virtual page zero) actually a valid address?  If not,
that's the cause of the problem.

By the way, having the oops message put out the BadVAddr and PRId CP0 registers
would be very helpful.

/gmu
-- 

  =*=*=*=*=*=*=*=*=*=*=*=*=*=*=*=*=*=*=*=*=*=*=*=*=*=*=*=*=*=*=*=*=*=*=*=*=
  Michael Uhler, VP, Systems, Architecture, and Software Products 
  MIPS Technologies, Inc.   Email: uhler@mips.com   Pager: uhler_p@mips.com
  1225 Charleston Road      Voice:  (650)567-5025   FAX:   (650)567-5225
  Mountain View, CA 94043   Mobile: (650)868-6870   Admin: (650)567-5085

Re: Improper handling of unaligned user address access?

[Steven J. Hill]

Mike Uhler wrote:
>>
>>This looks like the unaligned access in a branch delay slot problem I
>>experienced a while ago, where the CPU doesn't set the BD flag if the branch is
>>not taken. Can you please try the patch I posted?
> 
> 
> In this particular case, it would appear that it's not the delay slot problem.
> According to the Cause value above, BD is set, and EPC has been rolled
> back to point at the branch.  That all looks consistent to me.
> 
> Note that the lwl will not take an unaligned exception, and the Cause code
> value indicates a TLB miss.  I don't have the full context of the problem,
> but is 0xA (i.e., virtual page zero) actually a valid address?  If not,
> that's the cause of the problem.
> 
You are correct. 0xA is NOT actually a valid address which is the
problem. I believe the kernel should handle things more gracefully
and return EFAULT instead of killing the process. This code of mine
worked fine in an older 2.4.7 kernel. Why was 'verify_area' taken
out of 'traps.c' by the way?

> By the way, having the oops message put out the BadVAddr and PRId CP0 registers
> would be very helpful.
> 
Here it is:

    <1>Unable to handle kernel paging request at virtual address 000000a,
      epc == 8025f00c, ra == 8011c3d4
    <1>BADVADDR = 00000000, CP0_PRID = 00002d22
    Oops in fault.c:do_page_fault, line 201:
    $0 : 00000000 00000012 0000001a 0000001a 8789ff18 0000000a 00000008
      7fff7d58
    $8 : 00000000 00000000 00000000 00000000 71236429 2aaa8000 7fff77f8
      2aaa83f8
    $16: 8789ff18 7fff7d58 00000000 00000001 004009b4 00000000 00000000
      00000000
    $24: 00000000 00401638                   8789e000 8789ff08 7fff7d20
      8011c3d4
    Hi : 00000007
    Lo : 00000000
    epc  : 8025f00c    Tainted: P
    Status: 3000fc03
    Cause : 90000008

The processor is a TX4927.

-Steve

Re: Improper handling of unaligned user address access?

[Steven J. Hill]

[-- Attachment #1: Type: text/plain, Size: 576 bytes --]

The first thing I tried to fix this issue was to use the
'memcpy.S' file from 2.4.7 and that actually worked, but
that was a step backwards. It was much simpler to just
add a 'nop' after the offending branch instruction. It
fixes all of my problems with 'copy_from_user'. I have
already checked these into both the 2.4 and 2.5 trees.

I do have one further question. In 'arch/mips/mm/fault.c'
when we need to do a fixup:

    fixup = search_exception_table(regs->cp0_epc);

Why do we not check to see if the EPC is a branch insn
before looking in the exception table?

-Steve

[-- Attachment #2: memcpy-2.4.diff --]
[-- Type: text/plain, Size: 483 bytes --]

Index: memcpy.S
===================================================================
RCS file: /home/cvs/linux/arch/mips/lib/memcpy.S,v
retrieving revision 1.6.2.4
diff -u -r1.6.2.4 memcpy.S
--- memcpy.S	19 Sep 2002 14:01:24 -0000	1.6.2.4
+++ memcpy.S	4 Mar 2003 00:09:52 -0000
@@ -357,6 +357,7 @@
 	beqz	len, done
 	 and	rem, len, NBYTES-1  # rem = len % NBYTES
 	beq	rem, len, copy_bytes
+	nop
 1:
 EXC(	 LDFIRST t0, FIRST(0)(src),	l_exc)
 EXC(	LDREST	t0, REST(0)(src),	l_exc_copy)

[-- Attachment #3: memcpy-2.5.diff --]
[-- Type: text/plain, Size: 474 bytes --]

Index: memcpy.S
===================================================================
RCS file: /home/cvs/linux/arch/mips/lib/memcpy.S,v
retrieving revision 1.11
diff -u -r1.11 memcpy.S
--- memcpy.S	19 Sep 2002 14:01:28 -0000	1.11
+++ memcpy.S	4 Mar 2003 00:10:58 -0000
@@ -357,6 +357,7 @@
 	beqz	len, done
 	 and	rem, len, NBYTES-1  # rem = len % NBYTES
 	beq	rem, len, copy_bytes
+	nop
 1:
 EXC(	 LDFIRST t0, FIRST(0)(src),	l_exc)
 EXC(	LDREST	t0, REST(0)(src),	l_exc_copy)

Re: Improper handling of unaligned user address access?

[Jun Sun]

On Mon, Mar 03, 2003 at 06:14:20PM -0600, Steven J. Hill wrote:
> The first thing I tried to fix this issue was to use the
> 'memcpy.S' file from 2.4.7 and that actually worked, but
> that was a step backwards. It was much simpler to just
> add a 'nop' after the offending branch instruction. It
> fixes all of my problems with 'copy_from_user'. 

Adding 'nop' seems to be right.

> I do have one further question. In 'arch/mips/mm/fault.c'
> when we need to do a fixup:
> 
>     fixup = search_exception_table(regs->cp0_epc);
>
> Why do we not check to see if the EPC is a branch insn
> before looking in the exception table?
>

You must be looking at a different tree.  We do check epc
for branch instruction:

	fixup = search_exception_table(exception_epc(regs));

Jun