Subject: Unaligned address handling, and the cause of that login problem

From: "Mike Klar" <[email-masked]>
Date: Sun, 16 Apr 2000 15:19:01 -0700

While tracking down a random memory corruption bug, I stumbled across the
cause of that telnet/ssh problem in recent kernels reported about a month
ago:

The version of down_trylock() for CPUs with support LL/SC assumes that
struct semaphore is 64-bit aligned, since it accesses count and waking as a
single dualword (with lld/scd).  Nothing in struct semaphore guarantees this
alignment, and in fact, struct tty_struct has a struct semaphore that is not
64-bit aligned.  Depending on how a tty is used (I think it's a non-blocking
read that triggers the problem, in drivers/char/n_tty.c), the kernel will
attempt an unaligned lld, it will cause an address error, and the handler in
arch/mips/kernel/unaligned.c will kill current with SIGBUS (since lld/scd
cannot be properly simulated).

The quick-and-dirty workaround is to put 32 bits of padding before the
atomic_read member of struct tty_struct.  Of course, that doesn't fix the
real problem, and there may well be other non-64-bit aligned struct
semaphore's out there.  A proper fix would be to either hack up struct
semaphore to guarantee dualword alignment, or rework the was down_trylock
does its thing.

While I'm on the topic of unaligned handling, this behavior of sending
SIGBUS, SIGSEGV, or SIGILL to current on unaligned accesses seems to me like
incorrect behavior if the original fault happened in kernel mode.  The above
example of an unaligned lld sending SIGBUS is not too bad, since the fault
does happen while doing something on behalf of the current process.
Consider this example, though:  If kernel code attempts an unaligned word
read to virtual address 0x00000001 (for example), the unaligned handler will
attempt to simulate with 2 aligned reads, which will fault, and since the
unaligned handler catches those faults, it will wind up sending SIGSEGV to
current.  I would think that condition should cause an oops, since that's
what an equivalent aligned access would do, and especially since the access
may have had nothing to do with current (it may happen from an interrupt,
for example).

Comments?

Mike Klar
Wyldfier Technology

---

From: Harald Koerfgen <[email-masked]>
Date: Mon, 17 Apr 2000 18:33:34 +0200 (CEST)

On 16-Apr-00 Mike Klar wrote:
> single dualword (with lld/scd).

Good spotted. This is perfectly in line with my observation that telnet/ssh
worked perfectly well if you built a kernel without CONFIG_CPU_HAS_LLSC.

The attached patch seems to fix this, and another bug in
waking_non_zero_interruptible() as well.

Telnet is working again :)

-- 
Regards,
Harald

---

From: Ralf Baechle <[email-masked]>
Date: Mon, 17 Apr 2000 16:43:33 -0700

> semaphore to guarantee dualword alignment, or rework the was down_trylock

I'll put __attribute__ ((aligned(64))) to the structure which will fix this.
This will have to be changed again when we add support for 32-bit processors
with ll / sc instructions but for now we don't support them, so it's the
right thing.

> incorrect behavior if the original fault happened in kernel mode.

> The above

The assumption is that the kernel should never ever use ll, lld, sc and scd
on improperly aligned memory objects, so not checking is ok.  In other
words it's it's perfectly ok if the kernel dies or behaves silly following
such a can-not-happen case.

Note that while we don't attemt to handle missaligned ll/sc/lld/scd
instructions because that would break atomicity on SMP machines.  On the
other side again emulating them on CPUs that don't have them at all like
the R3000 is ok because those are not used on SMP systems.  That is not
counting the oddball SMP systems which we'll probably not support ever.

  Ralf

---

From: Florian Lohoff <[email-masked]>
Date: Tue, 18 Apr 2000 10:53:48 +0200

> Telnet is working again :)

Many thanks - It works :)

(root@193)~# cat /proc/cpuinfo 
cpu                     : MIPS
cpu model               : R4000SC V3.0
system type             : Digital DECstation 5000/1xx
BogoMIPS                : 49.81
byteorder               : little endian
unaligned accesses      : 0
wait instruction        : no
microsecond timers      : yes
extra interrupt vector  : no
hardware watchpoint     : yes
VCED exceptions         : 13636
VCEI exceptions         : 113050

(root@193)~# uname -a
Linux repeat.rfc822.org 2.3.99-pre3 #3 Tue Apr 18 10:31:47 CEST 2000 mips unknown

But the (kernel) fix from Ralph concerning the sleep? syscalls seems
to be incorrect or buggy - When calling top the display refreshes
multiple times a second without a sleep and on the console i get
an.

Setting flush to zero for top.
schedule_timeout: wrong timeout value fffbd0b2 from 800942b8 

Flo
-- 
Florian Lohoff		[email-masked]		      	+49-subject-2-change
"Technology is a constant battle between manufacturers producing bigger and
more idiot-proof systems and nature producing bigger and better idiots."

---

From: Florian Lohoff <[email-masked]>
Date: Tue, 18 Apr 2000 12:00:05 +0200

> schedule_timeout: wrong timeout value fffbd0b2 from 800942b8 

And NTP still doesnt work as it still has a to strong drift ...

status=c011 sync_alarm, sync_unspec, 1 event, event_restart
processor="mips", system="Linux2.3.99-pre3", leap=11, stratum=16,
precision=-18, rootdelay=0.00, rootdispersion=58.98, peer=0,
refid=0.0.0.0, reftime=00000000.00000000  Thu, Feb  7 2036  6:28:16.000,
poll=6, clock=bca6b000.defc8b00  Tue, Apr 18 2000  9:57:20.871, state=1,
phase=0.000, frequency=14.728, jitter=0.000, stability=0.000

~0.1 Seconds in 4 Seconds 

Flo

---

From: Ralf Baechle <[email-masked]>
Date: Tue, 18 Apr 2000 14:04:10 -0700

> schedule_timeout: wrong timeout value fffbd0b2 from 800942b8 

I'm fairly sure that my patch is not buggy - I haven't made any :-)

  Ralf

---

From: Florian Lohoff <[email-masked]>
Date: Tue, 18 Apr 2000 23:16:52 +0200

> I'm fairly sure that my patch is not buggy - I haven't made any :-)

Hasnt been there somethign with a missing syscall you wanted to add
again to all kernels ? I remembered dark but couldnt find anything
in the cvs. :)

Flo

---

From: Geert Uytterhoeven <[email-masked]>
Date: Tue, 18 Apr 2000 17:13:48 +0200 (CEST)

> I'll put __attribute__ ((aligned(64))) to the structure which will fix this.
                                   ^^
8, I suppose?

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- Linux/{m68k~Amiga,PPC~CHRP} -- [email-masked]

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

---

From: Ralf Baechle <[email-masked]>
Date: Tue, 18 Apr 2000 14:40:41 -0700

> > I'll put __attribute__ ((aligned(64))) to the structure which will fix this.

Of course.

  Ralf

---

From: Ralf Baechle <[email-masked]>
Date: Tue, 18 Apr 2000 14:39:59 -0700

> again to all kernels ? I remembered dark but couldnt find anything

Oh yes, it just got lost.  It's now in the works.

  Ralf

---

From: Florian Lohoff <[email-masked]>
Date: Tue, 18 Apr 2000 23:43:20 +0200

> Oh yes, it just got lost.  It's now in the works.

-----------flo
> multiple times a second without a sleep and on the console i get
-----------flo

I stand corrected - It hasnt even been applied :)

Flo