[PATCH 1/5] m68k: Disable/restore interrupts in hwreg_present()/hwreg_write()

[PATCH 1/5] m68k: Disable/restore interrupts in hwreg_present()/hwreg_write()

[Geert Uytterhoeven]

hwreg_present() and hwreg_write() temporarily change the VBR register to
another vector table. This table contains a valid bus error handler
only, all other entries point to arbitrary addresses.

If an interrupt comes in while the temporary table is active, the
processor will start executing at such an arbitrary address, and the
kernel will crash.

While most callers run early, before interrupt are disabled, or
explicitly disable interrupts, Finn Thain pointed out that macsonic has
one callsite that doesn't, causing intermittent boot crashes.
There's another unsafe callsite in hilkbd.

Fix this for good by disabling and restoring interrupts inside
hwreg_present() and hwreg_write().

Explicitly disabling interrupts can be removed from the callsites later.

Reported-by: Finn Thain <fthain@telegraphics.com.au>
Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: stable@vger.kernel.org
---
 arch/m68k/mm/hwtest.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/m68k/mm/hwtest.c b/arch/m68k/mm/hwtest.c
index 2c7dde3c6430fc3b..2a5259fd23ebc532 100644
--- a/arch/m68k/mm/hwtest.c
+++ b/arch/m68k/mm/hwtest.c
@@ -28,9 +28,11 @@
 int hwreg_present( volatile void *regp )
 {
     int	ret = 0;
+    unsigned long flags;
     long	save_sp, save_vbr;
     long	tmp_vectors[3];
 
+    local_irq_save(flags);
     __asm__ __volatile__
 	(	"movec	%/vbr,%2\n\t"
 		"movel	#Lberr1,%4@(8)\n\t"
@@ -46,6 +48,7 @@ int hwreg_present( volatile void *regp )
 		: "=&d" (ret), "=&r" (save_sp), "=&r" (save_vbr)
 		: "a" (regp), "a" (tmp_vectors)
                 );
+    local_irq_restore(flags);
 
     return( ret );
 }
@@ -58,9 +61,11 @@ EXPORT_SYMBOL(hwreg_present);
 int hwreg_write( volatile void *regp, unsigned short val )
 {
 	int		ret;
+	unsigned long flags;
 	long	save_sp, save_vbr;
 	long	tmp_vectors[3];
 
+	local_irq_save(flags);
 	__asm__ __volatile__
 	(	"movec	%/vbr,%2\n\t"
 		"movel	#Lberr2,%4@(8)\n\t"
@@ -78,6 +83,7 @@ int hwreg_write( volatile void *regp, unsigned short val )
 		: "=&d" (ret), "=&r" (save_sp), "=&r" (save_vbr)
 		: "a" (regp), "a" (tmp_vectors), "g" (val)
 	);
+	local_irq_restore(flags);
 
 	return( ret );
 }
-- 
1.9.1

[PATCH 2/5] m68k: Reformat arch/m68k/mm/hwtest.c

[Geert Uytterhoeven]

No functional changes

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 arch/m68k/mm/hwtest.c | 78 ++++++++++++++++++++++++++-------------------------
 1 file changed, 40 insertions(+), 38 deletions(-)

diff --git a/arch/m68k/mm/hwtest.c b/arch/m68k/mm/hwtest.c
index 2a5259fd23ebc532..fb8be4dd38c448d7 100644
--- a/arch/m68k/mm/hwtest.c
+++ b/arch/m68k/mm/hwtest.c
@@ -25,32 +25,32 @@
 
 #include <linux/module.h>
 
-int hwreg_present( volatile void *regp )
+int hwreg_present(volatile void *regp)
 {
-    int	ret = 0;
-    unsigned long flags;
-    long	save_sp, save_vbr;
-    long	tmp_vectors[3];
+	int ret = 0;
+	unsigned long flags;
+	long save_sp, save_vbr;
+	long tmp_vectors[3];
 
-    local_irq_save(flags);
-    __asm__ __volatile__
-	(	"movec	%/vbr,%2\n\t"
-		"movel	#Lberr1,%4@(8)\n\t"
-                "movec	%4,%/vbr\n\t"
-		"movel	%/sp,%1\n\t"
-		"moveq	#0,%0\n\t"
-		"tstb	%3@\n\t"
+	local_irq_save(flags);
+	__asm__ __volatile__ (
+		"movec %/vbr,%2\n\t"
+		"movel #Lberr1,%4@(8)\n\t"
+		"movec %4,%/vbr\n\t"
+		"movel %/sp,%1\n\t"
+		"moveq #0,%0\n\t"
+		"tstb %3@\n\t"
 		"nop\n\t"
-		"moveq	#1,%0\n"
-                "Lberr1:\n\t"
-		"movel	%1,%/sp\n\t"
-		"movec	%2,%/vbr"
+		"moveq #1,%0\n"
+	"Lberr1:\n\t"
+		"movel %1,%/sp\n\t"
+		"movec %2,%/vbr"
 		: "=&d" (ret), "=&r" (save_sp), "=&r" (save_vbr)
 		: "a" (regp), "a" (tmp_vectors)
-                );
-    local_irq_restore(flags);
+	);
+	local_irq_restore(flags);
 
-    return( ret );
+	return ret;
 }
 EXPORT_SYMBOL(hwreg_present);
 
@@ -58,34 +58,36 @@ EXPORT_SYMBOL(hwreg_present);
  * by a bus error handler. Returns 1 if successful, 0 otherwise.
  */
 
