From patchwork Thu Aug 4 20:32:22 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Viresh Kumar X-Patchwork-Id: 73313 Delivered-To: patch@linaro.org Received: by 10.140.29.52 with SMTP id a49csp1584645qga; Thu, 4 Aug 2016 13:33:36 -0700 (PDT) X-Received: by 10.66.49.137 with SMTP id u9mr28105560pan.72.1470342816605; Thu, 04 Aug 2016 13:33:36 -0700 (PDT) Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id n127si16219942pfn.241.2016.08.04.13.33.36; Thu, 04 Aug 2016 13:33:36 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of stable-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@linaro.org; spf=pass (google.com: best guess record for domain of stable-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=stable-owner@vger.kernel.org; dmarc=fail (p=NONE dis=NONE) header.from=linaro.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965213AbcHDUde (ORCPT + 3 others); Thu, 4 Aug 2016 16:33:34 -0400 Received: from mail-pf0-f172.google.com ([209.85.192.172]:33278 "EHLO mail-pf0-f172.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1759011AbcHDUdd (ORCPT ); Thu, 4 Aug 2016 16:33:33 -0400 Received: by mail-pf0-f172.google.com with SMTP id y134so89751535pfg.0 for ; Thu, 04 Aug 2016 13:32:26 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id; bh=Jkg4NlVWyBTC1xJy9lVNBku/YWp4hOGmVUROpphQoeA=; b=bTc2NSEMZ5Yun1k8LPj3vqot2vzAvbg1mc21w5mAQrKMstVqWmeVugkm4NyaQWQZXl VqfHfew5USBxD+DYKqOZQydWDJJ9Z0J7pS+hvZX2s421QjBOTE1yWoHycsobgU9a4lpf H32zkalzvEPJyfu9TAOkq/mZwNWiCnAHLRMcM= 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; bh=Jkg4NlVWyBTC1xJy9lVNBku/YWp4hOGmVUROpphQoeA=; b=OKFK4BJybhVqBAIUTisN/fi+NF2WVXfKXUJKmpufmduVhlq0YcWS4MG4mVeb/YxQLe wt5HZTLHSytmVEPczYo7LzjW1BU4tYL661vMiMfOoWTHXLa5FJ1j/J8xr+jmxAD3sBsB BQwsVzq3hLUkAYo38mAla1IcJlH65FYmu8BWhKNNWXraDh0nCjeb1C5RXlbgaBQ5TEd1 PV0iJLj1oAJuTQIi8H1TU1O074iu5SYinQ63+RahmRrTyJcNap7iXVzvUDIxIbppBEN9 CcNNPWeM7ZWDUlP/poK3R1F3DCsK1pn1XYi+JNofmImcoQ9ExZ0ILGJ6J/9Of+yCer3V /wMQ== X-Gm-Message-State: AEkoous+wmlU96oKLCUcD7TEMJXz+lNNrmWiTdByzKxgKUXl0ztJ8WesTM73wpWpuPYQgByZ X-Received: by 10.98.201.138 with SMTP id l10mr128587295pfk.77.1470342746320; Thu, 04 Aug 2016 13:32:26 -0700 (PDT) Received: from localhost ([104.132.1.108]) by smtp.gmail.com with ESMTPSA id b68sm22367733pfg.85.2016.08.04.13.32.25 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 04 Aug 2016 13:32:25 -0700 (PDT) From: Viresh Kumar To: Greg Kroah-Hartman Cc: linaro-kernel@lists.linaro.org, alexandru.cornea@intel.com, cristina.moraru@intel.com, Alex Elder , Johan Hovold , Viresh Kumar , "#4 . 4+" , Manu Gautam , Alan Stern , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH V2] usb: hub: Fix unbalanced reference count/memory leak/deadlocks Date: Thu, 4 Aug 2016 13:32:22 -0700 Message-Id: <70e6253f49c0a23aa847925ec0fe502823ac3503.1470342605.git.viresh.kumar@linaro.org> X-Mailer: git-send-email 2.7.4 Sender: stable-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: stable@vger.kernel.org Memory leak and unbalanced reference count: If the hub gets disconnected while the core is still activating it, this can result in leaking memory of few USB structures. This will happen if we have done a kref_get() from hub_activate() and scheduled a delayed work item for HUB_INIT2/3. Now if hub_disconnect() gets called before the delayed work expires, then we will cancel the work from hub_quiesce(), but wouldn't do a kref_put(). And so the unbalance. kmemleak reports this as (with the commit e50293ef9775 backported to 3.10 kernel with other changes, though the same is true for mainline as well): unreferenced object 0xffffffc08af5b800 (size 1024): comm "khubd", pid 73, jiffies 4295051211 (age 6482.350s) hex dump (first 32 bytes): 30 68 f3 8c c0 ff ff ff 00 a0 b2 2e c0 ff ff ff 0h.............. 01 00 00 00 00 00 00 00 00 94 7d 40 c0 ff ff ff ..........}@.... backtrace: [] create_object+0x148/0x2a0 [] kmemleak_alloc+0x80/0xbc [] kmem_cache_alloc_trace+0x120/0x1ac [] hub_probe+0x120/0xb84 [] usb_probe_interface+0x1ec/0x298 [] driver_probe_device+0x160/0x374 [] __device_attach+0x28/0x4c [] bus_for_each_drv+0x78/0xac [] device_attach+0x6c/0x9c [] bus_probe_device+0x28/0xa0 [] device_add+0x324/0x604 [] usb_set_configuration+0x660/0x6cc [] generic_probe+0x44/0x84 [] usb_probe_device+0x54/0x74 [] driver_probe_device+0x160/0x374 [] __device_attach+0x28/0x4c Deadlocks: If the hub gets disconnected early enough (i.e. before INIT2/INIT3 are finished and the init_work is still queued), the core may call hub_quiesce() after acquiring interface device locks and it will wait for the work to be cancelled synchronously. But if the work handler is already running in parallel, it may try to acquire the same interface device lock and this may result in deadlock. Fix both the issues by removing the call to cancel_delayed_work_sync(). CC: #4.4+ Fixes: e50293ef9775 ("USB: fix invalid memory access in hub_activate()") Reported-by: Manu Gautam Signed-off-by: Viresh Kumar Acked-by: Alan Stern --- V1->V2: - Solve the deadlock problem as well - Added Ack from Alan as he was happy with this solution in advance :) - Dropped cancel-work and kref-put from V1 drivers/usb/core/hub.c | 2 -- 1 file changed, 2 deletions(-) -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index bee13517676f..3ccffac0f647 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -1315,8 +1315,6 @@ static void hub_quiesce(struct usb_hub *hub, enum hub_quiescing_type type) struct usb_device *hdev = hub->hdev; int i; - cancel_delayed_work_sync(&hub->init_work); - /* hub_wq and related activity won't re-trigger */ hub->quiescing = 1;