linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/6] drivers/media/dvb-core/en50221: move code to dvb_ca_private_free()
@ 2016-03-21 13:30 Max Kellermann
  2016-03-21 13:30 ` [PATCH 2/6] drivers/media/dvb-core/en50221: postpone release until file is closed Max Kellermann
                   ` (5 more replies)
  0 siblings, 6 replies; 11+ messages in thread
From: Max Kellermann @ 2016-03-21 13:30 UTC (permalink / raw)
  To: linux-kernel, linux-media

Prepare for postponing the call until all file handles have been
closed.

Signed-off-by: Max Kellermann <max@duempel.org>
---
 drivers/media/dvb-core/dvb_ca_en50221.c |   16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/media/dvb-core/dvb_ca_en50221.c b/drivers/media/dvb-core/dvb_ca_en50221.c
index f82cd1f..e33364c 100644
--- a/drivers/media/dvb-core/dvb_ca_en50221.c
+++ b/drivers/media/dvb-core/dvb_ca_en50221.c
@@ -161,6 +161,17 @@ struct dvb_ca_private {
 	struct mutex ioctl_mutex;
 };
 
+static void dvb_ca_private_free(struct dvb_ca_private *ca)
+{
+	dvb_unregister_device(ca->dvbdev);
+	unsigned int i;
+	for (i = 0; i < ca->slot_count; i++) {
+		vfree(ca->slot_info[i].rx_buffer.data);
+	}
+	kfree(ca->slot_info);
+	kfree(ca);
+}
+
 static void dvb_ca_en50221_thread_wakeup(struct dvb_ca_private *ca);
 static int dvb_ca_en50221_read_data(struct dvb_ca_private *ca, int slot, u8 * ebuf, int ecount);
 static int dvb_ca_en50221_write_data(struct dvb_ca_private *ca, int slot, u8 * ebuf, int ecount);
@@ -1759,10 +1770,7 @@ void dvb_ca_en50221_release(struct dvb_ca_en50221 *pubca)
 
 	for (i = 0; i < ca->slot_count; i++) {
 		dvb_ca_en50221_slot_shutdown(ca, i);
-		vfree(ca->slot_info[i].rx_buffer.data);
 	}
-	kfree(ca->slot_info);
-	dvb_unregister_device(ca->dvbdev);
-	kfree(ca);
+	dvb_ca_private_free(ca);
 	pubca->private = NULL;
 }

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/6] drivers/media/dvb-core/en50221: postpone release until file is closed
  2016-03-21 13:30 [PATCH 1/6] drivers/media/dvb-core/en50221: move code to dvb_ca_private_free() Max Kellermann
@ 2016-03-21 13:30 ` Max Kellermann
  2016-06-09  9:38   ` Mauro Carvalho Chehab
  2016-03-21 13:30 ` [PATCH 3/6] drivers/media/media-devnode: clear private_data before put_device() Max Kellermann
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: Max Kellermann @ 2016-03-21 13:30 UTC (permalink / raw)
  To: linux-kernel, linux-media

Fixes use-after-free bug which occurs when I disconnect my DVB-S
received while VDR is running.

Signed-off-by: Max Kellermann <max@duempel.org>
---
 drivers/media/dvb-core/dvb_ca_en50221.c |   23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/media/dvb-core/dvb_ca_en50221.c b/drivers/media/dvb-core/dvb_ca_en50221.c
index e33364c..dfc686a 100644
--- a/drivers/media/dvb-core/dvb_ca_en50221.c
+++ b/drivers/media/dvb-core/dvb_ca_en50221.c
@@ -148,6 +148,9 @@ struct dvb_ca_private {
 	/* Flag indicating if the CA device is open */
 	unsigned int open:1;
 
+	/* Flag indicating if the CA device is released */
+	unsigned int released:1;
+
 	/* Flag indicating the thread should wake up now */
 	unsigned int wakeup:1;
 
@@ -1392,6 +1395,11 @@ static int dvb_ca_en50221_io_read_condition(struct dvb_ca_private *ca,
 	int found = 0;
 	u8 hdr[2];
 
+	if (ca->released) {
+		*result = -ENODEV;
+		return 1;
+	}
+
 	slot = ca->next_read_slot;
 	while ((slot_count < ca->slot_count) && (!found)) {
 		if (ca->slot_info[slot].slot_state != DVB_CA_SLOTSTATE_RUNNING)
@@ -1595,6 +1603,9 @@ static int dvb_ca_en50221_io_release(struct inode *inode, struct file *file)
 
 	err = dvb_generic_release(inode, file);
 
+	if (ca->released)
+		dvb_ca_private_free(ca);
+
 	module_put(ca->pub->owner);
 
 	return err;
@@ -1701,6 +1712,7 @@ int dvb_ca_en50221_init(struct dvb_adapter *dvb_adapter,
 	}
 	init_waitqueue_head(&ca->wait_queue);
 	ca->open = 0;
+	ca->released = 0;
 	ca->wakeup = 0;
 	ca->next_read_slot = 0;
 	pubca->private = ca;
@@ -1765,12 +1777,21 @@ void dvb_ca_en50221_release(struct dvb_ca_en50221 *pubca)
 
 	dprintk("%s\n", __func__);
 
+	BUG_ON(ca->released);
+
 	/* shutdown the thread if there was one */
 	kthread_stop(ca->thread);
 
 	for (i = 0; i < ca->slot_count; i++) {
 		dvb_ca_en50221_slot_shutdown(ca, i);
 	}
-	dvb_ca_private_free(ca);
+
+	if (ca->open) {
+		ca->released = 1;
+		mb();
+		wake_up_interruptible(&ca->wait_queue);
+	} else
+		dvb_ca_private_free(ca);
+
 	pubca->private = NULL;
 }

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/6] drivers/media/media-devnode: clear private_data before put_device()
  2016-03-21 13:30 [PATCH 1/6] drivers/media/dvb-core/en50221: move code to dvb_ca_private_free() Max Kellermann
  2016-03-21 13:30 ` [PATCH 2/6] drivers/media/dvb-core/en50221: postpone release until file is closed Max Kellermann
@ 2016-03-21 13:30 ` Max Kellermann
  2016-03-21 13:30 ` [PATCH 4/6] drivers/media/media-device: move debug log before _devnode_unregister() Max Kellermann
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Max Kellermann @ 2016-03-21 13:30 UTC (permalink / raw)
  To: linux-kernel, linux-media

Callbacks invoked from put_device() may free the struct media_devnode
pointer, so any cleanup needs to be done before put_device().

Signed-off-by: Max Kellermann <max@duempel.org>
---
 drivers/media/media-devnode.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/media/media-devnode.c b/drivers/media/media-devnode.c
index 4d7e8dd..e263816 100644
--- a/drivers/media/media-devnode.c
+++ b/drivers/media/media-devnode.c
@@ -196,10 +196,11 @@ static int media_release(struct inode *inode, struct file *filp)
 	if (mdev->fops->release)
 		mdev->fops->release(filp);
 
+	filp->private_data = NULL;
+
 	/* decrease the refcount unconditionally since the release()
 	   return value is ignored. */
 	put_device(&mdev->dev);
-	filp->private_data = NULL;
 	return 0;
 }
 

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 4/6] drivers/media/media-device: move debug log before _devnode_unregister()
  2016-03-21 13:30 [PATCH 1/6] drivers/media/dvb-core/en50221: move code to dvb_ca_private_free() Max Kellermann
  2016-03-21 13:30 ` [PATCH 2/6] drivers/media/dvb-core/en50221: postpone release until file is closed Max Kellermann
  2016-03-21 13:30 ` [PATCH 3/6] drivers/media/media-devnode: clear private_data before put_device() Max Kellermann
@ 2016-03-21 13:30 ` Max Kellermann
  2016-03-21 13:30 ` [PATCH 5/6] drivers/media/media-device: add "release" callback Max Kellermann
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Max Kellermann @ 2016-03-21 13:30 UTC (permalink / raw)
  To: linux-kernel, linux-media

After media_devnode_unregister(), the struct media_device may be freed
already, and dereferencing it may crash.

Signed-off-by: Max Kellermann <max@duempel.org>
---
 drivers/media/media-device.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index e9219f5..5c4669c 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -745,9 +745,8 @@ void media_device_unregister(struct media_device *mdev)
 	spin_unlock(&mdev->lock);
 
 	device_remove_file(&mdev->devnode.dev, &dev_attr_model);
+	dev_dbg(mdev->dev, "Media device unregistering\n");
 	media_devnode_unregister(&mdev->devnode);
