public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* Fix two theoretical races in atkbd
@ 2004-04-26 21:13 Pavel Machek
  0 siblings, 0 replies; only message in thread
From: Pavel Machek @ 2004-04-26 21:13 UTC (permalink / raw)
  To: kernel list, vojtech

Hi!

This fixes two theoretical problems in atkbd.c.

1) It is not okay to use volatile char instead of atomic type. It
should not matter on most architectures.

2) Theoretical race on smp. atkbd->ack = 1 at the begining.

sendbyte:

atkbd->ack = 0;
serio_write()
				atkbd_interrupt:
				if (!atkbd->ack)
[cpu delayed write up-to now]
				[acknowledge lost]

Memory barriers are needed to prevent that. Please apply,

						Pavel

--- tmp/linux/drivers/input/keyboard/atkbd.c	2004-04-05 10:45:16.000000000 +0200
+++ linux/drivers/input/keyboard/atkbd.c	2004-04-26 23:01:12.000000000 +0200
@@ -182,7 +182,7 @@
 	unsigned char extra;
 	unsigned char release;
 	int lastkey;
-	volatile signed char ack;
+	atomic_t ack;			/* 0 .. nothing, 1 .. ACK, 2 .. NAK */
 	unsigned char emul;
 	unsigned short id;
 	unsigned char write;
@@ -233,13 +233,14 @@
 		atkbd->resend = 0;
 #endif
 
-	if (!atkbd->ack)
+	mb();
+	if (!atomic_read(&atkbd->ack))
 		switch (code) {
 			case ATKBD_RET_ACK:
-				atkbd->ack = 1;
+				atomic_set(&atkbd->ack, 1);
 				goto out;
 			case ATKBD_RET_NAK:
-				atkbd->ack = -1;
+				atomic_set(&atkbd->ack, 2);
 				goto out;
 		}
 
@@ -368,7 +369,7 @@
 static int atkbd_sendbyte(struct atkbd *atkbd, unsigned char byte)
 {
 	int timeout = 20000; /* 200 msec */
-	atkbd->ack = 0;
+	atomic_set(&atkbd->ack, 0); mb();
 
 #ifdef ATKBD_DEBUG
 	printk(KERN_DEBUG "atkbd.c: Sent: %02x\n", byte);
@@ -376,9 +377,12 @@
 	if (serio_write(atkbd->serio, byte))
 		return -1;
 
-	while (!atkbd->ack && timeout--) udelay(10);
+	while (!atomic_read(&atkbd->ack) && timeout--) {
+		udelay(10);
+		mb();
+	}
 
-	return -(atkbd->ack <= 0);
+	return -(atomic_read(&atkbd->ack) != 1);
 }
 
 /*
@@ -710,7 +714,7 @@
 		atkbd->dev.rep[REP_PERIOD] = 33;
 	}
 
-	atkbd->ack = 1;
+	atomic_set(&atkbd->ack, 1); mb();
 	atkbd->serio = serio;
 
 	init_input_dev(&atkbd->dev);

-- 
934a471f20d6580d5aad759bf0d97ddc

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2004-04-26 21:14 UTC | newest]

Thread overview: (only message) (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-04-26 21:13 Fix two theoretical races in atkbd Pavel Machek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox