[PATCH] ppc32: Fix alignment exception checking on load/store multiple instructions

[PATCH] ppc32: Fix alignment exception checking on load/store multiple instructions

[Kumar Gala]

Paulus,

Can you take a look and ack this patch before I send to akpm.

The handling of misaligned load/store multiplies did not check to see if 
the address were ok to access before __{get,put}_user().


Signed-off-by: Kumar Gala <kumar.gala@freescale.com>

---
diff -Nru a/arch/ppc/kernel/align.c b/arch/ppc/kernel/align.c
--- a/arch/ppc/kernel/align.c	2005-04-12 01:00:10 -05:00
+++ b/arch/ppc/kernel/align.c	2005-04-12 01:00:10 -05:00
@@ -290,6 +290,10 @@
 			/* lwm, stmw */
 			nb = (32 - reg) * 4;
 		}
+
+		if (!access_ok((flags & ST? VERIFY_WRITE: VERIFY_READ), addr, nb+nb0))
+			return -EFAULT;	/* bad address */
+
 		rptr = (unsigned char *) &regs->gpr[reg];
 		if (flags & LD) {
 			for (i = 0; i < nb; ++i)

Re: [PATCH] ppc32: Fix alignment exception checking on load/store multiple instructions

[Dan Malek]

On Apr 12, 2005, at 2:03 AM, Kumar Gala wrote:

> The handling of misaligned load/store multiplies did not check to see 
> if
> the address were ok to access before __{get,put}_user().

I think we should also take the opportunity to fix up the lawrx
case and look for other reserved/conditional instructions
that may slip through.  Since these are atomic operations, we
can't emulate them.  According to the PEM, an alignment fault
on these is a fatal programming error.

Thanks.

	-- Dan

Re: [PATCH] ppc32: Fix alignment exception checking on load/store multiple instructions

[Kumar Gala]

On Apr 12, 2005, at 9:46 AM, Dan Malek wrote:

>
>
> On Apr 12, 2005, at 2:03 AM, Kumar Gala wrote:
>
> > The handling of misaligned load/store multiplies did not check to =
see
> > if
>  > the address were ok to access before __{get,put}_user().
>
> I think we should also take the opportunity to fix up the lawrx
> case and look for other reserved/conditional instructions
>  that may slip through.=A0 Since these are atomic operations, we
> can't emulate them.=A0 According to the PEM, an alignment fault
> on these is a fatal programming error.

When you say "fix up" I assume you mean lwarx should return 0.  It=20
appears that stwcx. is already doing that.  Can't think of any other=20
cases that need fixing.

- kumar

Re: [PATCH] ppc32: Fix alignment exception checking on load/store multiple instructions

[Kumar Gala]

>  > > The handling of misaligned load/store multiplies did not check to=20=

> see
>  > > if
>  >=A0 > the address were ok to access before __{get,put}_user().
> >
>  > I think we should also take the opportunity to fix up the lawrx
> > case and look for other reserved/conditional instructions
>  >=A0 that may slip through.=A0 Since these are atomic operations, we
> > can't emulate them.=A0 According to the PEM, an alignment fault
> > on these is a fatal programming error.
>
> When you say "fix up" I assume you mean lwarx should return 0.=A0 It
> appears that stwcx. is already doing that.=A0 Can't think of any other
> cases that need fixing.

Upon further review, the PEM and PPC Arch spec, say that its ok to=20
emulate lwarz as an lwz.  =46rom the spec:

The instructions lwz and lwarx give the same DSISR bits (all zero). But=20=

if lwarx causes an Alignment interrupt, it should not be emulated. It=20
is adequate for the Alignment interrupt handler simply to treat the=20
instruction as if it were lwz. The emulator
must use the address in the DAR, rather than compute it from RA/RB/D,=20
because lwz and lwarx have different instruction formats.

So we are handled lwarx according to the arch specs already.

- kumar=

Re: [PATCH] ppc32: Fix alignment exception checking on load/store multiple instructions

[Dan Malek]

On Apr 12, 2005, at 11:06 AM, Kumar Gala wrote:

> When you say "fix up" I assume you mean lwarx should return 0.  It 
> appears that stwcx. is already doing that.  Can't think of any other 
> cases that need fixing.

Yes, it should return an error.  From a quick look at the slicing of the
bits in the code, it appears lawrx is decoded the same as lw.

Thanks.

	-- Dan

Re: [PATCH] ppc32: Fix alignment exception checking on load/store multiple instructions

[Dan Malek]

On Apr 12, 2005, at 11:26 AM, Kumar Gala wrote:

> Upon further review, the PEM and PPC Arch spec, say that its ok to 
> emulate lwarz as an lwz.  From the spec:

Hmmm ...  Seems weird.  Since the emulation won't create the 
reservation,
the subsequent stwcx will fail.  If the stwcx to the same unaligned 
address
will be a programming error.

Also, the EREF states that neither the lwarx nor stwcx should be 
emulated,
and it's a programming error to have unaligned accesses with these.
I still don't like this "similar but different" Book-E architecture, 
but I guess
we have to live with it ....


> The instructions lwz and lwarx give the same DSISR bits (all zero). 
> But if lwarx causes an Alignment interrupt, it should not be emulated.

???  Those are nearly the same words from the EREF, I just didn't find 
anything
like the following.

> ... It is adequate for the Alignment interrupt handler simply to treat 
> the instruction as if it were lwz. The emulator
> must use the address in the DAR, rather than compute it from RA/RB/D, 
> because lwz and lwarx have different instruction formats.

I guess it's done as lwz because it's not possible to actually emulate 
an
unaligned lwarx?

> So we are handled lwarx according to the arch specs already.

If that's the way you read it :-)   Probably not worth the discussion, 
but
I brought it up since we are here and it will be soon forgotten.

Thanks.


	-- Dan

Re: [PATCH] ppc32: Fix alignment exception checking on load/store multiple instructions

[Kumar Gala]

Paul,

What is the state of ack'ing this patch?

- kumar

On Apr 12, 2005, at 1:03 AM, Kumar Gala wrote:

> Paulus,
>
> Can you take a look and ack this patch before I send to akpm.
>
> The handling of misaligned load/store multiplies did not check to see=20=

> if
> the address were ok to access before __{get,put}_user().
>
>
>
> Signed-off-by: Kumar Gala <kumar.gala@freescale.com>
>
> ---
>  diff -Nru a/arch/ppc/kernel/align.c b/arch/ppc/kernel/align.c
> --- a/arch/ppc/kernel/align.c=A0=A0 2005-04-12 01:00:10 -05:00
>  +++ b/arch/ppc/kernel/align.c=A0=A0 2005-04-12 01:00:10 -05:00
>  @@ -290,6 +290,10 @@
>  =A0=A0=A0=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 /* =
lwm, stmw */
>  =A0=A0=A0=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 nb =
=3D (32 - reg) * 4;
>  =A0=A0=A0=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 }
>  +
>  +=A0=A0=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 if (!access_ok((flags & ST? =
VERIFY_WRITE:=20
> VERIFY_READ), addr, nb+nb0))
> +=A0=A0=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 return =
-EFAULT; /* bad address */
> +
>  =A0=A0=A0=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 rptr =3D (unsigned char =
*) &regs->gpr[reg];
>  =A0=A0=A0=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 if (flags & LD) {
>  =A0=A0=A0=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 =A0=A0=A0=A0=A0=A0=A0 for =
(i =3D 0; i < nb; ++i)
>  _______________________________________________
> Linuxppc-dev mailing list
>  Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH] ppc32: Fix alignment exception checking on load/store multiple instructions

[Paul Mackerras]

Kumar Gala writes:

> What is the state of ack'ing this patch?

It's fine, I'll forward it to akpm in due course.

Paul.