[CHECKER][PATCH] cmpci user-pointer fix

[Hollis Blanchard]

--Apple-Mail-17-683902250
Content-Transfer-Encoding: 7bit
Content-Type: text/plain;
	charset=US-ASCII;
	format=flowed

Here's what the Stanford tool said:
---------------------------------------------------------
[BUG] at least bad programming practice. file_operations.write ->
cm_write -> trans_ac3. write can take tainted. write can take tainted
inputs. the pointer is vefied in cm_write

/home/junfeng/linux-2.5.63/sound/oss/cmpci.c:593:trans_ac3:
ERROR:TAINTED:593:593: dereferencing tainted ptr 'src' [Callstack:
/home/junfeng/linux-2.5.63/fs/read_write.c:307:vfs_write((tainted
1)(tainted 2)) ->
/home/junfeng/linux-2.5.63/fs/read_write.c:241:cm_write((tainted
1)(tainted 2)) ->
/home/junfeng/linux-2.5.63/sound/oss/cmpci.c:1662:trans_ac3((tainted
2))]

         unsigned long data;
         unsigned long *dst = (unsigned long *) dest;
         unsigned short *src = (unsigned short *)source;

         do {

Error --->
                 data = (unsigned long) *src++;
                 data <<= 12;                    // ok for 16-bit data
                 if (s->spdif_counter == 2 || s->spdif_counter == 3)
---------------------------------------------------------
[BUG] at least bad programming practice. file_operations.write ->
cm_write -> trans_ac3. write can take tainted. write can take tainted
inputs. the pointer is vefied in cm_write

/home/junfeng/linux-2.5.63/sound/oss/cmpci.c:593:trans_ac3:
ERROR:TAINTED:593:593: dereferencing tainted ptr 'src' [Callstack:
/home/junfeng/linux-2.5.63/fs/read_write.c:307:vfs_write((tainted
1)(tainted 2)) ->
/home/junfeng/linux-2.5.63/fs/read_write.c:241:cm_write((tainted
1)(tainted 2)) ->
/home/junfeng/linux-2.5.63/sound/oss/cmpci.c:1662:trans_ac3((tainted
2))]

         unsigned long data;
         unsigned long *dst = (unsigned long *) dest;
         unsigned short *src = (unsigned short *)source;

         do {

Error --->
                 data = (unsigned long) *src++;
                 data <<= 12;                    // ok for 16-bit data
                 if (s->spdif_counter == 2 || s->spdif_counter == 3)
                         data |= 0x40000000;     // indicate AC-3 raw 
data
---------------------------------------------------------

I believe the attached patch fixes it. cm_write was calling access_ok, 
but after that you must still access user space through the 
get/put/copy*_user functions. It should be safe to return -EFAULT at 
these points in cm_write, since there are other returns already in the 
code above and below that. Compile-tested only.

Junfeng, the checker seems to have missed the "*dst0++ = *src++;" bits 
at the bottom of the patch... or at least it wasn't in the mail I saw 
("4 potential user-pointer errors", 2 May 2003).

-- 
Hollis Blanchard
IBM Linux Technology Center

--Apple-Mail-17-683902250
Content-Disposition: attachment;
	filename=cmpci-userptr.diff
Content-Transfer-Encoding: 7bit
Content-Type: application/octet-stream;
	x-unix-mode=0644;
	name="cmpci-userptr.diff"

--- linux-2.5.70/sound/oss/cmpci.c.orig	Sat May 24 19:00:00 2003
+++ linux-2.5.70/sound/oss/cmpci.c	Wed May 28 14:53:15 2003
@@ -580,15 +580,17 @@
 	spin_unlock_irqrestore(&s->lock, flags);
 }
 
-static void trans_ac3(struct cm_state *s, void *dest, const char *source, int size)
+static int trans_ac3(struct cm_state *s, void *dest, const char *source, int size)
 {
 	int   i = size / 2;
+	int err;
 	unsigned long data;
 	unsigned long *dst = (unsigned long *) dest;
 	unsigned short *src = (unsigned short *)source;
 
 	do {
-		data = (unsigned long) *src++;
+		if ((err = __get_user(data, src++)))
+			return err;
 		data <<= 12;			// ok for 16-bit data
 		if (s->spdif_counter == 2 || s->spdif_counter == 3)
 			data |= 0x40000000;	// indicate AC-3 raw data
@@ -605,6 +607,8 @@
 		if (s->spdif_counter == 384)
 			s->spdif_counter = 0;
 	} while (--i);
+
+	return 0;
 }
 
 static void set_adc_rate_unlocked(struct cm_state *s, unsigned rate)
@@ -1655,13 +1659,16 @@
 			continue;
 		}
 		if (s->status & DO_AC3_SW) {
+			int err;
+
 			// clip exceeded data, caught by 033 and 037
 			if (swptr + 2 * cnt > s->dma_dac.dmasize)
 				cnt = (s->dma_dac.dmasize - swptr) / 2;
-			trans_ac3(s, s->dma_dac.rawbuf + swptr, buffer, cnt);
+			if ((err = trans_ac3(s, s->dma_dac.rawbuf + swptr, buffer, cnt)))
+				return err;
 			swptr = (swptr + 2 * cnt) % s->dma_dac.dmasize;
 		} else if (s->status & DO_DUAL_DAC) {
-			int	i;
+			int	i, err;
 			unsigned long *src, *dst0, *dst1;
 
 			src = (unsigned long *) buffer;
@@ -1669,8 +1676,10 @@
 			dst1 = (unsigned long *) (s->dma_adc.rawbuf + swptr);
 			// copy left/right sample at one time
 			for (i = 0; i <= cnt / 4; i++) {
-				*dst0++ = *src++;
-				*dst1++ = *src++;
+				if ((err = __get_user(*dst0++, src++)))
+					return err;
+				if ((err = __get_user(*dst1++, src++)))
+					return err;
 			}
 			swptr = (swptr + cnt) % s->dma_dac.dmasize;
 		} else {

--Apple-Mail-17-683902250--