From patchwork Fri Apr 29 22:37:32 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Shuah Khan X-Patchwork-Id: 8985881 Return-Path: X-Original-To: patchwork-linux-media@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 03DDABF29F for ; Fri, 29 Apr 2016 22:37:58 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id DEAEA201B4 for ; Fri, 29 Apr 2016 22:37:56 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id B703B2015E for ; Fri, 29 Apr 2016 22:37:55 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752331AbcD2Whh (ORCPT ); Fri, 29 Apr 2016 18:37:37 -0400 Received: from mailout.easymail.ca ([64.68.201.169]:37851 "EHLO mailout.easymail.ca" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751725AbcD2Whg (ORCPT ); Fri, 29 Apr 2016 18:37:36 -0400 Received: from localhost (localhost [127.0.0.1]) by mailout.easymail.ca (Postfix) with ESMTP id 3A2EBA12D3; Fri, 29 Apr 2016 18:37:35 -0400 (EDT) X-Virus-Scanned: Debian amavisd-new at mailout.easymail.ca X-Spam-Score: -3.695 X-Spam-Level: X-Spam-Status: No, score=-7.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 Received: from mailout.easymail.ca ([127.0.0.1]) by localhost (easymail-mailout.easydns.vpn [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id l8aLCQPaUoRF; Fri, 29 Apr 2016 18:37:34 -0400 (EDT) Received: from mail.gonehiking.org (c-73-181-52-62.hsd1.co.comcast.net [73.181.52.62]) by mailout.easymail.ca (Postfix) with ESMTPA id 9B927A12C6; Fri, 29 Apr 2016 18:37:34 -0400 (EDT) Received: from shuah-XPS-13-9350.sisa.samsung.com (shuah-xps.internal [192.168.1.87]) by mail.gonehiking.org (Postfix) with ESMTP id E5BD29F129; Fri, 29 Apr 2016 16:37:33 -0600 (MDT) From: Shuah Khan To: mchehab@osg.samsung.com, laurent.pinchart@ideasonboard.com, sakari.ailus@iki.fi, lars@metafoo.de Cc: Shuah Khan , linux-media@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH] media: fix use-after-free in cdev_put() when app exits after driver unbind Date: Fri, 29 Apr 2016 16:37:32 -0600 Message-Id: <1461969452-9276-1-git-send-email-shuahkh@osg.samsung.com> X-Mailer: git-send-email 2.7.4 Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP When driver unbinds while media_ioctl is in progress, cdev_put() fails with when app exits after driver unbinds. Add a kobject to the media_devnode structure and set this kobject as the cdev parent kobject. This allows cdev_add() to hold a reference to it and release the reference in cdev_del() ensuring that the media_devnode is not deallocated as long as the application has the cdev open. This problem is found on uvcvideo, em28xx, and au0828 drivers and fix has been tested on all three. cdev_put() error as follows: kernel: [ 193.599736] BUG: KASAN: use-after-free in cdev_put+0x4e/0x50 at addr ffff8801d31af2b8 kernel: [ 193.599745] Read of size 8 by task media_device_te/1851 kernel: [ 193.599792] INFO: Allocated in __media_device_register+0x54/0x2e0 [media] age=38615 cpu=0 pid=313 kernel: [ 193.599951] INFO: Freed in media_devnode_release+0xa4/0xc0 [media] age=0 cpu=0 pid=1851 kernel: [ 193.601083] Call Trace: kernel: [ 193.601093] [] dump_stack+0x67/0x94 kernel: [ 193.601102] [] print_trailer+0x112/0x1a0 kernel: [ 193.601111] [] object_err+0x34/0x40 kernel: [ 193.601119] [] kasan_report_error+0x224/0x530 kernel: [ 193.601128] [] ? kzfree+0x2d/0x40 kernel: [ 193.601137] [] ? kfree+0x1d2/0x1f0 kernel: [ 193.601145] [] __asan_report_load8_noabort+0x43/0x50 kernel: [ 193.601154] [] ? cdev_put+0x4e/0x50 kernel: [ 193.601162] [] cdev_put+0x4e/0x50 kernel: [ 193.601170] [] __fput+0x52b/0x6c0 kernel: [ 193.601179] [] ? switch_task_namespaces+0x2a/0x90 kernel: [ 193.601188] [] ____fput+0xe/0x10 kernel: [ 193.601196] [] task_work_run+0x133/0x1f0 kernel: [ 193.601204] [] ? switch_task_namespaces+0x5e/0x90 anduin kernel: [ 193.601213] [] do_exit+0x72c/0x2c20 anduin kernel: [ 193.601224] [] ? release_task+0x1250/0x1250 - - - kernel: [ 193.601360] [] ? exit_to_usermode_loop+0xe7/0x170 kernel: [ 193.601368] [] exit_to_usermode_loop+0x120/0x170 kernel: [ 193.601376] [] syscall_return_slowpath+0x16a/0x1a0 kernel: [ 193.601386] [] entry_SYSCALL_64_fastpath+0xa6/0xa8 Signed-off-by: Shuah Khan --- drivers/media/media-device.c | 6 ++++-- drivers/media/media-devnode.c | 21 +++++++++++++++++++-- include/media/media-devnode.h | 2 ++ 3 files changed, 25 insertions(+), 4 deletions(-) diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c index 84e6a0b..b388a0e 100644 --- a/drivers/media/media-device.c +++ b/drivers/media/media-device.c @@ -742,16 +742,16 @@ int __must_check __media_device_register(struct media_device *mdev, ret = media_devnode_register(mdev, devnode, owner); if (ret < 0) { + /* kfree devnode is done via kobject_put() handler */ mdev->devnode = NULL; - kfree(devnode); return ret; } ret = device_create_file(&devnode->dev, &dev_attr_model); if (ret < 0) { + /* kfree devnode is done via kobject_put() handler */ mdev->devnode = NULL; media_devnode_unregister(devnode); - kfree(devnode); return ret; } @@ -829,6 +829,8 @@ void media_device_unregister(struct media_device *mdev) if (media_devnode_is_registered(mdev->devnode)) { device_remove_file(&mdev->devnode->dev, &dev_attr_model); media_devnode_unregister(mdev->devnode); + /* kfree devnode is done via kobject_put() handler */ + mdev->devnode = NULL; } dev_dbg(mdev->dev, "Media device unregistered\n"); diff --git a/drivers/media/media-devnode.c b/drivers/media/media-devnode.c index ca7c4b9..21f81f3 100644 --- a/drivers/media/media-devnode.c +++ b/drivers/media/media-devnode.c @@ -75,8 +75,6 @@ static void media_devnode_release(struct device *cd) /* Release media_devnode and perform other cleanups as needed. */ if (devnode->release) devnode->release(devnode); - - kfree(devnode); } static struct bus_type media_bus_type = { @@ -221,6 +219,19 @@ static const struct file_operations media_devnode_fops = { .llseek = no_llseek, }; +static void media_devnode_free(struct kobject *kobj) +{ + struct media_devnode *devnode = + container_of(kobj, struct media_devnode, kobj); + + kfree(devnode); + pr_info("%s: Media Devnode Deallocated\n", __func__); +} + +static struct kobj_type media_devnode_ktype = { + .release = media_devnode_free, +}; + int __must_check media_devnode_register(struct media_device *mdev, struct media_devnode *devnode, struct module *owner) @@ -243,9 +254,12 @@ int __must_check media_devnode_register(struct media_device *mdev, devnode->minor = minor; devnode->media_dev = mdev; + kobject_init(&devnode->kobj, &media_devnode_ktype); + /* Part 2: Initialize and register the character device */ cdev_init(&devnode->cdev, &media_devnode_fops); devnode->cdev.owner = owner; + devnode->cdev.kobj.parent = &devnode->kobj; ret = cdev_add(&devnode->cdev, MKDEV(MAJOR(media_dev_t), devnode->minor), 1); if (ret < 0) { @@ -274,6 +288,7 @@ int __must_check media_devnode_register(struct media_device *mdev, error: cdev_del(&devnode->cdev); clear_bit(devnode->minor, media_devnode_nums); + kobject_put(&devnode->kobj); return ret; } @@ -287,6 +302,8 @@ void media_devnode_unregister(struct media_devnode *devnode) clear_bit(MEDIA_FLAG_REGISTERED, &devnode->flags); mutex_unlock(&media_devnode_lock); device_unregister(&devnode->dev); + devnode->media_dev = NULL; + kobject_put(&devnode->kobj); } /* diff --git a/include/media/media-devnode.h b/include/media/media-devnode.h index 5bb3b0e..ce9b051 100644 --- a/include/media/media-devnode.h +++ b/include/media/media-devnode.h @@ -72,6 +72,7 @@ struct media_file_operations { * @fops: pointer to struct &media_file_operations with media device ops * @dev: struct device pointer for the media controller device * @cdev: struct cdev pointer character device + * @kobj: struct kobject * @parent: parent device * @minor: device node minor number * @flags: flags, combination of the MEDIA_FLAG_* constants @@ -91,6 +92,7 @@ struct media_devnode { /* sysfs */ struct device dev; /* media device */ struct cdev cdev; /* character device */ + struct kobject kobj; /* set as cdev parent kobj */ struct device *parent; /* device parent */ /* device info */