-int hwreg_write( volatile void *regp, unsigned short val )
+int hwreg_write(volatile void *regp, unsigned short val)
 {
-	int		ret;
+	int ret;
 	unsigned long flags;
-	long	save_sp, save_vbr;
-	long	tmp_vectors[3];
+	long save_sp, save_vbr;
+	long tmp_vectors[3];
 
 	local_irq_save(flags);
-	__asm__ __volatile__
-	(	"movec	%/vbr,%2\n\t"
-		"movel	#Lberr2,%4@(8)\n\t"
-		"movec	%4,%/vbr\n\t"
-		"movel	%/sp,%1\n\t"
-		"moveq	#0,%0\n\t"
-		"movew	%5,%3@\n\t"
-		"nop	\n\t"	/* If this nop isn't present, 'ret' may already be
-				 * loaded with 1 at the time the bus error
-				 * happens! */
-		"moveq	#1,%0\n"
+	__asm__ __volatile__ (
+		"movec %/vbr,%2\n\t"
+		"movel #Lberr2,%4@(8)\n\t"
+		"movec %4,%/vbr\n\t"
+		"movel %/sp,%1\n\t"
+		"moveq #0,%0\n\t"
+		"movew %5,%3@\n\t"
+		"nop\n\t"
+		/*
+		 * If this nop isn't present, 'ret' may already be loaded
+		 * with 1 at the time the bus error happens!
+		 */
+		"moveq #1,%0\n"
 	"Lberr2:\n\t"
-		"movel	%1,%/sp\n\t"
-		"movec	%2,%/vbr"
+		"movel %1,%/sp\n\t"
+		"movec %2,%/vbr"
 		: "=&d" (ret), "=&r" (save_sp), "=&r" (save_vbr)
 		: "a" (regp), "a" (tmp_vectors), "g" (val)
 	);
 	local_irq_restore(flags);
 
-	return( ret );
+	return ret;
 }
 EXPORT_SYMBOL(hwreg_write);
 
-- 
1.9.1

[PATCH 3/5] cirrus/mac89x0: Remove superfluous interrupt disable/restore

[Geert Uytterhoeven]

As of commit XXX ("m68k: Disable/restore interrupts in
hwreg_present()/hwreg_write()"), this is no longer needed.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
Do not apply before commit XXX has been applied.
---
 drivers/net/ethernet/cirrus/mac89x0.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/cirrus/mac89x0.c b/drivers/net/ethernet/cirrus/mac89x0.c
index e285f384b0966dfe..07719676c305fc0b 100644
--- a/drivers/net/ethernet/cirrus/mac89x0.c
+++ b/drivers/net/ethernet/cirrus/mac89x0.c
@@ -216,14 +216,10 @@ struct net_device * __init mac89x0_probe(int unit)
 	ioaddr = (unsigned long)
 		nubus_slot_addr(slot) | (((slot&0xf) << 20) + DEFAULTIOBASE);
 	{
-		unsigned long flags;
 		int card_present;
 
-		local_irq_save(flags);
-		card_present = (hwreg_present((void*) ioaddr+4) &&
-				hwreg_present((void*) ioaddr + DATA_PORT));
-		local_irq_restore(flags);
-
+		card_present = (hwreg_present((void *)ioaddr + 4) &&
+				hwreg_present((void *)ioaddr + DATA_PORT));
 		if (!card_present)
 			goto out;
 	}
-- 
1.9.1

[PATCH 4/5] natsemi/macsonic: Remove superfluous interrupt disable/restore

[Geert Uytterhoeven]

As of commit XXX ("m68k: Disable/restore interrupts in
hwreg_present()/hwreg_write()"), this is no longer needed.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
Do not apply before commit XXX has been applied.
---
 drivers/net/ethernet/natsemi/macsonic.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/net/ethernet/natsemi/macsonic.c b/drivers/net/ethernet/natsemi/macsonic.c
index 9e4ddbba70363b8f..66c2d50d5b8d12df 100644
--- a/drivers/net/ethernet/natsemi/macsonic.c
+++ b/drivers/net/ethernet/natsemi/macsonic.c
@@ -326,13 +326,9 @@ static int mac_onboard_sonic_probe(struct net_device *dev)
 	    macintosh_config->ident == MAC_MODEL_P588 ||
 	    macintosh_config->ident == MAC_MODEL_P575 ||
 	    macintosh_config->ident == MAC_MODEL_C610) {
-		unsigned long flags;
 		int card_present;
 
-		local_irq_save(flags);
 		card_present = hwreg_present((void*)ONBOARD_SONIC_REGISTERS);
-		local_irq_restore(flags);
-
 		if (!card_present) {
 			printk("none.\n");
 			return -ENODEV;
-- 
1.9.1

[PATCH 5/5] nubus: Remove superfluous interrupt disable/restore

[Geert Uytterhoeven]

As of commit XXX ("m68k: Disable/restore interrupts in
hwreg_present()/hwreg_write()"), this is no longer needed.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
Do not apply before commit XXX has been applied.
---
 drivers/nubus/nubus.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/nubus/nubus.c b/drivers/nubus/nubus.c
index 5066a7ef7b6c38a2..3319cf19deebe0f2 100644
--- a/drivers/nubus/nubus.c
+++ b/drivers/nubus/nubus.c
@@ -920,14 +920,10 @@ void __init nubus_probe_slot(int slot)
 	rp = nubus_rom_addr(slot);	
 	for(i = 4; i; i--)
 	{
-		unsigned long flags;
 		int card_present;
 
 		rp--;
-		local_irq_save(flags);
 		card_present = hwreg_present(rp);
-		local_irq_restore(flags);
-	       
 		if (!card_present)
 			continue;
 
-- 
1.9.1

Re: [PATCH 1/5] m68k: Disable/restore interrupts in hwreg_present()/hwreg_write()

[Andreas Schwab]

Geert Uytterhoeven <geert@linux-m68k.org> writes:

> While most callers run early, before interrupt are disabled, or

                                       interrupts are enabled

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH 1/5] m68k: Disable/restore interrupts in hwreg_present()/hwreg_write()

[Geert Uytterhoeven]

On Sun, Sep 28, 2014 at 11:40 AM, Andreas Schwab <schwab@linux-m68k.org> wrote:
>> While most callers run early, before interrupt are disabled, or
>
>                                        interrupts are enabled

Thanks, will fix.

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: [PATCH 1/5] m68k: Disable/restore interrupts in hwreg_present()/hwreg_write()

[Geert Uytterhoeven]

On Sun, Sep 28, 2014 at 11:44 AM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> On Sun, Sep 28, 2014 at 11:40 AM, Andreas Schwab <schwab@linux-m68k.org> wrote:
>>> While most callers run early, before interrupt are disabled, or
>>
>>                                        interrupts are enabled
>
> Thanks, will fix.

Fixed and queued for 3.18, together with the reformat in PATCH 2/5.

I'll resend the remaining 3 after v3.18-rc1.

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