-
-	dev_dbg(mdev->dev, "Media device unregistered\n");
 }
 EXPORT_SYMBOL_GPL(media_device_unregister);
 

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 5/6] drivers/media/media-device: add "release" callback
  2016-03-21 13:30 [PATCH 1/6] drivers/media/dvb-core/en50221: move code to dvb_ca_private_free() Max Kellermann
                   ` (2 preceding siblings ...)
  2016-03-21 13:30 ` [PATCH 4/6] drivers/media/media-device: move debug log before _devnode_unregister() Max Kellermann
@ 2016-03-21 13:30 ` Max Kellermann
  2016-06-09 11:56   ` Mauro Carvalho Chehab
  2016-03-21 13:30 ` [PATCH 6/6] drivers/media/dvb-usb-dvb: postpone kfree(mdev) Max Kellermann
  2016-03-21 13:55 ` [PATCH 1/6] drivers/media/dvb-core/en50221: move code to dvb_ca_private_free() kbuild test robot
  5 siblings, 1 reply; 11+ messages in thread
From: Max Kellermann @ 2016-03-21 13:30 UTC (permalink / raw)
  To: linux-kernel, linux-media

Allow the client to free its data structures only after all files have
been closed (fixing use-after-free bugs).

Signed-off-by: Max Kellermann <max@duempel.org>
---
 drivers/media/media-device.c |    9 +++++++--
 include/media/media-device.h |    2 ++
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index 5c4669c..a3901f9 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -551,9 +551,14 @@ static DEVICE_ATTR(model, S_IRUGO, show_model, NULL);
  * Registration/unregistration
  */
 
