[linux-dvb] DVB driver bug, fixed it (quick hack)

["C.Y.M" <syphir@syphir.sytes.net>]

Jan Panteltje wrote:
> I found a problem with the DVB driber (any one from
> 2000 up).
> The problem showed itself when tuning a signal, and
> then 
> doing
> cat /dev/dvb/adapter0/dvr0 > filename.ts
>
> After a while, especially with high system load on a
> slow machine,
> the 'cat' aborts with 'Value to large for defined data
> type'
> Looking it up in errno.h shows this to be a buffer
> overflow.
> Looking in the driver source I found the problem came
> from 'dmxdev.c'.
>
> I put some printk statements in it, and ran some
> tests.
>
> The cause is in the typing., it is wrong all over
> the place in that driver ....
>
> In the original (uncorrected) driver only HALF the 32
> bit
> buffer is used, and if more data enters, the number
> flips
> to negative with as a result the 'Value to large for
> defined data type error'
> error abort (and a possible crash of the application
> that uses the driver).
> Even cat /dev/dvb/adapter0/dvr0 > file.ts crashed!
>
> Here are the 2 routines I changed so it works.
>  
> convergence.de uses 32 bit buffer......
> this returned int, changed to long!   
> static inline long
> dvb_dmxdev_buffer_write(dmxdev_buffer_t *buf, uint8_t
> *src, size_t len)
> {
> unsigned int split; // changed was int
> unsigned int free; // changed, was int
> unsigned int todo; // changed, was int
>
> if(! len)  return 0;
>  
> if(! buf->data) return 0;
>
> free = buf->pread - buf->pwrite;
>  
> split = 0;
> if(free == 0)
> {
> free += buf->size;
> split = buf->size - buf->pwrite;
> }
>  
> if(len >= free)
> {
> // added kernel reporting
> printk("dmxdev: buffer overflow len=%lu free=%ld\n",
> len, free);
>
> return -1;
> }
>  
> if (split >= len) split = 0;
> todo = len;
> if(split)  
> {
> memcpy(buf->data + buf->pwrite, src, split);
> todo -= split;
> buf->pwrite = 0;
> }
>  
> memcpy(buf -> data + buf -> pwrite, src + split,
> todo);
>
> buf -> pwrite = (buf -> pwrite + todo) % buf->size;
>
>
> return len; // at least now if it returns it can
> return the full length of
>             // a 32 bit buffer as a POSTIVE
> number!!!!.
> }
>  
>  
> static int dvb_dmxdev_ts_callback(\
> u8 *buffer1, size_t buffer1_len, u8 *buffer2, size_t
> buffer2_len, dmx_ts_feed_t *feed,\
> dmx_success_t success)
> {
> dmxdev_filter_t *dmxdevfilter = (dmxdev_filter_t *)
> feed->priv;
>
> dmxdev_buffer_t *buffer;
> long int ret; // changed was int
>
> spin_lock(&dmxdevfilter -> dev -> lock);
>
> if(dmxdevfilter->params.pes.output == DMX_OUT_DECODER)
> {
> spin_unlock(&dmxdevfilter -> dev -> lock);
> return 0;
> }
>  
> if(dmxdevfilter -> params.pes.output == DMX_OUT_TAP)
> buffer =& dmxdevfilter -> buffer;
> else buffer =& dmxdevfilter -> dev -> dvr_buffer;
>
> if(buffer -> error)
> {
> spin_unlock(&dmxdevfilter -> dev -> lock);
> wake_up(&buffer -> queue);
> return 0;
> }
>  
> ret = dvb_dmxdev_buffer_write(buffer, buffer1,
> buffer1_len);
>
> if(ret == buffer1_len)  ret =
> dvb_dmxdev_buffer_write(buffer, buffer2, buffer2_len);
>
> if(ret < 0)
> {
> buffer->pwrite = buffer -> pread;
> buffer->error =- EOVERFLOW;
>
> // added kernel reporting
> printk("WAS dvb_dmxdev_ts_callback(): EOVERFLOW
> ret=%d\n", ret);
> }
>  
> spin_unlock(&dmxdevfilter -> dev->lock);
> wake_up(&buffer -> queue);
> return 0;
> }
>  
>  
> So, the gist is guys, IF you use the DVB driver (any
> one since 2000),
> and you get strange errors / segfaults, aborts, when
> reading from the
> dvr device, then make this fix perhaps.
> But all the rest is crummy too, I will subcribe to
> their mailinglist
> and post this and then run ;-)
> I will also post to the VDR mailing list, as it
> explains many problems
> there ....
Thank you, Jan.  I took the liberty of converting your post to a unified diff. 
I hope I did not miss anything.

