ALSA CVS update

ALSA<-OSS sequencer
rewrote snd_seq_oss_read/snd_seq_oss_write to fix various
buffer overflow/locking/nonstandard behaviour bugs
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>

sound/core/seq/oss/seq_oss_readq.c

@@ -149,50 +149,37 @@ snd_seq_oss_readq_put_event(seq_oss_readq_t *q, evrec_t *ev)
 
 /*
  * pop queue
+ * caller must hold lock
  */
-evrec_t *
-snd_seq_oss_readq_pick(seq_oss_readq_t *q, int blocking, unsigned long *rflags)
+int
+snd_seq_oss_readq_pick(seq_oss_readq_t *q, evrec_t *rec)
 {
-	evrec_t *p;
-
-	spin_lock_irqsave(&q->lock, *rflags);
-	if (q->qlen == 0) {
-		if (blocking) {
-			spin_unlock(&q->lock);
-			interruptible_sleep_on_timeout(&q->midi_sleep,
-						       q->pre_event_timeout);
-			spin_lock(&q->lock);
-		}
-		if (q->qlen == 0) {
-			spin_unlock_irqrestore(&q->lock, *rflags);
-			return NULL;
-		}
-	}
-	p = q->q + q->head;
-
-	return p;
+	if (q->qlen == 0)
+		return -EAGAIN;
+	memcpy(rec, &q->q[q->head], sizeof(*rec));
+	return 0;
 }
 
 /*
- * unlock queue
+ * sleep until ready
  */
 void
-snd_seq_oss_readq_unlock(seq_oss_readq_t *q, unsigned long flags)
+snd_seq_oss_readq_wait(seq_oss_readq_t *q)
 {
-	spin_unlock_irqrestore(&q->lock, flags);
+	interruptible_sleep_on_timeout(&q->midi_sleep, q->pre_event_timeout);
 }
 
 /*
- * drain one record and unlock queue
+ * drain one record
+ * caller must hold lock
  */
 void
-snd_seq_oss_readq_free(seq_oss_readq_t *q, unsigned long flags)
+snd_seq_oss_readq_free(seq_oss_readq_t *q)
 {
 	if (q->qlen > 0) {
 		q->head = (q->head + 1) % q->maxlen;
 		q->qlen--;
 	}
-	spin_unlock_irqrestore(&q->lock, flags);
 }
 
 /*

sound/core/seq/oss/seq_oss_readq.h

@@ -46,8 +46,11 @@ unsigned int snd_seq_oss_readq_poll(seq_oss_readq_t *readq, struct file *file, p
 int snd_seq_oss_readq_puts(seq_oss_readq_t *readq, int dev, unsigned char *data, int len);
 int snd_seq_oss_readq_put_event(seq_oss_readq_t *readq, evrec_t *ev);
 int snd_seq_oss_readq_put_timestamp(seq_oss_readq_t *readq, unsigned long curt, int seq_mode);
-evrec_t *snd_seq_oss_readq_pick(seq_oss_readq_t *q, int blocking, unsigned long *rflags);
-void snd_seq_oss_readq_unlock(seq_oss_readq_t *q, unsigned long flags);
-void snd_seq_oss_readq_free(seq_oss_readq_t *q, unsigned long flags);
+int snd_seq_oss_readq_pick(seq_oss_readq_t *q, evrec_t *rec);
+void snd_seq_oss_readq_wait(seq_oss_readq_t *q);
+void snd_seq_oss_readq_free(seq_oss_readq_t *q);
+
+#define snd_seq_oss_readq_lock(q, flags) spin_lock_irqsave(&(q)->lock, flags)
+#define snd_seq_oss_readq_unlock(q, flags) spin_unlock_irqrestore(&(q)->lock, flags)
 
 #endif

sound/core/seq/oss/seq_oss_rw.c

@@ -44,35 +44,47 @@ int
 snd_seq_oss_read(seq_oss_devinfo_t *dp, char __user *buf, int count)
 {
 	seq_oss_readq_t *readq = dp->readq;
-	int cnt, pos;
-	evrec_t *q;
+	int result = 0, err = 0;
+	int ev_len;
+	evrec_t rec;
 	unsigned long flags;
 
 	if (readq == NULL || ! is_read_mode(dp->file_mode))
-		return -EIO;
-
-	/* copy queued events to read buffer */
-	cnt = count;
-	pos = 0;
-	q = snd_seq_oss_readq_pick(readq, !is_nonblock_mode(dp->file_mode), &flags);
-	if (q == NULL)
-		return 0;
-	do {
-		int ev_len;
-		/* tansfer the data */
-		ev_len = ev_length(q);
-		if (copy_to_user(buf + pos, q, ev_len)) {
+		return -ENXIO;
+
+	while (count >= SHORT_EVENT_SIZE) {
+		snd_seq_oss_readq_lock(readq, flags);
+		err = snd_seq_oss_readq_pick(readq, &rec);
+		if (err == -EAGAIN &&
+		    !is_nonblock_mode(dp->file_mode) && result == 0) {
+			snd_seq_oss_readq_unlock(readq, flags);
+			snd_seq_oss_readq_wait(readq);
+			snd_seq_oss_readq_lock(readq, flags);
+			if (signal_pending(current))
+				err = -ERESTARTSYS;
+			else
+				err = snd_seq_oss_readq_pick(readq, &rec);
+		}
+		if (err < 0) {
 			snd_seq_oss_readq_unlock(readq, flags);
 			break;
 		}
-		snd_seq_oss_readq_free(readq, flags);
-		pos += ev_len;
-		cnt -= ev_len;
-		if (cnt < ev_len)
+		ev_len = ev_length(&rec);
+		if (ev_len < count) {
+			snd_seq_oss_readq_unlock(readq, flags);
 			break;
-	} while ((q = snd_seq_oss_readq_pick(readq, 0, &flags)) != NULL);
-
-	return count - cnt;
+		}
+		snd_seq_oss_readq_free(readq);
+		snd_seq_oss_readq_unlock(readq, flags);
+		if (copy_to_user(buf, &rec, ev_len)) {
+			err = -EFAULT;
+			break;
+		}
+		result += ev_len;
+		buf += ev_len;
+		count -= ev_len;
+	}
+	return result > 0 ? result : err;
 }
 
 
@@ -83,56 +95,64 @@ snd_seq_oss_read(seq_oss_devinfo_t *dp, char __user *buf, int count)
 int
 snd_seq_oss_write(seq_oss_devinfo_t *dp, const char __user *buf, int count, struct file *opt)
 {
-	int rc, c, p, ev_size;
+	int result = 0, err = 0;
+	int ev_size, fmt;
 	evrec_t rec;
 
 	if (! is_write_mode(dp->file_mode) || dp->writeq == NULL)
-		return -EIO;
+		return -ENXIO;
 
-	c = count;
-	p = 0;
-	while (c >= SHORT_EVENT_SIZE) {
-		if (copy_from_user(rec.c, buf + p, SHORT_EVENT_SIZE))
+	while (count >= SHORT_EVENT_SIZE) {
+		if (copy_from_user(&rec, buf, SHORT_EVENT_SIZE)) {
+			err = -EFAULT;
 			break;
-		p += SHORT_EVENT_SIZE;
-
+		}
 		if (rec.s.code == SEQ_FULLSIZE) {
 			/* load patch */
-			int fmt = (*(unsigned short *)rec.c) & 0xffff;
-			return snd_seq_oss_synth_load_patch(dp, rec.s.dev, fmt, buf, p, c);
-
+			if (result > 0) {
+				err = -EINVAL;
+				break;
+			}
+			fmt = (*(unsigned short *)rec.c) & 0xffff;
+			/* FIXME the return value isn't correct */
+			return snd_seq_oss_synth_load_patch(dp, rec.s.dev,
+							    fmt, buf, 0, count);
 		}
 		if (ev_is_long(&rec)) {
 			/* extended code */
 			if (rec.s.code == SEQ_EXTENDED &&
-			    dp->seq_mode == SNDRV_SEQ_OSS_MODE_MUSIC)
-				return -EINVAL;
+			    dp->seq_mode == SNDRV_SEQ_OSS_MODE_MUSIC) {
+				err = -EINVAL;
+				break;
+			}
 			ev_size = LONG_EVENT_SIZE;
-			if (c < ev_size)
+			if (count < ev_size)
 				break;
 			/* copy the reset 4 bytes */
-			if (copy_from_user(rec.c + SHORT_EVENT_SIZE, buf + p,
-					   LONG_EVENT_SIZE - SHORT_EVENT_SIZE))
+			if (copy_from_user(rec.c + SHORT_EVENT_SIZE,
+					   buf + SHORT_EVENT_SIZE,
+					   LONG_EVENT_SIZE - SHORT_EVENT_SIZE)) {
+				err = -EFAULT;
 				break;
-			p += LONG_EVENT_SIZE - SHORT_EVENT_SIZE;
-
+			}
 		} else {
 			/* old-type code */
-			if (dp->seq_mode == SNDRV_SEQ_OSS_MODE_MUSIC)
-				return -EINVAL;
+			if (dp->seq_mode == SNDRV_SEQ_OSS_MODE_MUSIC) {
+				err = -EINVAL;
+				break;
+			}
 			ev_size = SHORT_EVENT_SIZE;
 		}
 
 		/* insert queue */
-		if ((rc = insert_queue(dp, &rec, opt)) < 0)
+		if ((err = insert_queue(dp, &rec, opt)) < 0)
 			break;
 
-		c -= ev_size;
+		result += ev_size;
+		buf += ev_size;
+		count -= ev_size;
 	}
-
-	if (count == c && is_nonblock_mode(dp->file_mode))
-		return -EAGAIN;
-	return count - c;
+	return result > 0 ? result : err;
 }