[CHECKER][PATCH] cmpci user-pointer fix

Hollis Blanchard
2003-05-28T20:04:15Z

[-- Attachment #1: Type: text/plain, Size: 2659 bytes --]

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

[-- Attachment #2: cmpci-userptr.diff --]
[-- Type: application/octet-stream, Size: 1916 bytes --]

--- 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 {