-static void media_device_release(struct media_devnode *mdev)
+static void media_device_release(struct media_devnode *devnode)
 {
-	dev_dbg(mdev->parent, "Media device released\n");
+	struct media_device *mdev = to_media_device(devnode);
+
+	dev_dbg(devnode->parent, "Media device released\n");
+
+	if (mdev->release)
+		mdev->release(mdev);
 }
 
 /**
diff --git a/include/media/media-device.h b/include/media/media-device.h
index d385589..d184d0c 100644
--- a/include/media/media-device.h
+++ b/include/media/media-device.h
@@ -326,6 +326,8 @@ struct media_device {
 
 	int (*link_notify)(struct media_link *link, u32 flags,
 			   unsigned int notification);
+
+	void (*release)(struct media_device *mdev);
 };
 
 #ifdef CONFIG_MEDIA_CONTROLLER

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 6/6] drivers/media/dvb-usb-dvb: postpone kfree(mdev)
  2016-03-21 13:30 [PATCH 1/6] drivers/media/dvb-core/en50221: move code to dvb_ca_private_free() Max Kellermann
                   ` (3 preceding siblings ...)
  2016-03-21 13:30 ` [PATCH 5/6] drivers/media/media-device: add "release" callback Max Kellermann
@ 2016-03-21 13:30 ` Max Kellermann
  2016-03-21 13:45   ` kbuild test robot
  2016-03-21 13:56   ` kbuild test robot
  2016-03-21 13:55 ` [PATCH 1/6] drivers/media/dvb-core/en50221: move code to dvb_ca_private_free() kbuild test robot
  5 siblings, 2 replies; 11+ messages in thread
From: Max Kellermann @ 2016-03-21 13:30 UTC (permalink / raw)
  To: linux-kernel, linux-media

Fixes use-after-free bug which occurs when I disconnect my DVB-S
received while VDR is running.

Signed-off-by: Max Kellermann <max@duempel.org>
---
 drivers/media/usb/dvb-usb/dvb-usb-dvb.c |   14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/media/usb/dvb-usb/dvb-usb-dvb.c b/drivers/media/usb/dvb-usb/dvb-usb-dvb.c
index 9ddfcab..7859479 100644
--- a/drivers/media/usb/dvb-usb/dvb-usb-dvb.c
+++ b/drivers/media/usb/dvb-usb/dvb-usb-dvb.c
@@ -95,6 +95,12 @@ static int dvb_usb_stop_feed(struct dvb_demux_feed *dvbdmxfeed)
 	return dvb_usb_ctrl_feed(dvbdmxfeed, 0);
 }
 
+static void dvb_usb_media_device_release(struct media_device *mdev)
+{
+	media_device_cleanup(mdev);
+	kfree(mdev);
+}
+
 static int dvb_usb_media_device_init(struct dvb_usb_adapter *adap)
 {
 #ifdef CONFIG_MEDIA_CONTROLLER_DVB
@@ -113,6 +119,7 @@ static int dvb_usb_media_device_init(struct dvb_usb_adapter *adap)
 	strcpy(mdev->bus_info, udev->devpath);
 	mdev->hw_revision = le16_to_cpu(udev->descriptor.bcdDevice);
 	mdev->driver_version = LINUX_VERSION_CODE;
+	mdev->release = dvb_usb_media_device_release;
 
 	media_device_init(mdev);
 
@@ -138,10 +145,11 @@ static void dvb_usb_media_device_unregister(struct dvb_usb_adapter *adap)
 	if (!adap->dvb_adap.mdev)
 		return;
 
-	media_device_unregister(adap->dvb_adap.mdev);
-	media_device_cleanup(adap->dvb_adap.mdev);
-	kfree(adap->dvb_adap.mdev);
+	struct media_device *mdev = adap->dvb_adap.mdev;
 	adap->dvb_adap.mdev = NULL;
+	media_device_unregister(mdev);
+	/* media_device_cleanup() and kfree() will be called by the
+	   callback function dvb_usb_media_device_release() */
 #endif
 }
 

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 6/6] drivers/media/dvb-usb-dvb: postpone kfree(mdev)
  2016-03-21 13:30 ` [PATCH 6/6] drivers/media/dvb-usb-dvb: postpone kfree(mdev) Max Kellermann
@ 2016-03-21 13:45   ` kbuild test robot
  2016-03-21 13:56   ` kbuild test robot
  1 sibling, 0 replies; 11+ messages in thread
From: kbuild test robot @ 2016-03-21 13:45 UTC (permalink / raw)
  To: Max Kellermann; +Cc: kbuild-all, linux-kernel, linux-media

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

Hi Max,

[auto build test ERROR on v4.5-rc7]
[cannot apply to sailus-media/master linuxtv-media/master next-20160321]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Max-Kellermann/drivers-media-dvb-core-en50221-move-code-to-dvb_ca_private_free/20160321-213556
config: x86_64-randconfig-x000-201612 (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/media/usb/dvb-usb/dvb-usb-dvb.c: In function 'dvb_usb_media_device_release':
>> drivers/media/usb/dvb-usb/dvb-usb-dvb.c:100:2: error: implicit declaration of function 'media_device_cleanup' [-Werror=implicit-function-declaration]
     media_device_cleanup(mdev);
     ^
   drivers/media/usb/dvb-usb/dvb-usb-dvb.c: At top level:
   drivers/media/usb/dvb-usb/dvb-usb-dvb.c:98:13: warning: 'dvb_usb_media_device_release' defined but not used [-Wunused-function]
    static void dvb_usb_media_device_release(struct media_device *mdev)
                ^
   In file included from include/uapi/linux/stddef.h:1:0,
                    from include/linux/stddef.h:4,
                    from include/uapi/linux/posix_types.h:4,
                    from include/uapi/linux/types.h:13,
                    from include/linux/types.h:5,
                    from include/uapi/linux/sysinfo.h:4,
                    from include/uapi/linux/kernel.h:4,
                    from include/linux/cache.h:4,
                    from include/linux/time.h:4,
                    from include/linux/input.h:11,
                    from drivers/media/usb/dvb-usb/dvb-usb.h:13,
                    from drivers/media/usb/dvb-usb/dvb-usb-common.h:12,
                    from drivers/media/usb/dvb-usb/dvb-usb-dvb.c:9:
   drivers/media/usb/dvb-usb/dvb-usb-dvb.c: In function 'dvb_usb_adapter_frontend_init':
   include/linux/compiler.h:147:6: warning: 'ret' may be used uninitialized in this function [-Wmaybe-uninitialized]
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
         ^
   drivers/media/usb/dvb-usb/dvb-usb-dvb.c:289:6: note: 'ret' was declared here
     int ret, i;
         ^
   cc1: some warnings being treated as errors

vim +/media_device_cleanup +100 drivers/media/usb/dvb-usb/dvb-usb-dvb.c

    94		deb_ts("stop pid: 0x%04x, feedtype: %d\n", dvbdmxfeed->pid, dvbdmxfeed->type);
    95		return dvb_usb_ctrl_feed(dvbdmxfeed, 0);
    96	}
    97	
    98	static void dvb_usb_media_device_release(struct media_device *mdev)
    99	{
 > 100		media_device_cleanup(mdev);
   101		kfree(mdev);
   102	}
   103	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 33903 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/6] drivers/media/dvb-core/en50221: move code to dvb_ca_private_free()
  2016-03-21 13:30 [PATCH 1/6] drivers/media/dvb-core/en50221: move code to dvb_ca_private_free() Max Kellermann
                   ` (4 preceding siblings ...)
  2016-03-21 13:30 ` [PATCH 6/6] drivers/media/dvb-usb-dvb: postpone kfree(mdev) Max Kellermann
@ 2016-03-21 13:55 ` kbuild test robot
  5 siblings, 0 replies; 11+ messages in thread
