Skip to contentSkip to conversation

1/3: BUG FIX in dvb_ringbuffer_flush

PATCH In review Andrea 2 participants 2 messages Mar 22, 2008 Media (V4L/DVB)

lore
Share & tools
lore

Andrea

author
#0 lore raw

PATCH 1/1 1/3: BUG FIX in dvb_ringbuffer_flush

I've been playing with DMX_SET_BUFFER_SIZE and I've had a problem when I tried to resize the buffer to a smaller size.

dvb_dmxdev_set_buffer_size flushes the ringbuffer and then replaces it with a new one. What happens if the current pointer is on a position that would be invalid in the new buffer? An access violation.

This because dvb_ringbuffer_flush resets the 2 pointers to the vaule of pwrite (which could be after the end of the new buffer).
I think it is safer to reset them to 0.

Andrea

linux/drivers/media/dvb/dvb-core/dvb_ringbuffer.c +1 -1
diff -r 1886a5ea2f84 linux/drivers/media/dvb/dvb-core/dvb_ringbuffer.c
@@ -86,7 +86,7 @@ ssize_t dvb_ringbuffer_avail(struct dvb_
 
 void dvb_ringbuffer_flush(struct dvb_ringbuffer *rbuf)
 {
-	rbuf->pread = rbuf->pwrite;
+	rbuf->pread = rbuf->pwrite = 0;
 	rbuf->error = 0;
 }
 

_______________________________________________
linux-dvb mailing list
[email protected]
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb

#1 lore raw
Quoted 9 lines · Andrea

I've been playing with DMX_SET_BUFFER_SIZE and I've had a problem when I tried to resize the buffer to a smaller size.

dvb_dmxdev_set_buffer_size flushes the ringbuffer and then replaces it with a new one. What happens if the current pointer is on a position that would be invalid in the new buffer? An access violation.

This because dvb_ringbuffer_flush resets the 2 pointers to the vaule of pwrite (which could be after the end of the new buffer).

Ack, this _is_ a bug. :-(

Quoted 1 line

I think it is safer to reset them to 0.

Nak. At the first glance one might think that this patch is correct. Unfortunately, it introduces a subtle bug.

Citing the comments in dvb_ringbuffer.h:

| (2) If there is exactly one reader and one writer, there is no need
|     to lock read or write operations.
|     Two or more readers must be locked against each other.
|     Flushing the buffer counts as a read operation.
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

| Two or more writers must be locked against each other.

With your patch all dvb_ringbuffer_flush() calls must also be considered as write operations. As a consequence, you would have to add spinlock protection at many places in the code...

So I suggest to leave dvb_ringbuffer_flush() as is and zero the read and write pointers only where it is really required...

CU
Oliver

Signature · 8 lines
----------------------------------------------------------------
VDR Remote Plugin 0.4.0: http://www.escape-edv.de/endriss/vdr/
----------------------------------------------------------------

_______________________________________________
linux-dvb mailing list
[email protected]
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb