From patchwork Mon Oct 28 11:42:23 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Keeping X-Patchwork-Id: 11215189 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id DA12B913 for ; Mon, 28 Oct 2019 11:43:24 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id B83E321835 for ; Mon, 28 Oct 2019 11:43:24 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=metanate.com header.i=@metanate.com header.b="SCyhU9n5" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1731425AbfJ1LnX (ORCPT ); Mon, 28 Oct 2019 07:43:23 -0400 Received: from dougal.metanate.com ([90.155.101.14]:65410 "EHLO metanate.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S2388615AbfJ1LnA (ORCPT ); Mon, 28 Oct 2019 07:43:00 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=simple/simple; d=metanate.com; s=stronger; h=Content-Transfer-Encoding:MIME-Version:References:In-Reply-To: Message-Id:Date:Subject:Cc:To:From:Sender:Reply-To:Content-Type:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=SGkZ9L6ksEJoGJ6vp6RGQ8/g+HjRI+7LlWLrCTmofEk=; b=SCyhU9n5+rNKbQmbcRe/RO33Qb 8U+JwdBdzLvZ7MNTf5vyV3ijael9zi8mjbL1P5VOWum/7CFxQbbtRrvPDW/RO4sqJ5IvOITfy7eec xC+3O1k2mHoAL1lr3qX5ArOXc7+uThTDgPs3R9PcKB4Q9Wq31z00BqBUJWv6ltQm2oPZUvLT6eKoO dvfpZGHTflO/EfMPI/VmJNYe6Ma5lZuVlyV1t/kBAcY1bJN1bnZRNwxGrXG+zUu9r+WWqUBMkZ5Rs GnsHKF/dJnnu6ALNQzAXsgrdSsde01+PM9qJVjOOYJki5AfQjLBo/U5UHS9kSRPSG+D8G8tmkNciq 5Y1Oni/g==; Received: from dougal.metanate.com ([192.168.88.1] helo=localhost.localdomain) by email.metanate.com with esmtpsa (TLSv1.3:TLS_AES_256_GCM_SHA384:256) (Exim 4.92.3) (envelope-from ) id 1iP3QH-00010o-7u; Mon, 28 Oct 2019 11:42:57 +0000 From: John Keeping To: linux-usb@vger.kernel.org Cc: Felipe Balbi , Greg Kroah-Hartman , linux-kernel@vger.kernel.org, John Keeping Subject: [PATCH v2 1/6] USB: gadget: f_hid: move chardev setup to module init Date: Mon, 28 Oct 2019 11:42:23 +0000 Message-Id: <20191028114228.3679219-2-john@metanate.com> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20191028114228.3679219-1-john@metanate.com> References: <20191024164538.3161474-1-john@metanate.com> <20191028114228.3679219-1-john@metanate.com> MIME-Version: 1.0 X-Authenticated: YES Sender: linux-usb-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-usb@vger.kernel.org Lifetime of the objects in the HID gadget is currently tied to the USB function, which makes it very easy to trigger a use-after-free by holding a file descriptor on the HID device after deleting the gadget. As a first step towards fixing this, move the character device allocation to module initialisation so that we don't have to worry about when the allocation can be released after closing all of the open handles on the device. Note that we also have to move the USB function registration into the initialisation function in order to avoid having two module_init() functions. Signed-off-by: John Keeping --- v2: - Switch to DECLARE_USB_FUNCTION and manual function (de)registration to fix compiling as a module drivers/usb/gadget/function/f_hid.c | 41 ++++++++++++++--------------- drivers/usb/gadget/function/u_hid.h | 3 --- 2 files changed, 20 insertions(+), 24 deletions(-) diff --git a/drivers/usb/gadget/function/f_hid.c b/drivers/usb/gadget/function/f_hid.c index f3816a5c861e..e5e18d02b77a 100644 --- a/drivers/usb/gadget/function/f_hid.c +++ b/drivers/usb/gadget/function/f_hid.c @@ -1006,11 +1006,7 @@ static void hidg_free_inst(struct usb_function_instance *f) opts = container_of(f, struct f_hid_opts, func_inst); mutex_lock(&hidg_ida_lock); - hidg_put_minor(opts->minor); - if (ida_is_empty(&hidg_ida)) - ghid_cleanup(); - mutex_unlock(&hidg_ida_lock); if (opts->report_desc_alloc) @@ -1023,7 +1019,6 @@ static struct usb_function_instance *hidg_alloc_inst(void) { struct f_hid_opts *opts; struct usb_function_instance *ret; - int status = 0; opts = kzalloc(sizeof(*opts), GFP_KERNEL); if (!opts) @@ -1034,21 +1029,10 @@ static struct usb_function_instance *hidg_alloc_inst(void) mutex_lock(&hidg_ida_lock); - if (ida_is_empty(&hidg_ida)) { - status = ghid_setup(NULL, HIDG_MINORS); - if (status) { - ret = ERR_PTR(status); - kfree(opts); - goto unlock; - } - } - opts->minor = hidg_get_minor(); if (opts->minor < 0) { ret = ERR_PTR(opts->minor); kfree(opts); - if (ida_is_empty(&hidg_ida)) - ghid_cleanup(); goto unlock; } config_group_init_type_name(&opts->func_inst.group, "", &hid_func_type); @@ -1129,11 +1113,11 @@ static struct usb_function *hidg_alloc(struct usb_function_instance *fi) return &hidg->func; } -DECLARE_USB_FUNCTION_INIT(hid, hidg_alloc_inst, hidg_alloc); +DECLARE_USB_FUNCTION(hid, hidg_alloc_inst, hidg_alloc); MODULE_LICENSE("GPL"); MODULE_AUTHOR("Fabien Chouteau"); -int ghid_setup(struct usb_gadget *g, int count) +static int ghid_setup(void) { int status; dev_t dev; @@ -1145,7 +1129,7 @@ int ghid_setup(struct usb_gadget *g, int count) return status; } - status = alloc_chrdev_region(&dev, 0, count, "hidg"); + status = alloc_chrdev_region(&dev, 0, HIDG_MINORS, "hidg"); if (status) { class_destroy(hidg_class); hidg_class = NULL; @@ -1153,13 +1137,25 @@ int ghid_setup(struct usb_gadget *g, int count) } major = MAJOR(dev); - minors = count; + minors = HIDG_MINORS; + + status = usb_function_register(&hidusb_func); + if (status) + goto fail_unregister; return 0; + +fail_unregister: + unregister_chrdev_region(dev, HIDG_MINORS); + class_destroy(hidg_class); + hidg_class = NULL; + return status; } -void ghid_cleanup(void) +static void ghid_cleanup(void) { + usb_function_unregister(&hidusb_func); + if (major) { unregister_chrdev_region(MKDEV(major, 0), minors); major = minors = 0; @@ -1168,3 +1164,6 @@ void ghid_cleanup(void) class_destroy(hidg_class); hidg_class = NULL; } + +module_init(ghid_setup); +module_exit(ghid_cleanup); diff --git a/drivers/usb/gadget/function/u_hid.h b/drivers/usb/gadget/function/u_hid.h index 1594bfa312eb..d52acc977c7e 100644 --- a/drivers/usb/gadget/function/u_hid.h +++ b/drivers/usb/gadget/function/u_hid.h @@ -33,7 +33,4 @@ struct f_hid_opts { int refcnt; }; -int ghid_setup(struct usb_gadget *g, int count); -void ghid_cleanup(void); - #endif /* U_HID_H */ From patchwork Mon Oct 28 11:42:24 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Keeping X-Patchwork-Id: 11215179 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 1A53C112B for ; Mon, 28 Oct 2019 11:43:11 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id ED985214E0 for ; Mon, 28 Oct 2019 11:43:10 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=metanate.com header.i=@metanate.com header.b="abKHpX5L" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2388636AbfJ1LnB (ORCPT ); Mon, 28 Oct 2019 07:43:01 -0400 Received: from dougal.metanate.com ([90.155.101.14]:40476 "EHLO metanate.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1727385AbfJ1LnA (ORCPT ); Mon, 28 Oct 2019 07:43:00 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=simple/simple; d=metanate.com; s=stronger; h=Content-Transfer-Encoding:MIME-Version:References:In-Reply-To: Message-Id:Date:Subject:Cc:To:From:Sender:Reply-To:Content-Type:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=lwODNx/KjIj+Y/Cod9TZnDM+jFNNhhm9mbvUKZSiSdY=; b=abKHpX5LsJ0gM4zh/a3HRf7+Dz 4Y8eWJJaa/6e+/94YeREhIkRrZFkeKv/YejskVC7f5/x7SiHQGYtztOKOSGpz5cBv5bvQk7HSEzQ9 45M2hqjGCXMQTeLuss0aOt+3fF+m3OPXbY+RATZKN1lF/tCA/2LKUNhWV+5LhfjQsVxqS350kNk4x XLiH0UFgUq/t/LGQDtUbKoqldOdohrQF8aZfVrp5cYYpo/mmSB4udFKeZcb4fxNkXRmDmYy/ytMbO 2odavFBBkmzCerU1A5woP2lhBkkvGFL/dSPJMeak8jayBO+oVaDuD6cJya4DxhxoIBl+T0xL3S12o Pe62aF2A==; Received: from dougal.metanate.com ([192.168.88.1] helo=localhost.localdomain) by email.metanate.com with esmtpsa (TLSv1.3:TLS_AES_256_GCM_SHA384:256) (Exim 4.92.3) (envelope-from ) id 1iP3QH-00010o-AE; Mon, 28 Oct 2019 11:42:57 +0000 From: John Keeping To: linux-usb@vger.kernel.org Cc: Felipe Balbi , Greg Kroah-Hartman , linux-kernel@vger.kernel.org, John Keeping Subject: [PATCH v2 2/6] USB: gadget: f_hid: switch to IDR for tracking minors Date: Mon, 28 Oct 2019 11:42:24 +0000 Message-Id: <20191028114228.3679219-3-john@metanate.com> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20191028114228.3679219-1-john@metanate.com> References: <20191024164538.3161474-1-john@metanate.com> <20191028114228.3679219-1-john@metanate.com> MIME-Version: 1.0 X-Authenticated: YES Sender: linux-usb-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-usb@vger.kernel.org In the following commits, we are going to move the cdev allocation up to the module initialisation, and will look up the associated f_hidg structure from the open operation. Start preparing for this by switching from IDA to IDR. Signed-off-by: John Keeping --- v2: - No changes drivers/usb/gadget/function/f_hid.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/usb/gadget/function/f_hid.c b/drivers/usb/gadget/function/f_hid.c index e5e18d02b77a..1eb8b545e5b4 100644 --- a/drivers/usb/gadget/function/f_hid.c +++ b/drivers/usb/gadget/function/f_hid.c @@ -24,8 +24,8 @@ static int major, minors; static struct class *hidg_class; -static DEFINE_IDA(hidg_ida); -static DEFINE_MUTEX(hidg_ida_lock); /* protects access to hidg_ida */ +static DEFINE_IDR(hidg_idr); +static DEFINE_MUTEX(hidg_idr_lock); /* protects access to hidg_idr */ /*-------------------------------------------------------------------------*/ /* HID gadget struct */ @@ -851,9 +851,9 @@ static inline int hidg_get_minor(void) { int ret; - ret = ida_simple_get(&hidg_ida, 0, 0, GFP_KERNEL); + ret = idr_alloc(&hidg_idr, NULL, 0, 0, GFP_KERNEL); if (ret >= HIDG_MINORS) { - ida_simple_remove(&hidg_ida, ret); + idr_remove(&hidg_idr, ret); ret = -ENODEV; } @@ -996,7 +996,7 @@ static const struct config_item_type hid_func_type = { static inline void hidg_put_minor(int minor) { - ida_simple_remove(&hidg_ida, minor); + idr_remove(&hidg_idr, minor); } static void hidg_free_inst(struct usb_function_instance *f) @@ -1005,9 +1005,9 @@ static void hidg_free_inst(struct usb_function_instance *f) opts = container_of(f, struct f_hid_opts, func_inst); - mutex_lock(&hidg_ida_lock); + mutex_lock(&hidg_idr_lock); hidg_put_minor(opts->minor); - mutex_unlock(&hidg_ida_lock); + mutex_unlock(&hidg_idr_lock); if (opts->report_desc_alloc) kfree(opts->report_desc); @@ -1027,7 +1027,7 @@ static struct usb_function_instance *hidg_alloc_inst(void) opts->func_inst.free_func_inst = hidg_free_inst; ret = &opts->func_inst; - mutex_lock(&hidg_ida_lock); + mutex_lock(&hidg_idr_lock); opts->minor = hidg_get_minor(); if (opts->minor < 0) { @@ -1038,7 +1038,7 @@ static struct usb_function_instance *hidg_alloc_inst(void) config_group_init_type_name(&opts->func_inst.group, "", &hid_func_type); unlock: - mutex_unlock(&hidg_ida_lock); + mutex_unlock(&hidg_idr_lock); return ret; } From patchwork Mon Oct 28 11:42:25 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Keeping X-Patchwork-Id: 11215185 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 65B88112B for ; Mon, 28 Oct 2019 11:43:21 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 4435521835 for ; Mon, 28 Oct 2019 11:43:21 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=metanate.com header.i=@metanate.com header.b="JFJJkAY0" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2388624AbfJ1LnA (ORCPT ); Mon, 28 Oct 2019 07:43:00 -0400 Received: from dougal.metanate.com ([90.155.101.14]:46592 "EHLO metanate.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S2388603AbfJ1LnA (ORCPT ); Mon, 28 Oct 2019 07:43:00 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=simple/simple; d=metanate.com; s=stronger; h=Content-Transfer-Encoding:MIME-Version:References:In-Reply-To: Message-Id:Date:Subject:Cc:To:From:Sender:Reply-To:Content-Type:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=3yHp3hDaqlC9sVsaUEOh4hHDVX97afo481OX9Wv/28o=; b=JFJJkAY0J7hR+kY0KnpsFW0Rk9 Sq3llvjAUA+tuAY0LLX35UJmclE0S1q9ht/dntPdOxOQqip+NTg3/n/UTvXoK2JENJjLhBCg2JXTd W0eaBplcq/gcEJyu2dGF74zmCMdVPOSHlNUtWVywKgMUfE744jr6k06cjoX9GgAk7z1pEH/7k0AK+ QRSXgX5f8+RXsa0zBXcZnz235T17UcIu5GTnzLuRCSa3heJGxokM1SLQWUver77S+mpClWnyZzfE/ 6ACk3pV9GEnO/fZAN69RJxCJyjYUCTSee3AjvCq8dsR/Es9AzcxBhOCDmvc8vw33ghF9YewbdwF8y IvJVjrIQ==; Received: from dougal.metanate.com ([192.168.88.1] helo=localhost.localdomain) by email.metanate.com with esmtpsa (TLSv1.3:TLS_AES_256_GCM_SHA384:256) (Exim 4.92.3) (envelope-from ) id 1iP3QH-00010o-CL; Mon, 28 Oct 2019 11:42:57 +0000 From: John Keeping To: linux-usb@vger.kernel.org Cc: Felipe Balbi , Greg Kroah-Hartman , linux-kernel@vger.kernel.org, John Keeping Subject: [PATCH v2 3/6] USB: gadget: f_hid: find f_hidg by IDR lookup on open Date: Mon, 28 Oct 2019 11:42:25 +0000 Message-Id: <20191028114228.3679219-4-john@metanate.com> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20191028114228.3679219-1-john@metanate.com> References: <20191024164538.3161474-1-john@metanate.com> <20191028114228.3679219-1-john@metanate.com> MIME-Version: 1.0 X-Authenticated: YES Sender: linux-usb-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-usb@vger.kernel.org As a step towards decoupling the cdev lifetime from f_hidg, lookup the f_hidg structure by minor number from IDR when opening the device. Signed-off-by: John Keeping --- v2: - No changes drivers/usb/gadget/function/f_hid.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/drivers/usb/gadget/function/f_hid.c b/drivers/usb/gadget/function/f_hid.c index 1eb8b545e5b4..6cf3b5b14ded 100644 --- a/drivers/usb/gadget/function/f_hid.c +++ b/drivers/usb/gadget/function/f_hid.c @@ -441,8 +441,14 @@ static int f_hidg_release(struct inode *inode, struct file *fd) static int f_hidg_open(struct inode *inode, struct file *fd) { - struct f_hidg *hidg = - container_of(inode->i_cdev, struct f_hidg, cdev); + struct f_hidg *hidg; + + mutex_lock(&hidg_idr_lock); + hidg = idr_find(&hidg_idr, iminor(inode)); + mutex_unlock(&hidg_idr_lock); + + if (!hidg) + return -ENODEV; fd->private_data = hidg; @@ -827,6 +833,10 @@ static int hidg_bind(struct usb_configuration *c, struct usb_function *f) if (status) goto fail_free_descs; + mutex_lock(&hidg_idr_lock); + idr_replace(&hidg_idr, hidg, hidg->minor); + mutex_unlock(&hidg_idr_lock); + device = device_create(hidg_class, NULL, dev, NULL, "%s%d", "hidg", hidg->minor); if (IS_ERR(device)) { From patchwork Mon Oct 28 11:42:26 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Keeping X-Patchwork-Id: 11215187 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 8E653197C for ; Mon, 28 Oct 2019 11:43:23 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 6C78021744 for ; Mon, 28 Oct 2019 11:43:23 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=metanate.com header.i=@metanate.com header.b="mjFa0gJw" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2388618AbfJ1LnA (ORCPT ); Mon, 28 Oct 2019 07:43:00 -0400 Received: from dougal.metanate.com ([90.155.101.14]:58166 "EHLO metanate.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1727225AbfJ1LnA (ORCPT ); Mon, 28 Oct 2019 07:43:00 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=simple/simple; d=metanate.com; s=stronger; h=Content-Transfer-Encoding:MIME-Version:References:In-Reply-To: Message-Id:Date:Subject:Cc:To:From:Sender:Reply-To:Content-Type:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=gvMfLh+YRX5h+NeO0cijiJcG+SWNrmorQ1gq6rn2++0=; b=mjFa0gJwbwJ2hNyjVmg6/IdfVu 59RoUPhgGAffgLln4Aopn65srewCihVIXydpvnMzE4pEckky26bdmw32mFGR0hC3BJVdF65qnILdd 2nkl3nEGhwjGJkEr9yykQWs0zXucTGBrpOkZIiYthe50XGvooWgGeWGkK4evp7BvUcNaqWhvSPTyo VeSvj75d//V9doHG1EPhYXllAv/E/rbFcGmGtDOb4+1Rr7GxaNcfGCbbNRzdWx6yycrkCmJ0zcGhA SbLNkiEKXAeXSjdwYk72xoHqE6fOZMJl1L6x2F/QTtDmxOprCmK0f5JVuv8weof1UdIXbzt6DFDkR pT+zrPvg==; Received: from dougal.metanate.com ([192.168.88.1] helo=localhost.localdomain) by email.metanate.com with esmtpsa (TLSv1.3:TLS_AES_256_GCM_SHA384:256) (Exim 4.92.3) (envelope-from ) id 1iP3QH-00010o-EZ; Mon, 28 Oct 2019 11:42:57 +0000 From: John Keeping To: linux-usb@vger.kernel.org Cc: Felipe Balbi , Greg Kroah-Hartman , linux-kernel@vger.kernel.org, John Keeping Subject: [PATCH v2 4/6] USB: gadget: f_hid: decouple cdev from f_hidg lifetime Date: Mon, 28 Oct 2019 11:42:26 +0000 Message-Id: <20191028114228.3679219-5-john@metanate.com> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20191028114228.3679219-1-john@metanate.com> References: <20191024164538.3161474-1-john@metanate.com> <20191028114228.3679219-1-john@metanate.com> MIME-Version: 1.0 X-Authenticated: YES Sender: linux-usb-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-usb@vger.kernel.org The character device needs to live until the last file descriptor has been closed (and until after ->release() is called in that case). Change the lifetime of our character devices to match the module, so as to avoid a use-after-free when closing a file descriptor after the gadget function has been deleted. Signed-off-by: John Keeping --- v2: - Updated for changes in patch 1 drivers/usb/gadget/function/f_hid.c | 42 +++++++++++++++++++++-------- 1 file changed, 31 insertions(+), 11 deletions(-) diff --git a/drivers/usb/gadget/function/f_hid.c b/drivers/usb/gadget/function/f_hid.c index 6cf3b5b14ded..eda4f24d2790 100644 --- a/drivers/usb/gadget/function/f_hid.c +++ b/drivers/usb/gadget/function/f_hid.c @@ -24,6 +24,7 @@ static int major, minors; static struct class *hidg_class; +static struct cdev *hidg_cdev; static DEFINE_IDR(hidg_idr); static DEFINE_MUTEX(hidg_idr_lock); /* protects access to hidg_idr */ @@ -58,7 +59,6 @@ struct f_hidg { struct usb_request *req; int minor; - struct cdev cdev; struct usb_function func; struct usb_ep *in_ep; @@ -827,11 +827,7 @@ static int hidg_bind(struct usb_configuration *c, struct usb_function *f) INIT_LIST_HEAD(&hidg->completed_out_req); /* create char device */ - cdev_init(&hidg->cdev, &f_hidg_fops); dev = MKDEV(major, hidg->minor); - status = cdev_add(&hidg->cdev, dev, 1); - if (status) - goto fail_free_descs; mutex_lock(&hidg_idr_lock); idr_replace(&hidg_idr, hidg, hidg->minor); @@ -841,13 +837,14 @@ static int hidg_bind(struct usb_configuration *c, struct usb_function *f) "%s%d", "hidg", hidg->minor); if (IS_ERR(device)) { status = PTR_ERR(device); - goto del; + goto fail_idr_remove; } return 0; -del: - cdev_del(&hidg->cdev); -fail_free_descs: +fail_idr_remove: + mutex_lock(&hidg_idr_lock); + idr_replace(&hidg_idr, NULL, hidg->minor); + mutex_unlock(&hidg_idr_lock); usb_free_all_descriptors(f); fail: ERROR(f->config->cdev, "hidg_bind FAILED\n"); @@ -1071,7 +1068,10 @@ static void hidg_unbind(struct usb_configuration *c, struct usb_function *f) struct f_hidg *hidg = func_to_hidg(f); device_destroy(hidg_class, MKDEV(major, hidg->minor)); - cdev_del(&hidg->cdev); + + mutex_lock(&hidg_idr_lock); + idr_replace(&hidg_idr, NULL, hidg->minor); + mutex_unlock(&hidg_idr_lock); usb_free_all_descriptors(f); } @@ -1129,6 +1129,7 @@ MODULE_AUTHOR("Fabien Chouteau"); static int ghid_setup(void) { + struct cdev *cdev = NULL; int status; dev_t dev; @@ -1149,12 +1150,30 @@ static int ghid_setup(void) major = MAJOR(dev); minors = HIDG_MINORS; + status = -ENOMEM; + cdev = cdev_alloc(); + if (!cdev) + goto fail_unregister; + + cdev->owner = THIS_MODULE; + cdev->ops = &f_hidg_fops; + kobject_set_name(&cdev->kobj, "hidg"); + + status = cdev_add(cdev, dev, HIDG_MINORS); + if (status) + goto fail_put; + status = usb_function_register(&hidusb_func); if (status) - goto fail_unregister; + goto fail_cdev_del; + hidg_cdev = cdev; return 0; +fail_cdev_del: + cdev_del(cdev); +fail_put: + kobject_put(&cdev->kobj); fail_unregister: unregister_chrdev_region(dev, HIDG_MINORS); class_destroy(hidg_class); @@ -1165,6 +1184,7 @@ static int ghid_setup(void) static void ghid_cleanup(void) { usb_function_unregister(&hidusb_func); + cdev_del(hidg_cdev); if (major) { unregister_chrdev_region(MKDEV(major, 0), minors); From patchwork Mon Oct 28 11:42:27 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Keeping X-Patchwork-Id: 11215181 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id E0918913 for ; Mon, 28 Oct 2019 11:43:18 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id BEDDC21734 for ; Mon, 28 Oct 2019 11:43:18 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=metanate.com header.i=@metanate.com header.b="ABxj6U1v" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2388668AbfJ1LnL (ORCPT ); Mon, 28 Oct 2019 07:43:11 -0400 Received: from dougal.metanate.com ([90.155.101.14]:44636 "EHLO metanate.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1731449AbfJ1LnA (ORCPT ); Mon, 28 Oct 2019 07:43:00 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=simple/simple; d=metanate.com; s=stronger; h=Content-Transfer-Encoding:MIME-Version:References:In-Reply-To: Message-Id:Date:Subject:Cc:To:From:Sender:Reply-To:Content-Type:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=Ac3UIMQkFOh3X333lUIUMM5j7cJ/cHx1R763/WKhAn4=; b=ABxj6U1v40rqe+BRDL1iA6ovBQ hELzNsoYeEuYVWrdf5X78Z1SBx/Tf9u4G7tJKcCg3N7FTxZtK3cDDqAaE89jOyLqAzy+WClFWbi+z Tum7nzFZfW4hVB13SrcFheqIxU1aXblEHruUsZSfUUVgxGPHis2Eg1Yu33sniLkGLRW9CHo+jGn2k 9AQdT+OZ5eqmCLMfDims30YFC+NW8PzjfNgdInkexXhzHDVhxBoZNAM6S4f0UFW1KOxGC5IEnZQn1 V6poFBhTgQb6s9rUUFf5OXzPrF91ez2QjvbVaI9Nc0HGh9m3woD+DEBp/ERjJVMr5ymfWNofNryE5 nBuef9wQ==; Received: from dougal.metanate.com ([192.168.88.1] helo=localhost.localdomain) by email.metanate.com with esmtpsa (TLSv1.3:TLS_AES_256_GCM_SHA384:256) (Exim 4.92.3) (envelope-from ) id 1iP3QH-00010o-Go; Mon, 28 Oct 2019 11:42:57 +0000 From: John Keeping To: linux-usb@vger.kernel.org Cc: Felipe Balbi , Greg Kroah-Hartman , linux-kernel@vger.kernel.org, John Keeping Subject: [PATCH v2 5/6] USB: gadget: f_hid: refcount f_hidg structure Date: Mon, 28 Oct 2019 11:42:27 +0000 Message-Id: <20191028114228.3679219-6-john@metanate.com> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20191028114228.3679219-1-john@metanate.com> References: <20191024164538.3161474-1-john@metanate.com> <20191028114228.3679219-1-john@metanate.com> MIME-Version: 1.0 X-Authenticated: YES Sender: linux-usb-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-usb@vger.kernel.org f_hidg is referenced by file descriptors opened on /dev/hidgN as well as being the USB gadget function. Since these file descriptors can be kept alive after the gadget function has been deleted, we need to decouple the lifetime of the f_hidg structure from the function. Make f_hidg reference counted so that it remains alive after the gadget function has been deleted if necessary. Signed-off-by: John Keeping --- v2: - No changes drivers/usb/gadget/function/f_hid.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/drivers/usb/gadget/function/f_hid.c b/drivers/usb/gadget/function/f_hid.c index eda4f24d2790..3d848f7a4cca 100644 --- a/drivers/usb/gadget/function/f_hid.c +++ b/drivers/usb/gadget/function/f_hid.c @@ -58,6 +58,7 @@ struct f_hidg { wait_queue_head_t write_queue; struct usb_request *req; + struct kref kref; int minor; struct usb_function func; @@ -70,6 +71,14 @@ static inline struct f_hidg *func_to_hidg(struct usb_function *f) return container_of(f, struct f_hidg, func); } +static void hidg_release(struct kref *kref) +{ + struct f_hidg *hidg = container_of(kref, struct f_hidg, kref); + + kfree(hidg->report_desc); + kfree(hidg); +} + /*-------------------------------------------------------------------------*/ /* Static descriptors */ @@ -435,6 +444,9 @@ static __poll_t f_hidg_poll(struct file *file, poll_table *wait) static int f_hidg_release(struct inode *inode, struct file *fd) { + struct f_hidg *hidg = fd->private_data; + + kref_put(&hidg->kref, hidg_release); fd->private_data = NULL; return 0; } @@ -445,6 +457,8 @@ static int f_hidg_open(struct inode *inode, struct file *fd) mutex_lock(&hidg_idr_lock); hidg = idr_find(&hidg_idr, iminor(inode)); + if (hidg) + kref_get(&hidg->kref); mutex_unlock(&hidg_idr_lock); if (!hidg) @@ -1056,8 +1070,7 @@ static void hidg_free(struct usb_function *f) hidg = func_to_hidg(f); opts = container_of(f->fi, struct f_hid_opts, func_inst); - kfree(hidg->report_desc); - kfree(hidg); + kref_put(&hidg->kref, hidg_release); mutex_lock(&opts->lock); --opts->refcnt; mutex_unlock(&opts->lock); @@ -1109,6 +1122,8 @@ static struct usb_function *hidg_alloc(struct usb_function_instance *fi) mutex_unlock(&opts->lock); + kref_init(&hidg->kref); + hidg->func.name = "hid"; hidg->func.bind = hidg_bind; hidg->func.unbind = hidg_unbind; From patchwork Mon Oct 28 11:42:28 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Keeping X-Patchwork-Id: 11215177 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 48C0A913 for ; Mon, 28 Oct 2019 11:43:09 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 270D6214D9 for ; Mon, 28 Oct 2019 11:43:09 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=metanate.com header.i=@metanate.com header.b="OD/lK/7E" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2388642AbfJ1LnB (ORCPT ); Mon, 28 Oct 2019 07:43:01 -0400 Received: from dougal.metanate.com ([90.155.101.14]:47937 "EHLO metanate.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1729941AbfJ1LnA (ORCPT ); Mon, 28 Oct 2019 07:43:00 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=simple/simple; d=metanate.com; s=stronger; h=Content-Transfer-Encoding:MIME-Version:References:In-Reply-To: Message-Id:Date:Subject:Cc:To:From:Sender:Reply-To:Content-Type:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=zSqO7Gbt5gFHhPgKnnb70g2Sd1MZYdjH1WNB38pjbSY=; b=OD/lK/7ESCD34gj9SUJGd6S+A4 ZYTY7/ZbACOVtsMl9wTwor4rVAA1Zya0eDOfbk8xlvpWMwTiJHDG/5YnGYEBdR/2M5XzbUT3KStaC wLKmPQVXWdWrM7tZNTM38OmcIJjMxZ1FxtDNqNghIoADwFmAbfDpC9+Ubh4Bq4TH6ET4kv2s5rnkp LtJ+UZbX8F2mYHBBjNjzBkXL6S8on3wsHM7r6s76zWDjqjlTK+6MWGJdJPOGLBEAokyE7sQQd2QIY J9MRlwmYxRzGZDXQ9xTAJJP5M1HliW1fWbQsUG4IQSU+dh8OtOc5yXRWNFOeHTrJ0I+rSEINiYE9z r56Ajgxg==; Received: from dougal.metanate.com ([192.168.88.1] helo=localhost.localdomain) by email.metanate.com with esmtpsa (TLSv1.3:TLS_AES_256_GCM_SHA384:256) (Exim 4.92.3) (envelope-from ) id 1iP3QH-00010o-JZ; Mon, 28 Oct 2019 11:42:57 +0000 From: John Keeping To: linux-usb@vger.kernel.org Cc: Felipe Balbi , Greg Kroah-Hartman , linux-kernel@vger.kernel.org, John Keeping Subject: [PATCH v2 6/6] USB: gadget: f_hid: return ENODEV from read/write after deletion Date: Mon, 28 Oct 2019 11:42:28 +0000 Message-Id: <20191028114228.3679219-7-john@metanate.com> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20191028114228.3679219-1-john@metanate.com> References: <20191024164538.3161474-1-john@metanate.com> <20191028114228.3679219-1-john@metanate.com> MIME-Version: 1.0 X-Authenticated: YES Sender: linux-usb-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-usb@vger.kernel.org If a file descriptor to /dev/hidgN is kept open after the gadget function has been deleted, reading or writing will block indefinitely since no requests will ever be processed. Add a flag to note that the function has been deleted and check this in read & write if there is no other action to take. When setting this flag, also wake up any readers/writers so that they get ENODEV immediately. Signed-off-by: John Keeping --- v2: - No changes drivers/usb/gadget/function/f_hid.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/drivers/usb/gadget/function/f_hid.c b/drivers/usb/gadget/function/f_hid.c index 3d848f7a4cca..a65bdf08ca54 100644 --- a/drivers/usb/gadget/function/f_hid.c +++ b/drivers/usb/gadget/function/f_hid.c @@ -59,6 +59,7 @@ struct f_hidg { struct usb_request *req; struct kref kref; + bool deleted; int minor; struct usb_function func; @@ -271,10 +272,14 @@ static ssize_t f_hidg_read(struct file *file, char __user *buffer, /* wait for at least one buffer to complete */ while (!READ_COND) { spin_unlock_irqrestore(&hidg->read_spinlock, flags); + if (READ_ONCE(hidg->deleted)) + return -ENODEV; + if (file->f_flags & O_NONBLOCK) return -EAGAIN; - if (wait_event_interruptible(hidg->read_queue, READ_COND)) + if (wait_event_interruptible(hidg->read_queue, + READ_COND || READ_ONCE(hidg->deleted))) return -ERESTARTSYS; spin_lock_irqsave(&hidg->read_spinlock, flags); @@ -358,11 +363,14 @@ static ssize_t f_hidg_write(struct file *file, const char __user *buffer, /* write queue */ while (!WRITE_COND) { spin_unlock_irqrestore(&hidg->write_spinlock, flags); + if (READ_ONCE(hidg->deleted)) + return -ENODEV; + if (file->f_flags & O_NONBLOCK) return -EAGAIN; - if (wait_event_interruptible_exclusive( - hidg->write_queue, WRITE_COND)) + if (wait_event_interruptible_exclusive(hidg->write_queue, + WRITE_COND || READ_ONCE(hidg->deleted))) return -ERESTARTSYS; spin_lock_irqsave(&hidg->write_spinlock, flags); @@ -1070,6 +1078,10 @@ static void hidg_free(struct usb_function *f) hidg = func_to_hidg(f); opts = container_of(f->fi, struct f_hid_opts, func_inst); + WRITE_ONCE(hidg->deleted, true); + wake_up(&hidg->read_queue); + wake_up(&hidg->write_queue); + kref_put(&hidg->kref, hidg_release); mutex_lock(&opts->lock); --opts->refcnt;