Best Regards,
C.Y.M.

--- dvb-kernel/linux/drivers/media/dvb/dvb-core/dmxdev.c.orig	2005-02-02 13:40:07.000000000 -0800
+++ dvb-kernel/linux/drivers/media/dvb/dvb-core/dmxdev.c	2005-02-02 14:04:57.000000000 -0800
@@ -58,11 +58,11 @@
 	init_waitqueue_head(&buffer->queue);
 }
 
-static inline int dvb_dmxdev_buffer_write(struct dmxdev_buffer *buf, const u8 *src, int len) 
+static inline long dvb_dmxdev_buffer_write(dmxdev_buffer_t *buf, uint8_t *src, size_t len)
 {
-	int split;
-	int free;
-	int todo;
+	unsigned int split;
+	unsigned int free;
+	unsigned int todo;
 
 	if (!len)
 		return 0;
@@ -71,12 +71,12 @@
 	
 	free=buf->pread-buf->pwrite;
 	split=0;
-	if (free<=0) {
+	if (free==0) {
 		free+=buf->size;
 		split=buf->size-buf->pwrite;
 	}
 	if (len>=free) {
-		dprintk("dmxdev: buffer overflow\n");
+		printk("dmxdev: buffer overflow len=%lu free=%ld\n", len, free);
 		return -1;
 	}
 	if (split>=len)
@@ -378,13 +378,13 @@
 	return 0;
 }
 
-static int dvb_dmxdev_ts_callback(const u8 *buffer1, size_t buffer1_len,
-		       const u8 *buffer2, size_t buffer2_len,
-		       struct dmx_ts_feed *feed, enum dmx_success success)
-{
-	struct dmxdev_filter *dmxdevfilter=(struct dmxdev_filter *) feed->priv;
-	struct dmxdev_buffer *buffer;
-	int ret;
+static int dvb_dmxdev_ts_callback(u8 *buffer1, size_t buffer1_len,
+		       u8 *buffer2, size_t buffer2_len,
+		       dmx_ts_feed *feed, dmx_success success)
+{
+	dmxdev_filter *dmxdevfilter=(dmxdev_filter *) feed->priv;
+	dmxdev_buffer *buffer;
+	long int ret;
 	
 	spin_lock(&dmxdevfilter->dev->lock);
 	if (dmxdevfilter->params.pes.output==DMX_OUT_DECODER) {
@@ -407,6 +407,7 @@
 	if (ret<0) {
 		buffer->pwrite=buffer->pread;    
 		buffer->error=-EOVERFLOW;
+		printk("WAS dvb_dmxdev_ts_callback(): EOVERFLOW ret=%d\n", ret);
 	}
 	spin_unlock(&dmxdevfilter->dev->lock);
 	wake_up(&buffer->queue);
--- dvb-kernel/linux/drivers/media/dvb/dvb-core/dmxdev.h.orig	2005-02-02 12:56:38.000000000 -0800
+++ dvb-kernel/linux/drivers/media/dvb/dvb-core/dmxdev.h	2005-02-02 12:51:39.000000000 -0800
@@ -115,7 +115,7 @@
         struct dmx_frontend *dvr_orig_fe;
 
         struct dmxdev_buffer dvr_buffer;
-#define DVR_BUFFER_SIZE (10*188*1024)
+#define DVR_BUFFER_SIZE (100*188*1024)
 
 	struct semaphore mutex;
 	spinlock_t lock;