Message ID | 20230916-topic-powermate_use_after_free-v1-1-2ffa46652869@gmail.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:612c:172:b0:3f2:4152:657d with SMTP id h50csp1934399vqi; Sat, 16 Sep 2023 19:19:55 -0700 (PDT) X-Google-Smtp-Source: AGHT+IFGDVEMa8X1usxkj23BQmWqdC/pWikDGX3DTwp70Fv9DDqJBZI566vOUMq+x6xQ9Mi4n81K X-Received: by 2002:a05:6358:921d:b0:142:d678:f708 with SMTP id d29-20020a056358921d00b00142d678f708mr6508854rwb.19.1694917195489; Sat, 16 Sep 2023 19:19:55 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1694917195; cv=none; d=google.com; s=arc-20160816; b=M3ARsDQapiBT9y6rOqBE/JudiZ/aLTdL1caWucTd8+r85xyl9jXNug0ARnVS0yWVBF CQxjbLP3ZMXWk45l4NhBq1BrXzrczPEkK4vnYKAUocvTD5N0okwkqG9mcUTpU0NsE5Yu +baXr/RCPxZ89knisL/Per2O1dKf9AwbHjfWUh1b+dZcmyYT2Hgdo2CVdNXNs0KCYrvk aqMFCAXA9OxBWyy2tOQ9TawGH+V+uAcNIknGIYxe/jLIkbWeewUvkU8TKbwhT2eZbZGM ktu8iDU3/QQtLhTvdhAK7UmWH/3KL82DpqeNhYQUoCQ0DZa7LLHA+7xSB9SgKzRwm4zd 1R+Q== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:cc:to:message-id:content-transfer-encoding :mime-version:subject:date:from:dkim-signature; bh=T4Wi30eAqxi74z6532D+2PHxNulQWSGewYfam9dn07c=; fh=HbD8I8C32Kws+Jm1NRkTRbXmoSd+4wRU1XqN5k6Wag0=; b=qbfhwvaO9fY3pDMWXjnFVZ21qD3b5jW300ZMLOsJ0kfIVOLzWt3QVQbKzNnZEgJfRB asGUFkBrx8GCi0FsVj5QpuQAt0H46JYHP0KcbyvoC/1uDx4U7Ge1+5mdDexQF4aAmxr1 1XM4Xqvi4s8bmBjv5hcrKBOk13MV8WHQ0Cy55vpWYOw8mBX7o1bod8Q0cBcgdpu46cUd SKKgLE69GrbA4hUjF/jIAq/2+Vs91qvBA8aha6Bkhecp2Cv862AlZBogrmFOaWWNNvoa KomNy/iAMov5Vo5OimAtU/nJYYI2RdsOBhQMAVfLlyNup3IJnQ3BEiSd8C9bhpOQAflJ +mKQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20230601 header.b=LmN+8m42; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:5 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: from groat.vger.email (groat.vger.email. [2620:137:e000::3:5]) by mx.google.com with ESMTPS id k7-20020a170902c40700b001b3c63eba76si5990698plk.492.2023.09.16.19.19.55 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 16 Sep 2023 19:19:55 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:5 as permitted sender) client-ip=2620:137:e000::3:5; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20230601 header.b=LmN+8m42; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:5 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by groat.vger.email (Postfix) with ESMTP id 844C0809A7A6; Sat, 16 Sep 2023 14:36:46 -0700 (PDT) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.10 at groat.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232389AbjIPV2k (ORCPT <rfc822;toshivichauhan@gmail.com> + 29 others); Sat, 16 Sep 2023 17:28:40 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:60724 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229843AbjIPV2a (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Sat, 16 Sep 2023 17:28:30 -0400 Received: from mail-lj1-x22d.google.com (mail-lj1-x22d.google.com [IPv6:2a00:1450:4864:20::22d]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 1578D1A5; Sat, 16 Sep 2023 14:28:25 -0700 (PDT) Received: by mail-lj1-x22d.google.com with SMTP id 38308e7fff4ca-2bf8b9c5ca0so53240641fa.0; Sat, 16 Sep 2023 14:28:24 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1694899703; x=1695504503; darn=vger.kernel.org; h=cc:to:message-id:content-transfer-encoding:mime-version:subject :date:from:from:to:cc:subject:date:message-id:reply-to; bh=T4Wi30eAqxi74z6532D+2PHxNulQWSGewYfam9dn07c=; b=LmN+8m42GpPmPk1MEuEXqLPcSVL2W86zmpNXIn+vxPUi6WxX+tfavc0fkqzw4yEiUI ytyvWxsB6q5vepszHc95pMYpaIPDvEE7HSCOXRQQ6hU5cKAbn0bMOPUBCvDqNjK87PaW Fd8yrEg8NdHqoVp04bvebCl+BO8o3H36KgbwWL0J8sbjGn8QGx4WlTbnrLJBWy/rTKzC 3V3JcHdAQQUHkl44ARQBXg2VnANGctQcFbNzbX4Ik0eQPn3c6bn0GoI2/alprBfJD5gq bO/Whg/kW38sD6F5VqTEqUSO05oqLGDFhAWbdJF5XJzRyoQjgSB01xddWCGReXQ6QJSe xqHg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1694899703; x=1695504503; h=cc:to:message-id:content-transfer-encoding:mime-version:subject :date:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=T4Wi30eAqxi74z6532D+2PHxNulQWSGewYfam9dn07c=; b=xHYXjQ0T0Z9vUEMtXXzDEovqCPGtxhtB1WLScj863h6SFI2duz0qbPxEV4EqnNTXOG nWCKhx6yquVrxkLjErDkO7z5OBFEl6EDdeCqqt7UYsN3WJoKFCr9iAHHVE9fPfJLH/vA mGTKQGPDSwQ2B1ZBomNDXwufWhMY4rAcuyEtyRGLdrS2bk0p0uofgE/HHq4eLv+vfnxA CHV1CJKhVbmSj+DkmxPn0MkO1W/w7xwcZUfw5ihjLA00zI0IuA+ehRRJq/b0vESVDMwm lIhBeDCOZgBA8C2N/UTQ8CGeBX9sShfHHVGl49x1TBJZ+MhOguRzEpfpQdfQ7Wpv4et4 yybQ== X-Gm-Message-State: AOJu0Yx2eumUP0UflrQ7fKiXn/vD2FtaCXEsmMyYAx2ocBppkKRi9/UG jGvgyn270QWXFW+ydQMslSdfpIM6sKCXkA== X-Received: by 2002:a2e:950d:0:b0:2bc:dcd6:97b1 with SMTP id f13-20020a2e950d000000b002bcdcd697b1mr4495087ljh.47.1694899702511; Sat, 16 Sep 2023 14:28:22 -0700 (PDT) Received: from [127.0.1.1] (2a02-8389-41b4-ce80-c1aa-e5ad-22b7-62c7.cable.dynamic.v6.surfer.at. [2a02:8389:41b4:ce80:c1aa:e5ad:22b7:62c7]) by smtp.gmail.com with ESMTPSA id cw17-20020a170906c79100b009a1a5a7ebacsm4117567ejb.201.2023.09.16.14.28.21 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 16 Sep 2023 14:28:22 -0700 (PDT) From: Javier Carrasco <javier.carrasco.cruz@gmail.com> Date: Sat, 16 Sep 2023 23:28:18 +0200 Subject: [PATCH] Input: powermate - fix use-after-free in powermate_config_complete MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Message-Id: <20230916-topic-powermate_use_after_free-v1-1-2ffa46652869@gmail.com> X-B4-Tracking: v=1; b=H4sIAPEdBmUC/x2NQQrDIBQFrxL+uoImkGCvUkowv89GSFW+pi2E3 L3S5cxi5qACCSh07Q4SvEMJKTYwl454dfEJFR6Nqdf9oK0ZVU05sMrpA3m5inkvmJ2vkNkLoHj SA092tGwNtcjiCtQiLvLaMnHftiazwIfv/3q7n+cPRBKSgYUAAAA= To: Dmitry Torokhov <dmitry.torokhov@gmail.com> Cc: linux-input@vger.kernel.org, linux-kernel@vger.kernel.org, Javier Carrasco <javier.carrasco.cruz@gmail.com>, syzbot+0434ac83f907a1dbdd1e@syzkaller.appspotmail.com X-Mailer: b4 0.12.0 X-Developer-Signature: v=1; a=ed25519-sha256; t=1694899701; l=1588; i=javier.carrasco.cruz@gmail.com; s=20230509; h=from:subject:message-id; bh=MOtjOjGctqmbOeILqG8GCyG823EYZNV4T1oZhE/JWNw=; b=/O5dX8BkYFUeoX9xnZzPE9IPokF/qf6mHmEcyfIpLse+q6YXom7Nt8G4F10CsHR9SJCfx/jhz kCz53G0RikvDh3d22UfoIONC6yYDhx/spwHmD1WKn9dYaHuIDuu3xZv X-Developer-Key: i=javier.carrasco.cruz@gmail.com; a=ed25519; pk=tIGJV7M+tCizagNijF0eGMBGcOsPD+0cWGfKjl4h6K8= X-Spam-Status: No, score=-0.6 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on groat.vger.email Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (groat.vger.email [0.0.0.0]); Sat, 16 Sep 2023 14:36:46 -0700 (PDT) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1777249492977324266 X-GMAIL-MSGID: 1777249492977324266 |
Series |
Input: powermate - fix use-after-free in powermate_config_complete
|
|
Commit Message
Javier Carrasco
Sept. 16, 2023, 9:28 p.m. UTC
syzbot has found a use-after-free bug [1] in the powermate driver. This
happens when the device is disconnected, which leads to a memory free
from the powermate_device struct.
When an asynchronous control message completes after the kfree and its
callback is invoked, the lock does not exist anymore and hence the bug.
Return immediately if the URB status is -ESHUTDOWN (the actual status
that triggered this bug) or -ENOENT, avoiding any access to potentially
freed memory.
[1] https://syzkaller.appspot.com/bug?extid=0434ac83f907a1dbdd1e
Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
Reported-by: syzbot+0434ac83f907a1dbdd1e@syzkaller.appspotmail.com
---
drivers/input/misc/powermate.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
---
base-commit: 0bb80ecc33a8fb5a682236443c1e740d5c917d1d
change-id: 20230916-topic-powermate_use_after_free-c703c7969c91
Best regards,
Comments
On Mon, Sep 18, 2023 at 06:51:49AM +0200, Javier Carrasco Cruz wrote: > Hi, > > There's an obvious error in the patch I introduced when cleaningup > (urb->status should be used instead of just status). I will send a v2. I think what we need is call to usb_kill_urb(pm->config) in powermate_disconnect(), right after call to input_unregister_device(). Thanks.
Hi Javier, kernel test robot noticed the following build errors: [auto build test ERROR on 0bb80ecc33a8fb5a682236443c1e740d5c917d1d] url: https://github.com/intel-lab-lkp/linux/commits/Javier-Carrasco/Input-powermate-fix-use-after-free-in-powermate_config_complete/20230917-052943 base: 0bb80ecc33a8fb5a682236443c1e740d5c917d1d patch link: https://lore.kernel.org/r/20230916-topic-powermate_use_after_free-v1-1-2ffa46652869%40gmail.com patch subject: [PATCH] Input: powermate - fix use-after-free in powermate_config_complete config: powerpc-ppc6xx_defconfig (https://download.01.org/0day-ci/archive/20230921/202309210232.d7MpKEIm-lkp@intel.com/config) compiler: powerpc-linux-gcc (GCC) 11.3.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230921/202309210232.d7MpKEIm-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202309210232.d7MpKEIm-lkp@intel.com/ All errors (new ones prefixed by >>): drivers/input/misc/powermate.c: In function 'powermate_config_complete': >> drivers/input/misc/powermate.c:201:21: error: 'status' undeclared (first use in this function); did you mean 'kstatfs'? 201 | if (status == -ENOENT || status == -ESHUTDOWN) | ^~~~~~ | kstatfs drivers/input/misc/powermate.c:201:21: note: each undeclared identifier is reported only once for each function it appears in vim +201 drivers/input/misc/powermate.c 192 193 /* Called when our asynchronous control message completes. We may need to issue another immediately */ 194 static void powermate_config_complete(struct urb *urb) 195 { 196 struct powermate_device *pm = urb->context; 197 unsigned long flags; 198 199 if (urb->status) { 200 printk(KERN_ERR "powermate: config urb returned %d\n", urb->status); > 201 if (status == -ENOENT || status == -ESHUTDOWN) 202 return; 203 } 204 205 spin_lock_irqsave(&pm->lock, flags); 206 powermate_sync_state(pm); 207 spin_unlock_irqrestore(&pm->lock, flags); 208 } 209
Hi Dmitry, On 19.09.23 00:10, Dmitry Torokhov wrote: > On Mon, Sep 18, 2023 at 06:51:49AM +0200, Javier Carrasco Cruz wrote: >> Hi, >> >> There's an obvious error in the patch I introduced when cleaningup >> (urb->status should be used instead of just status). I will send a v2. > > I think what we need is call to usb_kill_urb(pm->config) in > powermate_disconnect(), right after call to input_unregister_device(). > > Thanks. > That is definitely a more meaningful and elegant solution, so I will check it out and eventually send a v2 with it if everything seems ok. On the other hand usb_kill_urb() is already used on pm->irq before calling input_unregister_device(), so I would move the existing usb_kill_urb to have both calls right after the unregister_device call for code consistency, if that is alright. Thanks and best regards.
diff --git a/drivers/input/misc/powermate.c b/drivers/input/misc/powermate.c index c1c733a9cb89..f61333fea35f 100644 --- a/drivers/input/misc/powermate.c +++ b/drivers/input/misc/powermate.c @@ -196,8 +196,11 @@ static void powermate_config_complete(struct urb *urb) struct powermate_device *pm = urb->context; unsigned long flags; - if (urb->status) + if (urb->status) { printk(KERN_ERR "powermate: config urb returned %d\n", urb->status); + if (status == -ENOENT || status == -ESHUTDOWN) + return; + } spin_lock_irqsave(&pm->lock, flags); powermate_sync_state(pm);