From: kbuild test robot @ 2016-03-21 13:55 UTC (permalink / raw)
  To: Max Kellermann; +Cc: kbuild-all, linux-kernel, linux-media

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

Hi Max,

[auto build test WARNING on v4.5-rc7]
[also build test WARNING on next-20160321]
[cannot apply to sailus-media/master linuxtv-media/master]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Max-Kellermann/drivers-media-dvb-core-en50221-move-code-to-dvb_ca_private_free/20160321-213556
config: xtensa-allyesconfig (attached as .config)
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=xtensa 

All warnings (new ones prefixed by >>):

   drivers/media/dvb-core/dvb_ca_en50221.c: In function 'dvb_ca_private_free':
>> drivers/media/dvb-core/dvb_ca_en50221.c:167:2: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
     unsigned int i;
     ^

vim +167 drivers/media/dvb-core/dvb_ca_en50221.c

   151		/* Flag indicating the thread should wake up now */
   152		unsigned int wakeup:1;
   153	
   154		/* Delay the main thread should use */
   155		unsigned long delay;
   156	
   157		/* Slot to start looking for data to read from in the next user-space read operation */
   158		int next_read_slot;
   159	
   160		/* mutex serializing ioctls */
   161		struct mutex ioctl_mutex;
   162	};
   163	
   164	static void dvb_ca_private_free(struct dvb_ca_private *ca)
   165	{
   166		dvb_unregister_device(ca->dvbdev);
 > 167		unsigned int i;
   168		for (i = 0; i < ca->slot_count; i++) {
   169			vfree(ca->slot_info[i].rx_buffer.data);
   170		}
   171		kfree(ca->slot_info);
   172		kfree(ca);
   173	}
   174	
   175	static void dvb_ca_en50221_thread_wakeup(struct dvb_ca_private *ca);

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 44058 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 6/6] drivers/media/dvb-usb-dvb: postpone kfree(mdev)
  2016-03-21 13:30 ` [PATCH 6/6] drivers/media/dvb-usb-dvb: postpone kfree(mdev) Max Kellermann
  2016-03-21 13:45   ` kbuild test robot
@ 2016-03-21 13:56   ` kbuild test robot
  1 sibling, 0 replies; 11+ messages in thread
From: kbuild test robot @ 2016-03-21 13:56 UTC (permalink / raw)
  To: Max Kellermann; +Cc: kbuild-all, linux-kernel, linux-media

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

Hi Max,

[auto build test WARNING on v4.5-rc7]
[cannot apply to sailus-media/master linuxtv-media/master next-20160321]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Max-Kellermann/drivers-media-dvb-core-en50221-move-code-to-dvb_ca_private_free/20160321-213556
config: xtensa-allyesconfig (attached as .config)
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=xtensa 

All warnings (new ones prefixed by >>):

   drivers/media/usb/dvb-usb/dvb-usb-dvb.c: In function 'dvb_usb_media_device_unregister':
>> drivers/media/usb/dvb-usb/dvb-usb-dvb.c:148:2: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
     struct media_device *mdev = adap->dvb_adap.mdev;
     ^

vim +148 drivers/media/usb/dvb-usb/dvb-usb-dvb.c

   132	
   133	static int  dvb_usb_media_device_register(struct dvb_usb_adapter *adap)
   134	{
   135	#ifdef CONFIG_MEDIA_CONTROLLER_DVB
   136		return media_device_register(adap->dvb_adap.mdev);
   137	#else
   138		return 0;
   139	#endif
   140	}
   141	
   142	static void dvb_usb_media_device_unregister(struct dvb_usb_adapter *adap)
   143	{
   144	#ifdef CONFIG_MEDIA_CONTROLLER_DVB
   145		if (!adap->dvb_adap.mdev)
   146			return;
   147	
 > 148		struct media_device *mdev = adap->dvb_adap.mdev;
   149		adap->dvb_adap.mdev = NULL;
   150		media_device_unregister(mdev);
   151		/* media_device_cleanup() and kfree() will be called by the
   152		   callback function dvb_usb_media_device_release() */
   153	#endif
   154	}
   155	
   156	int dvb_usb_adapter_dvb_init(struct dvb_usb_adapter *adap, short *adapter_nums)

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 44058 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/6] drivers/media/dvb-core/en50221: postpone release until file is closed
  2016-03-21 13:30 ` [PATCH 2/6] drivers/media/dvb-core/en50221: postpone release until file is closed Max Kellermann
@ 2016-06-09  9:38   ` Mauro Carvalho Chehab
  0 siblings, 0 replies; 11+ messages in thread
From: Mauro Carvalho Chehab @ 2016-06-09  9:38 UTC (permalink / raw)
  To: Max Kellermann; +Cc: linux-kernel, linux-media

Em Mon, 21 Mar 2016 14:30:22 +0100
Max Kellermann <max@duempel.org> escreveu:

> Fixes use-after-free bug which occurs when I disconnect my DVB-S
> received while VDR is running.

I guess that the best way to fix it is to use a kref() that would be also
incremented at open() and decremented at release(). 

This works better than adding other non-standard ways to manage data livetime
cycle.

Regards,
Mauro

> 
> Signed-off-by: Max Kellermann <max@duempel.org>
> ---
>  drivers/media/dvb-core/dvb_ca_en50221.c |   23 ++++++++++++++++++++++-
>  1 file changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/dvb-core/dvb_ca_en50221.c b/drivers/media/dvb-core/dvb_ca_en50221.c
> index e33364c..dfc686a 100644
> --- a/drivers/media/dvb-core/dvb_ca_en50221.c
> +++ b/drivers/media/dvb-core/dvb_ca_en50221.c
> @@ -148,6 +148,9 @@ struct dvb_ca_private {
>  	/* Flag indicating if the CA device is open */
>  	unsigned int open:1;
>  
> +	/* Flag indicating if the CA device is released */
> +	unsigned int released:1;
> +
>  	/* Flag indicating the thread should wake up now */
>  	unsigned int wakeup:1;
>  
> @@ -1392,6 +1395,11 @@ static int dvb_ca_en50221_io_read_condition(struct dvb_ca_private *ca,
>  	int found = 0;
>  	u8 hdr[2];
>  
> +	if (ca->released) {
> +		*result = -ENODEV;
> +		return 1;
> +	}
> +
>  	slot = ca->next_read_slot;
>  	while ((slot_count < ca->slot_count) && (!found)) {
>  		if (ca->slot_info[slot].slot_state != DVB_CA_SLOTSTATE_RUNNING)
> @@ -1595,6 +1603,9 @@ static int dvb_ca_en50221_io_release(struct inode *inode, struct file *file)
>  
>  	err = dvb_generic_release(inode, file);
>  
> +	if (ca->released)
> +		dvb_ca_private_free(ca);
> +
>  	module_put(ca->pub->owner);
>  
>  	return err;
> @@ -1701,6 +1712,7 @@ int dvb_ca_en50221_init(struct dvb_adapter *dvb_adapter,
>  	}
>  	init_waitqueue_head(&ca->wait_queue);
>  	ca->open = 0;
> +	ca->released = 0;
>  	ca->wakeup = 0;
>  	ca->next_read_slot = 0;
>  	pubca->private = ca;
> @@ -1765,12 +1777,21 @@ void dvb_ca_en50221_release(struct dvb_ca_en50221 *pubca)
>  
>  	dprintk("%s\n", __func__);
>  
> +	BUG_ON(ca->released);
> +
>  	/* shutdown the thread if there was one */
>  	kthread_stop(ca->thread);
>  
>  	for (i = 0; i < ca->slot_count; i++) {
>  		dvb_ca_en50221_slot_shutdown(ca, i);
>  	}
> -	dvb_ca_private_free(ca);
> +
> +	if (ca->open) {
> +		ca->released = 1;
> +		mb();
> +		wake_up_interruptible(&ca->wait_queue);
> +	} else
> +		dvb_ca_private_free(ca);
> +
>  	pubca->private = NULL;
>  }
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


-- 
Thanks,
Mauro

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 5/6] drivers/media/media-device: add "release" callback
  2016-03-21 13:30 ` [PATCH 5/6] drivers/media/media-device: add "release" callback Max Kellermann
@ 2016-06-09 11:56   ` Mauro Carvalho Chehab
  0 siblings, 0 replies; 11+ messages in thread
From: Mauro Carvalho Chehab @ 2016-06-09 11:56 UTC (permalink / raw)
  To: Max Kellermann; +Cc: linux-kernel, linux-media, Shuah Khan

Em Mon, 21 Mar 2016 14:30:38 +0100
Max Kellermann <max@duempel.org> escreveu:

> Allow the client to free its data structures only after all files have
> been closed (fixing use-after-free bugs).

Hmm... Shuah is also working on fixing such issues at the media controller
stuff, and I made a few fix patches myself.

Our work is at:
	https://git.linuxtv.org/mchehab/experimental.git/log/?h=media_cdev_fix

Please base your changes on it, to avoid conflicting work or rework.

I just rebased it on the top of media_tree (plus the patches for it
that I didn't push yet).


Thanks!
Mauro


> 
> Signed-off-by: Max Kellermann <max@duempel.org>
> ---
>  drivers/media/media-device.c |    9 +++++++--
>  include/media/media-device.h |    2 ++
>  2 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> index 5c4669c..a3901f9 100644
> --- a/drivers/media/media-device.c
> +++ b/drivers/media/media-device.c
> @@ -551,9 +551,14 @@ static DEVICE_ATTR(model, S_IRUGO, show_model, NULL);
>   * Registration/unregistration
>   */
>  
> -static void media_device_release(struct media_devnode *mdev)
> +static void media_device_release(struct media_devnode *devnode)
>  {
> -	dev_dbg(mdev->parent, "Media device released\n");
> +	struct media_device *mdev = to_media_device(devnode);
> +
> +	dev_dbg(devnode->parent, "Media device released\n");
> +
> +	if (mdev->release)
> +		mdev->release(mdev);
>  }
>  
>  /**
> diff --git a/include/media/media-device.h b/include/media/media-device.h
> index d385589..d184d0c 100644
> --- a/include/media/media-device.h
> +++ b/include/media/media-device.h
> @@ -326,6 +326,8 @@ struct media_device {
>  
>  	int (*link_notify)(struct media_link *link, u32 flags,
>  			   unsigned int notification);
> +
> +	void (*release)(struct media_device *mdev);
>  };
>  
>  #ifdef CONFIG_MEDIA_CONTROLLER
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


-- 
Thanks,
Mauro

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2016-06-09 11:56 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-21 13:30 [PATCH 1/6] drivers/media/dvb-core/en50221: move code to dvb_ca_private_free() Max Kellermann
2016-03-21 13:30 ` [PATCH 2/6] drivers/media/dvb-core/en50221: postpone release until file is closed Max Kellermann
2016-06-09  9:38   ` Mauro Carvalho Chehab
2016-03-21 13:30 ` [PATCH 3/6] drivers/media/media-devnode: clear private_data before put_device() Max Kellermann
2016-03-21 13:30 ` [PATCH 4/6] drivers/media/media-device: move debug log before _devnode_unregister() Max Kellermann
2016-03-21 13:30 ` [PATCH 5/6] drivers/media/media-device: add "release" callback Max Kellermann
2016-06-09 11:56   ` Mauro Carvalho Chehab
2016-03-21 13:30 ` [PATCH 6/6] drivers/media/dvb-usb-dvb: postpone kfree(mdev) Max Kellermann
2016-03-21 13:45   ` kbuild test robot
2016-03-21 13:56   ` kbuild test robot
2016-03-21 13:55 ` [PATCH 1/6] drivers/media/dvb-core/en50221: move code to dvb_ca_private_free() kbuild test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).