Mailing List archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[linux-dvb] Re: Crashes in dvb_demux.c
On Sun, Feb 09, 2003 at 07:54:43PM +0100, Florian Schirmer wrote:
> Hi,
>
> >What if sec->seclen is already negative (-1) ? It can be -1 < count and
> >also -1 < 4096, but would lead to infinite memcopy
>
> sec->seclen is set as "3+((buf[1]&0x0f)<<8)+buf[2]". This can not result in
> a negative length.
>
> Bye,
> Florian
>
OK, you win.
There was well, ok, not a bug, but misleading line in
vpeirq that caused my test system deliver out-of-range
data in some cases when dmapos wasn't divisible by 188.
With this vpeirq-cosmetic fix (Holger please apply this
to dvb-kernel), it will accept any dmapos, no matter is
it divisible by 188 or not, it will run completely stable
and it won't have any performance penalty.
dvb_demux.c, un-obvious as it is has passed my die-hard
test (attached, but don't apply). Passing this test,
and suppose oops from rmmod with running szap is fixed,
the dvb-kernel stuff can go directly to linux 2.5.x kernel
as far as I am concerned). Of course, dvb-ttpci should be
marked as (DAMGEROUS) because of av7110.c
However, I have added extra checks to the dvb_demux.c
It adds extra safety checks before each memcopy but
eats few more CPU cycles.
Because I don't know PES structure exactly I'm not sure
should this be added to dvb-kernel or not.
Comments are welcome.
Best regards, Emard
diff -pur orig/dvb-kernel/linux/drivers/media/dvb/ttpci-budget/budget-core.c dvb-kernel/linux/drivers/media/dvb/ttpci-budget/budget-core.c
--- orig/dvb-kernel/linux/drivers/media/dvb/ttpci-budget/budget-core.c Fri Feb 7 20:57:15 2003
+++ dvb-kernel/linux/drivers/media/dvb/ttpci-budget/budget-core.c Sun Feb 9 23:53:47 2003
@@ -122,7 +122,7 @@ static void vpeirq (unsigned long data)
} else {
if (budget->feeding && mem[budget->ttbp]==0x47)
dvb_dmx_swfilter_packets(&budget->demux,
- mem+budget->ttbp, 1024-budget->ttbp/188);
+ mem+budget->ttbp, (TS_BUFLEN-budget->ttbp)/188);
num=dmapos/188;
}
diff -pur orig/dvb-kernel/linux/drivers/media/dvb/ttpci-budget/budget-core.c dvb-kernel/linux/drivers/media/dvb/ttpci-budget/budget-core.c
--- orig/dvb-kernel/linux/drivers/media/dvb/ttpci-budget/budget-core.c Fri Feb 7 20:57:15 2003
+++ dvb-kernel/linux/drivers/media/dvb/ttpci-budget/budget-core.c Sun Feb 9 23:45:43 2003
@@ -112,7 +112,12 @@ static void vpeirq (unsigned long data)
dmapos=saa7146_read(budget->dev, PCI_VDP3);
dmapos-=(dmapos%188);
-
+#if 1
+ /* hard stability test by EMARD */
+ if(dmapos < TS_BUFLEN-1000)
+ dmapos += mem[dmapos+71];
+ dmapos %= TS_BUFLEN;
+#endif
if (dmapos >= TS_BUFLEN)
return;
@@ -120,16 +125,16 @@ static void vpeirq (unsigned long data)
mem+=budget->ttbp;
num=(dmapos-budget->ttbp)/188;
} else {
- if (budget->feeding && mem[budget->ttbp]==0x47)
+ if (budget->feeding)
dvb_dmx_swfilter_packets(&budget->demux,
- mem+budget->ttbp, 1024-budget->ttbp/188);
+ mem+budget->ttbp, (TS_BUFLEN-budget->ttbp)/188);
num=dmapos/188;
}
budget->ttbp=dmapos;
- if (budget->feeding && mem[0]==0x47)
+ if (budget->feeding)
dvb_dmx_swfilter_packets(&budget->demux, mem, num);
}
Only in dvb-kernel/build-2.4: alps_bsrv2.c
Only in dvb-kernel/build-2.4: alps_tdlb7.c
Only in dvb-kernel/build-2.4: alps_tdmb7.c
Only in dvb-kernel/build-2.4: at76c651.c
Only in dvb-kernel/build-2.4: av7110.c
Only in dvb-kernel/build-2.4: av7110.h
Only in dvb-kernel/build-2.4: av7110_firm.h
Only in dvb-kernel/build-2.4: av7110_ipack.c
Only in dvb-kernel/build-2.4: av7110_ipack.h
Only in dvb-kernel/build-2.4: av7110_ir.c
Only in dvb-kernel/build-2.4: bt848.h
Only in dvb-kernel/build-2.4: bt878.c
Only in dvb-kernel/build-2.4: bt878.h
Only in dvb-kernel/build-2.4: budget-av.c
Only in dvb-kernel/build-2.4: budget-core.c
Only in dvb-kernel/build-2.4: budget.c
Only in dvb-kernel/build-2.4: budget.h
Only in dvb-kernel/build-2.4: compat.c
Only in dvb-kernel/build-2.4: compat.h
Only in dvb-kernel/build-2.4: demux.h
Only in dvb-kernel/build-2.4: dmxdev.c
Only in dvb-kernel/build-2.4: dmxdev.h
Only in dvb-kernel/build-2.4: driver.av7110
Only in dvb-kernel/build-2.4: dvb-bt8xx.c
Only in dvb-kernel/build-2.4: dvb-bt8xx.h
Only in dvb-kernel/build-2.4: dvb_demux.c
Only in dvb-kernel/build-2.4: dvb_demux.h
Only in dvb-kernel/build-2.4: dvb_filter.c
Only in dvb-kernel/build-2.4: dvb_filter.h
Only in dvb-kernel/build-2.4: dvb_frontend.c
Only in dvb-kernel/build-2.4: dvb_frontend.h
Only in dvb-kernel/build-2.4: dvb_i2c.c
Only in dvb-kernel/build-2.4: dvb_i2c.h
Only in dvb-kernel/build-2.4: dvb_ksyms.c
Only in dvb-kernel/build-2.4: dvb_net.c
Only in dvb-kernel/build-2.4: dvb_net.h
Only in dvb-kernel/build-2.4: dvbdev.c
Only in dvb-kernel/build-2.4: dvbdev.h
Only in dvb-kernel/build-2.4: grundig_29504-401.c
Only in dvb-kernel/build-2.4: grundig_29504-491.c
Only in dvb-kernel/build-2.4: nxt6000.c
Only in dvb-kernel/build-2.4: nxt6000.h
Only in dvb-kernel/build-2.4: saa7146.h
Only in dvb-kernel/build-2.4: saa7146_core.c
Only in dvb-kernel/build-2.4: saa7146_fops.c
Only in dvb-kernel/build-2.4: saa7146_hlp.c
Only in dvb-kernel/build-2.4: saa7146_i2c.c
Only in dvb-kernel/build-2.4: saa7146_vbi.c
Only in dvb-kernel/build-2.4: saa7146_video.c
Only in dvb-kernel/build-2.4: saa7146_vv.h
Only in dvb-kernel/build-2.4: stv0299.c
Only in dvb-kernel/build-2.4: ves1820.c
diff -pur /lib/dvb/orig/dvb-kernel/linux/drivers/media/dvb/dvb-core/dvb_demux.c dvb-kernel/linux/drivers/media/dvb/dvb-core/dvb_demux.c
--- /lib/dvb/orig/dvb-kernel/linux/drivers/media/dvb/dvb-core/dvb_demux.c Wed Jan 22 22:00:34 2003
+++ dvb-kernel/linux/drivers/media/dvb/dvb-core/dvb_demux.c Sun Feb 9 23:27:28 2003
@@ -152,9 +152,9 @@ u16 section_length(const u8 *buf)
static inline
-u16 ts_pid(const u8 *buf)
+u32 ts_pid(const u8 *buf)
{
- return ((buf[1]&0x1f)<<8)+buf[2];
+ return ((buf[1] & 0x1fL)<<8)+buf[2];
}
@@ -296,12 +296,13 @@ int dvb_dmx_swfilter_section_packet(stru
{
struct dvb_demux *demux = feed->demux;
dmx_section_feed_t *sec = &feed->feed.sec;
- int p, count;
- int ccok, rest;
- u8 cc;
+ u32 p, count, rest;
+ u8 ccok, cc;
if (!(count = payload(buf)))
return -1;
+ if(count > 188)
+ return -1;
p = 188-count;
@@ -322,52 +323,70 @@ int dvb_dmx_swfilter_section_packet(stru
if (p + tmp >= 187)
return -1;
- demux->memcopy (feed, sec->secbuf+sec->secbufp,
+ if(sec->secbufp + tmp <= 4096 && p+1+tmp <= 188)
+ demux->memcopy (feed, sec->secbuf+sec->secbufp,
buf+p+1, tmp);
+ else
+ return -1;
sec->seclen = section_length(sec->secbuf);
- if (sec->seclen > 4096)
+ if (sec->seclen > 4096 || sec->seclen < 0)
return -1;
}
rest = sec->seclen - sec->secbufp;
if (rest == buf[p] && sec->seclen) {
- demux->memcopy (feed, sec->secbuf + sec->secbufp,
+ if(sec->secbufp + buf[p] <= 4096 && p+1+buf[p] <= 188)
+ demux->memcopy (feed, sec->secbuf + sec->secbufp,
buf+p+1, buf[p]);
+ else
+ return -1;
sec->secbufp += buf[p];
dvb_dmx_swfilter_section_feed(feed);
}
}
p += buf[p] + 1; // skip rest of last section
+ if(p > 188)
+ return -1;
count = 188 - p;
- while (count) {
+ while (count > 0) {
sec->crc_val = ~0;
if ((count>2) && // enough data to determine sec length?
((sec->seclen = section_length(buf+p)) <= count)) {
- if (sec->seclen>4096)
+ if (sec->seclen>4096 || sec->seclen < 0)
return -1;
- demux->memcopy (feed, sec->secbuf, buf+p,
- sec->seclen);
+ // this memcopy might be wrong
+ if(p+sec->seclen <= 188)
+ demux->memcopy (feed, sec->secbuf,
+ buf+p, sec->seclen);
+ else
+ return -1;
sec->secbufp = sec->seclen;
p += sec->seclen;
+ if(p > 188)
+ return -1;
count = 188 - p;
dvb_dmx_swfilter_section_feed(feed);
// filling bytes until packet end?
- if (count && buf[p]==0xff)
+ if (count != 0 && buf[p]==0xff)
count=0;
} else { // section continues to following TS packet
- demux->memcopy(feed, sec->secbuf, buf+p, count);
+ if(p + count <= 188)
+ demux->memcopy(feed, sec->secbuf,
+ buf+p, count);
+ else
+ return -1;
sec->secbufp+=count;
count=0;
}
@@ -384,7 +403,7 @@ int dvb_dmx_swfilter_section_packet(stru
return -1;
// did we have enough data in last packet to calc section length?
- if (sec->secbufp < 3) {
+ if (sec->secbufp < 3 && sec->secbufp >= 0) {
int tmp = 3 - sec->secbufp;
if (tmp>count)
@@ -392,25 +411,40 @@ int dvb_dmx_swfilter_section_packet(stru
sec->crc_val = ~0;
- demux->memcopy (feed, sec->secbuf + sec->secbufp, buf+p, tmp);
+ if(sec->secbufp + tmp <= 4096 && p+tmp <= 188)
+ demux->memcopy (feed, sec->secbuf + sec->secbufp,
+ buf+p, tmp);
+ else
+ return -1;
sec->seclen = section_length(sec->secbuf);
- if (sec->seclen > 4096)
+ if (sec->seclen > 4096 || sec->seclen < 0)
return -1;
}
rest = sec->seclen - sec->secbufp;
- if (rest < 0)
+ if (rest < 0 || rest > 4096)
+ return -1;
+
+ if(count < 0 || count > 188)
return -1;
if (rest <= count) { // section completed in this TS packet
- demux->memcopy (feed, sec->secbuf + sec->secbufp, buf+p, rest);
+ if(sec->secbufp + rest <= 4096 && p+rest <= 188)
+ demux->memcopy (feed, sec->secbuf + sec->secbufp,
+ buf+p, rest);
+ else
+ return -1;
sec->secbufp += rest;
dvb_dmx_swfilter_section_feed(feed);
} else { // section continues in following ts packet
- demux->memcopy (feed, sec->secbuf + sec->secbufp, buf+p, count);
+ if(sec->secbufp + count <= 4096 && p+count <= 188)
+ demux->memcopy (feed, sec->secbuf + sec->secbufp,
+ buf+p, count);
+ else
+ return -1;
sec->secbufp += count;
}
Home |
Main Index |
Thread Index