From patchwork Tue Feb 5 18:24:40 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Stultz X-Patchwork-Id: 157520 Delivered-To: patches@linaro.org Received: by 2002:a02:48:0:0:0:0:0 with SMTP id 69csp5486791jaa; Tue, 5 Feb 2019 10:24:44 -0800 (PST) X-Received: by 2002:a17:902:7608:: with SMTP id k8mr1181026pll.245.1549391084395; Tue, 05 Feb 2019 10:24:44 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1549391084; cv=none; d=google.com; s=arc-20160816; b=xH2ZpGa0LauiSwd2MFYv9DRaZAgm0fXKJagnZW20KgLJuctuI0YHzw6RDEkfVOpUp1 DS7lsJewiJrlgdtkCWog3/TCKYUQ7VNg3VeuUtOv6jwh/mByxAXgoupcMGaWxuV/Mlmy 93rkhwDOtIOqCYU9F8Gm2qnSbSa3FCkbeRe+gjihwE++jNHcDseh8khkNyv+X1jdrWnY Yy6wGuOgXPqGxWWyC5QcJAAc4WLL2b0/CZO/+/jQZDvfNAiZBebtKo8fEJWlNuanNu7G MR8pfpekTy74h9/13tlGYLHaJlevFta+hB68HqYZJsSjA9gzJg0NRbAlVvhy5+FSI8HO s0RA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=message-id:date:subject:cc:to:from:dkim-signature; bh=29P8k5ncuXKUQyRURsMYPjVkN9pG7vuj0vNBGghAWIo=; b=0hTLLwGFlJd7yqugZvlafcz9UUk3E2akLVAKgOlNtL7kITP1XqOGCGRHugueafcWDy ewX9gyPhGF4koY+/zmwp6SJ4IBG2MQnTlUx/B2+6JWF2pGvaDxJybdB7q24NMbpuCw0J HMYPY5VedZs5ty7c2EjQuJmYPOrmnrZIBgtLHtBYe2C0gypk2uilBBtk1Jx49FEBw2ja UQ3sqx/3v1x8bUBExKeGTEAWl232y286CaaMpLcYqwk8j0vwEnkAPr49PAyim5i0wlCS wDaI2WJYF5j1owa5P87nGm3aGpM2YSdBolswGQ+FfG6wBfPBYodESsP/1DsGYP4MRlVT 319Q== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=BmBMxRhx; spf=pass (google.com: domain of john.stultz@linaro.org designates 209.85.220.65 as permitted sender) smtp.mailfrom=john.stultz@linaro.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from mail-sor-f65.google.com (mail-sor-f65.google.com. [209.85.220.65]) by mx.google.com with SMTPS id f21sor6075101pgm.40.2019.02.05.10.24.44 for (Google Transport Security); Tue, 05 Feb 2019 10:24:44 -0800 (PST) Received-SPF: pass (google.com: domain of john.stultz@linaro.org designates 209.85.220.65 as permitted sender) client-ip=209.85.220.65; Authentication-Results: mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=BmBMxRhx; spf=pass (google.com: domain of john.stultz@linaro.org designates 209.85.220.65 as permitted sender) smtp.mailfrom=john.stultz@linaro.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id; bh=29P8k5ncuXKUQyRURsMYPjVkN9pG7vuj0vNBGghAWIo=; b=BmBMxRhx8B9FXghORrcyCycY/5e5CDa1od8MIBcw+qLJDuG5XJnLW1Di2x/8Bqb3Jo cLNIl9R+SjiaEGgMA0FRkwMR6cQne5LSsjvsiT/3rF7igl3R/Du5sEGtc6Qt6VxkSvne LrY9W08z4ocgRg08ZMnxbhap6aJwib9JR+sQLyhsY3P3R0tFSg+Y07MwNzKuBTLACCyw XckXyArQGdXT2o1UgeHMD+S3AipZYBn+DyslLU4gCCi/UvJcHFxJ1bk7XzqvXYPy+LLY vM7ePnlGSz/m/NddpPJwdbmbhDIcZIYI4NJQ9Jot/wrEEMW7UxkMsSm03ZkN93w/kALJ 2qaw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=29P8k5ncuXKUQyRURsMYPjVkN9pG7vuj0vNBGghAWIo=; b=FB7dGu00SfMRJwA63na5wY2glIi3Xx1VAlZcw2odGEkYnn0HPXS95YdXEeLb0XXYMm WgwdbIZ4CcVT0lPHlCgnNA5RpNboFNlTz1R1UXqIdNhVM2s4q6pavqZzsnTQvRr5ZsFt xlyKA+tbyOjkD9wGtRolDhiJ9P4kjv6bFz3nk4eF+GCohkL3wpTup4nw0b9261h4So26 hWYJUG0TmShdXosE6eCoVYTAprQB0f51HNimVhEfznYVaNRAJjwgrk8orTaLYdagKfps LcNPEZcwZtbWHVrqiEpBz4sl2802U3TlGe+tgtCoCCNCHniAxpNqJKQEvXBnufx9tTkm b+HA== X-Gm-Message-State: AHQUAuYv+t1IKWSz79kZNoTZ05RXrfrokbh/UrSm5npAOlGedVBbG9QV VfcyFW7vZiNzR+VE+OxRdkfhUvyk X-Google-Smtp-Source: AHgI3IZaGoyes8rmaNzxuDLdGcsypja4ggGI7vJCRf9Pjz7oLWA63vhVOyOZrdP9QRf+eJNTK9lR8A== X-Received: by 2002:a65:6094:: with SMTP id t20mr1684265pgu.285.1549391083808; Tue, 05 Feb 2019 10:24:43 -0800 (PST) Return-Path: Received: from localhost.localdomain ([2601:1c2:680:1319:4e72:b9ff:fe99:466a]) by smtp.gmail.com with ESMTPSA id b194sm6932326pga.45.2019.02.05.10.24.42 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Tue, 05 Feb 2019 10:24:42 -0800 (PST) From: John Stultz To: lkml Cc: John Stultz , Alan Stern , Felipe Balbi , Zeng Tao , Jack Pham , Thinh Nguyen , Chen Yu , Jerry Zhang , Lars-Peter Clausen , Vincent Pelletier , Andrzej Pietrasiewicz , Greg Kroah-Hartman , Linux USB List Subject: [RFC][PATCH] usb: f_fs: Avoid crash due to out-of-scope stack ptr access Date: Tue, 5 Feb 2019 10:24:40 -0800 Message-Id: <1549391080-7413-1-git-send-email-john.stultz@linaro.org> X-Mailer: git-send-email 2.7.4 Since the 5.0 merge window opened, I've been seeing frequent crashes on suspend and reboot with the trace: [ 36.911170] Unable to handle kernel paging request at virtual address ffffff801153d660 [ 36.912769] Unable to handle kernel paging request at virtual address ffffff800004b564 ... [ 36.950666] Call trace: [ 36.950670] queued_spin_lock_slowpath+0x1cc/0x2c8 [ 36.950681] _raw_spin_lock_irqsave+0x64/0x78 [ 36.950692] complete+0x28/0x70 [ 36.950703] ffs_epfile_io_complete+0x3c/0x50 [ 36.950713] usb_gadget_giveback_request+0x34/0x108 [ 36.950721] dwc3_gadget_giveback+0x50/0x68 [ 36.950723] dwc3_thread_interrupt+0x358/0x1488 [ 36.950731] irq_thread_fn+0x30/0x88 [ 36.950734] irq_thread+0x114/0x1b0 [ 36.950739] kthread+0x104/0x130 [ 36.950747] ret_from_fork+0x10/0x1c I isolated this down to in ffs_epfile_io(): https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/usb/gadget/function/f_fs.c#n1065 Where the completion done is setup on the stack: DECLARE_COMPLETION_ONSTACK(done); Then later we setup a request and queue it, and wait for it: if (unlikely(wait_for_completion_interruptible(&done))) { /* * To avoid race condition with ffs_epfile_io_complete, * dequeue the request first then check * status. usb_ep_dequeue API should guarantee no race * condition with req->complete callback. */ usb_ep_dequeue(ep->ep, req); interrupted = ep->status < 0; } The problem is, that we end up being interrupted, dequeue the request, and exit. But then the irq triggers and we try calling complete() on the context pointer which points to now random stack space, which results in the panic. Alan Stern pointed out there is a bug here, in that the snippet above "assumes that usb_ep_dequeue() waits until the request has been completed." And that: wait_for_completion(&done); Is needed right after the usb_ep_dequeue(). Thus this patch implements that change. With it I no longer see the crashes on suspend or reboot. This issue seems to have been uncovered by behavioral changes in the dwc3 driver in commit fec9095bdef4e ("usb: dwc3: gadget: remove wait_end_transfer"). Cc: Alan Stern Cc: Felipe Balbi Cc: Zeng Tao Cc: Jack Pham Cc: Thinh Nguyen Cc: Chen Yu Cc: Jerry Zhang Cc: Lars-Peter Clausen Cc: Vincent Pelletier Cc: Andrzej Pietrasiewicz Cc: Greg Kroah-Hartman Cc: Linux USB List Suggested-by: Alan Stern Signed-off-by: John Stultz --- drivers/usb/gadget/function/f_fs.c | 1 + 1 file changed, 1 insertion(+) -- 2.7.4 diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c index 1e54304..0f8d16d 100644 --- a/drivers/usb/gadget/function/f_fs.c +++ b/drivers/usb/gadget/function/f_fs.c @@ -1082,6 +1082,7 @@ static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data) * condition with req->complete callback. */ usb_ep_dequeue(ep->ep, req); + wait_for_completion(&done); interrupted = ep->status < 0; }