On Fri, 2008-05-02 at 22:43 -0400, Andy Walls wrote:
> Hans,
>
> When investigating Mike Krufky's report of module reload problems, I ran
> across problems with the management of the cx18_cards[] array. They're
> corner cases and not likely to be the cause of Mike problems though.
> The attached patch was made against the latest v4l-dvb hg repository.
This is a new version of the patch. The previous version would change
card minor number ordering upon errors, which is not what users with a
number of cards and multiple video sources wired up would want to
happen.
This new patch is almost a minimal set of changes to fix the
cx18_cards[] leak and possible bad pointers being left in cx18_cards[]
on error. It does include some additional lines to obtain the
cx18_cards_lock when accessing cx18_cards[] and not just
cx18_cards_active.
Regards,
Andy
# HG changeset patch
# User Andy Walls <[EMAIL PROTECTED]>
# Date 1209820624 14400
# Node ID 2f883fedfb85c1979d9352ffdcf97a2d901dfbeb
# Parent 4c4fd6b8755cc9918255876ff1010bc77374a310
Prevent cx18_cards[] leak and possible bad pointer dereference
From: Andy Walls <[EMAIL PROTECTED]>
The code at label 'err:' in cx18_probe() would try and free the wrong
cx18_cards[] entry on error exit, and leave a bad pointer in place.
cx18_v4l2_open() could have derefernced this bad pointer or NULL pointer
after the fix, so fixed that as well.
Obtained spin lock in all places where cx18_cards[] is accessed, not just where
cx18_cards_active is accessed.
Signed-off-by: Andy Walls <[EMAIL PROTECTED]>
diff -r 4c4fd6b8755c -r 2f883fedfb85 linux/drivers/media/video/cx18/cx18-driver.c
--- a/linux/drivers/media/video/cx18/cx18-driver.c Fri May 02 07:51:27 2008 -0300
+++ b/linux/drivers/media/video/cx18/cx18-driver.c Sat May 03 09:17:04 2008 -0400
@@ -598,6 +598,7 @@ static int __devinit cx18_probe(struct p
const struct pci_device_id *pci_id)
{
int retval = 0;
+ int i;
int vbi_buf_size;
u32 devtype;
struct cx18 *cx;
@@ -816,8 +817,11 @@ err:
retval = -ENODEV;
CX18_ERR("Error %d on initialization\n", retval);
- kfree(cx18_cards[cx18_cards_active]);
- cx18_cards[cx18_cards_active] = NULL;
+ spin_lock(&cx18_cards_lock);
+ i = cx->num;
+ kfree(cx18_cards[i]);
+ cx18_cards[i] = NULL;
+ spin_unlock(&cx18_cards_lock);
return retval;
}
@@ -960,11 +964,14 @@ static void module_cleanup(void)
pci_unregister_driver(&cx18_pci_driver);
+ spin_lock(&cx18_cards_lock);
for (i = 0; i < cx18_cards_active; i++) {
if (cx18_cards[i] == NULL)
continue;
kfree(cx18_cards[i]);
- }
+ cx18_cards[i] = NULL;
+ }
+ spin_unlock(&cx18_cards_lock);
}
module_init(module_start);
diff -r 4c4fd6b8755c -r 2f883fedfb85 linux/drivers/media/video/cx18/cx18-fileops.c
--- a/linux/drivers/media/video/cx18/cx18-fileops.c Fri May 02 07:51:27 2008 -0300
+++ b/linux/drivers/media/video/cx18/cx18-fileops.c Sat May 03 09:17:04 2008 -0400
@@ -695,6 +695,8 @@ int cx18_v4l2_open(struct inode *inode,
/* Find which card this open was on */
spin_lock(&cx18_cards_lock);
for (x = 0; cx == NULL && x < cx18_cards_active; x++) {
+ if (cx18_cards[x] == NULL)
+ continue;
/* find out which stream this open was on */
for (y = 0; y < CX18_MAX_STREAMS; y++) {
s = &cx18_cards[x]->streams[y];
_______________________________________________
ivtv-devel mailing list
[email protected]
http://ivtvdriver.org/mailman/listinfo/ivtv-devel
