From patchwork Fri Nov 20 20:19:02 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adi Ratiu X-Patchwork-Id: 7671111 X-Patchwork-Delegate: jikos@jikos.cz Return-Path: X-Original-To: patchwork-linux-input@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id ECF1E9F1C2 for ; Fri, 20 Nov 2015 20:19:31 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 2305F2043C for ; Fri, 20 Nov 2015 20:19:31 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 434EC20430 for ; Fri, 20 Nov 2015 20:19:30 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1163358AbbKTUTI (ORCPT ); Fri, 20 Nov 2015 15:19:08 -0500 Received: from mail-wm0-f48.google.com ([74.125.82.48]:34171 "EHLO mail-wm0-f48.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1163342AbbKTUTG (ORCPT ); Fri, 20 Nov 2015 15:19:06 -0500 Received: by wmvv187 with SMTP id v187so87562364wmv.1 for ; Fri, 20 Nov 2015 12:19:04 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=adirat-com.20150623.gappssmtp.com; s=20150623; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=7LvnozbojOa709/VNFcBXbAaBjADMnkvrirfLLN9bLA=; b=duH2WNBvZxMfA2xArxEjKHboD6NjZZwv56ulXAf7GvL+G8ftMCUon8zBb2bCNPBiNL 2NOqJNqssBfhZ3AWUjEtOBc4S97bR7FSKCrjblajfrHgXZLIlIZRLQw0c0uUlIOG2dL0 dG3NJWOVUObioBYBNH8N88DSII/wU3c7FdA/xWVtyrCiEAc/6ijmf7uaKASjByoW2sb5 0ok4XsZ9Mi4maPacigGfHPUA7uYo4MxQNPaRfJiq4hn9GUP3NBeSbgb29zTdw1406zE4 Hq8xuJnrChE3oOwpihXTbcAgHOOPcXL0R+JNa24l6WIJri6hP0YNbDxR7GRuRI6TPqiO z0Ig== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=7LvnozbojOa709/VNFcBXbAaBjADMnkvrirfLLN9bLA=; b=XXfuPuK4rz/QTvMSBu81q1o3kb+xX9TgpGoAn3ITti+e2HdlnP5HxrzAuEw2Ax42sb /LO/oaExO9j/MMcfrdNYn1uTyUPOhKOSzBEWo0dn3PqBRIdjMBpsU3pfpe8zYAQiI6Ak xgezvdyxlVmoPK+d0Wr3dSBQgTaeDf0l2szhURbo6fCE6CtIJCySMnc9kIG4NfxI8T8w VLcUzGi6rv3cI0Xpncl/AVqWP9rqlxHcE5a/S/QpQrL7wp85r9MF81NKG92RcijjVbTJ 6anoVGQHExSMOkdI6MHk+geZDLhmzrRvubCPbS1zfTK7edD7lSUaeH9pFCh2koTPidql NIHg== X-Gm-Message-State: ALoCoQmLtXW2/GrR4h5EkOLLC9RMA4kjzgslIuwbznIi4f8NApJuv3kdEtWj9pz/rGqOlK6Xl0S9 X-Received: by 10.28.223.212 with SMTP id w203mr1966378wmg.88.1448050744382; Fri, 20 Nov 2015 12:19:04 -0800 (PST) Received: from adipc.lan ([188.24.12.4]) by smtp.gmail.com with ESMTPSA id v4sm1052254wjx.18.2015.11.20.12.19.02 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Fri, 20 Nov 2015 12:19:03 -0800 (PST) From: Ioan-Adrian Ratiu To: jikos@kernel.org Cc: pinglinux@gmail.com, linux-usb@vger.kernel.org, linux-input@vger.kernel.org, linux-kernel@vger.kernel.org, joshc@ni.com Subject: [PATCH v2] hid: usbhid: hid-core: fix recursive deadlock Date: Fri, 20 Nov 2015 22:19:02 +0200 Message-Id: <1448050742-10777-1-git-send-email-adi@adirat.com> X-Mailer: git-send-email 2.6.3 In-Reply-To: <1447874755-8673-1-git-send-email-adi@adirat.com> References: <1447874755-8673-1-git-send-email-adi@adirat.com> Sender: linux-input-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-input@vger.kernel.org X-Spam-Status: No, score=-7.3 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD,T_DKIM_INVALID,UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP The critical section protected by usbhid->lock in hid_ctrl() is too big and because of this it causes a recursive deadlock. "Too big" means the case statement and the call to hid_input_report() do not need to be protected by the spinlock (no URB operations are done inside them). The deadlock happens because in certain rare cases drivers try to grab the lock while handling the ctrl irq which grabs the lock before them as described above. For example newer wacom tablets like 056a:033c try to reschedule proximity reads from wacom_intuos_schedule_prox_event() calling hid_hw_request() -> usbhid_request() -> usbhid_submit_report() which tries to grab the usbhid lock already held by hid_ctrl(). There are two ways to get out of this deadlock: 1. Make the drivers work "around" the ctrl critical region, in the wacom case for ex. by delaying the scheduling of the proximity read request itself to a workqueue. 2. Shrink the critical region so the usbhid lock protects only the instructions which modify usbhid state, calling hid_input_report() with the spinlock unlocked, allowing the device driver to grab the lock first, finish and then grab the lock afterwards in hid_ctrl(). This patch implements the 2nd solution. Signed-off-by: Ioan-Adrian Ratiu --- drivers/hid/usbhid/hid-core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c index 36712e9..5dd426f 100644 --- a/drivers/hid/usbhid/hid-core.c +++ b/drivers/hid/usbhid/hid-core.c @@ -477,8 +477,6 @@ static void hid_ctrl(struct urb *urb) struct usbhid_device *usbhid = hid->driver_data; int unplug = 0, status = urb->status; - spin_lock(&usbhid->lock); - switch (status) { case 0: /* success */ if (usbhid->ctrl[usbhid->ctrltail].dir == USB_DIR_IN) @@ -498,6 +496,8 @@ static void hid_ctrl(struct urb *urb) hid_warn(urb->dev, "ctrl urb status %d received\n", status); } + spin_lock(&usbhid->lock); + if (unplug) { usbhid->ctrltail = usbhid->ctrlhead; } else {