usb_kill_urb warranties that all the handlers are finished when it
returns, but does not protect against threads that might be handling
asynchronously the urb.
For UVC, the function uvc_ctrl_status_event_async() takes care of
control changes asynchronously.
If the code is executed in the following order:
CPU 0 CPU 1
===== =====
uvc_status_complete()
uvc_status_stop()
uvc_ctrl_status_event_work()
uvc_status_start() -> FAIL
Then uvc_status_start will keep failing and this error will be shown:
<4>[ 5.540139] URB 0000000000000000 submitted while active
drivers/usb/core/urb.c:378 usb_submit_urb+0x4c3/0x528
Let's improve the current situation, by not re-submiting the urb if
we are stopping the status event. Also process the queued work
(if any) during stop.
CPU 0 CPU 1
===== =====
uvc_status_complete()
uvc_status_stop()
uvc_status_start()
uvc_ctrl_status_event_work() -> FAIL
Hopefully, with the usb layer protection this should be enough to cover
all the cases.
Cc: stable(a)vger.kernel.org
Fixes: e5225c820c05 ("media: uvcvideo: Send a control event when a Control Change interrupt arrives")
Reviewed-by: Yunke Cao <yunkec(a)chromium.org>
Signed-off-by: Ricardo Ribalda <ribalda(a)chromium.org>
---
uvc: Fix race condition on uvc
Make sure that all the async work is finished when we stop the status urb.
To: Yunke Cao <yunkec(a)chromium.org>
To: Sergey Senozhatsky <senozhatsky(a)chromium.org>
To: Max Staudt <mstaudt(a)google.com>
To: Laurent Pinchart <laurent.pinchart(a)ideasonboard.com>
To: Mauro Carvalho Chehab <mchehab(a)kernel.org>
Cc: linux-media(a)vger.kernel.org
Cc: linux-kernel(a)vger.kernel.org
---
Changes in v4:
- Replace bool with atomic_t to avoid compiler reordering
- First complete the async work and then kill the urb to avoid race (Thanks Laurent!)
- Link to v3: https://lore.kernel.org/r/20221212-uvc-race-v3-0-954efc752c9a@chromium.org
Changes in v3:
- Remove the patch for dev->status, makes more sense in another series, and makes
the zero day less nervous.
- Update reviewed-by (thanks Yunke!).
- Link to v2: https://lore.kernel.org/r/20221212-uvc-race-v2-0-54496cc3b8ab@chromium.org
Changes in v2:
- Add a patch for not kalloc dev->status
- Redo the logic mechanism, so it also works with suspend (Thanks Yunke!)
- Link to v1: https://lore.kernel.org/r/20221212-uvc-race-v1-0-c52e1783c31d@chromium.org
---
drivers/media/usb/uvc/uvc_ctrl.c | 3 +++
drivers/media/usb/uvc/uvc_status.c | 6 ++++++
drivers/media/usb/uvc/uvcvideo.h | 1 +
3 files changed, 10 insertions(+)
diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index c95a2229f4fa..1be6897a7d6d 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -1442,6 +1442,9 @@ static void uvc_ctrl_status_event_work(struct work_struct *work)
uvc_ctrl_status_event(w->chain, w->ctrl, w->data);
+ if (atomic_read(&dev->flush_status))
+ return;
+
/* Resubmit the URB. */
w->urb->interval = dev->int_ep->desc.bInterval;
ret = usb_submit_urb(w->urb, GFP_KERNEL);
diff --git a/drivers/media/usb/uvc/uvc_status.c b/drivers/media/usb/uvc/uvc_status.c
index 7518ffce22ed..4a95850cdc1b 100644
--- a/drivers/media/usb/uvc/uvc_status.c
+++ b/drivers/media/usb/uvc/uvc_status.c
@@ -304,10 +304,16 @@ int uvc_status_start(struct uvc_device *dev, gfp_t flags)
if (dev->int_urb == NULL)
return 0;
+ atomic_set(&dev->flush_status, 0);
return usb_submit_urb(dev->int_urb, flags);
}
void uvc_status_stop(struct uvc_device *dev)
{
+ struct uvc_ctrl_work *w = &dev->async_ctrl;
+
+ atomic_set(&dev->flush_status, 1);
+ if (cancel_work_sync(&w->work))
+ uvc_ctrl_status_event(w->chain, w->ctrl, w->data);
usb_kill_urb(dev->int_urb);
}
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index df93db259312..1274691f157f 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -560,6 +560,7 @@ struct uvc_device {
struct usb_host_endpoint *int_ep;
struct urb *int_urb;
u8 *status;
+ atomic_t flush_status;
struct input_dev *input;
char input_phys[64];
---
base-commit: 0ec5a38bf8499f403f81cb81a0e3a60887d1993c
change-id: 20221212-uvc-race-09276ea68bf8
Best regards,
--
Ricardo Ribalda <ribalda(a)chromium.org>
The patch below does not apply to the 4.9-stable tree.
If someone wants it applied there, or to any other stable or longterm
tree, then please email the backport, including the original git commit
id to <stable(a)vger.kernel.org>.
Possible dependencies:
da522b5fe1a5 ("SUNRPC: Don't leak netobj memory when gss_read_proxy_verf() fails")
5866efa8cbfb ("SUNRPC: Fix svcauth_gss_proxy_init()")
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From da522b5fe1a5f8b7c20a0023e87b52a150e53bf5 Mon Sep 17 00:00:00 2001
From: Chuck Lever <chuck.lever(a)oracle.com>
Date: Sat, 26 Nov 2022 15:55:18 -0500
Subject: [PATCH] SUNRPC: Don't leak netobj memory when gss_read_proxy_verf()
fails
Fixes: 030d794bf498 ("SUNRPC: Use gssproxy upcall for server RPCGSS authentication.")
Signed-off-by: Chuck Lever <chuck.lever(a)oracle.com>
Cc: <stable(a)vger.kernel.org>
Reviewed-by: Jeff Layton <jlayton(a)kernel.org>
diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
index bcd74dddbe2d..9a5db285d4ae 100644
--- a/net/sunrpc/auth_gss/svcauth_gss.c
+++ b/net/sunrpc/auth_gss/svcauth_gss.c
@@ -1162,18 +1162,23 @@ static int gss_read_proxy_verf(struct svc_rqst *rqstp,
return res;
inlen = svc_getnl(argv);
- if (inlen > (argv->iov_len + rqstp->rq_arg.page_len))
+ if (inlen > (argv->iov_len + rqstp->rq_arg.page_len)) {
+ kfree(in_handle->data);
return SVC_DENIED;
+ }
pages = DIV_ROUND_UP(inlen, PAGE_SIZE);
in_token->pages = kcalloc(pages, sizeof(struct page *), GFP_KERNEL);
- if (!in_token->pages)
+ if (!in_token->pages) {
+ kfree(in_handle->data);
return SVC_DENIED;
+ }
in_token->page_base = 0;
in_token->page_len = inlen;
for (i = 0; i < pages; i++) {
in_token->pages[i] = alloc_page(GFP_KERNEL);
if (!in_token->pages[i]) {
+ kfree(in_handle->data);
gss_free_in_token_pages(in_token);
return SVC_DENIED;
}
The patch below does not apply to the 4.14-stable tree.
If someone wants it applied there, or to any other stable or longterm
tree, then please email the backport, including the original git commit
id to <stable(a)vger.kernel.org>.
Possible dependencies:
da522b5fe1a5 ("SUNRPC: Don't leak netobj memory when gss_read_proxy_verf() fails")
5866efa8cbfb ("SUNRPC: Fix svcauth_gss_proxy_init()")
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From da522b5fe1a5f8b7c20a0023e87b52a150e53bf5 Mon Sep 17 00:00:00 2001
From: Chuck Lever <chuck.lever(a)oracle.com>
Date: Sat, 26 Nov 2022 15:55:18 -0500
Subject: [PATCH] SUNRPC: Don't leak netobj memory when gss_read_proxy_verf()
fails
Fixes: 030d794bf498 ("SUNRPC: Use gssproxy upcall for server RPCGSS authentication.")
Signed-off-by: Chuck Lever <chuck.lever(a)oracle.com>
Cc: <stable(a)vger.kernel.org>
Reviewed-by: Jeff Layton <jlayton(a)kernel.org>
diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
index bcd74dddbe2d..9a5db285d4ae 100644
--- a/net/sunrpc/auth_gss/svcauth_gss.c
+++ b/net/sunrpc/auth_gss/svcauth_gss.c
@@ -1162,18 +1162,23 @@ static int gss_read_proxy_verf(struct svc_rqst *rqstp,
return res;
inlen = svc_getnl(argv);
- if (inlen > (argv->iov_len + rqstp->rq_arg.page_len))
+ if (inlen > (argv->iov_len + rqstp->rq_arg.page_len)) {
+ kfree(in_handle->data);
return SVC_DENIED;
+ }
pages = DIV_ROUND_UP(inlen, PAGE_SIZE);
in_token->pages = kcalloc(pages, sizeof(struct page *), GFP_KERNEL);
- if (!in_token->pages)
+ if (!in_token->pages) {
+ kfree(in_handle->data);
return SVC_DENIED;
+ }
in_token->page_base = 0;
in_token->page_len = inlen;
for (i = 0; i < pages; i++) {
in_token->pages[i] = alloc_page(GFP_KERNEL);
if (!in_token->pages[i]) {
+ kfree(in_handle->data);
gss_free_in_token_pages(in_token);
return SVC_DENIED;
}
The patch below does not apply to the 4.9-stable tree.
If someone wants it applied there, or to any other stable or longterm
tree, then please email the backport, including the original git commit
id to <stable(a)vger.kernel.org>.
Possible dependencies:
37e90c374dd1 ("tpm: tpm_crb: Add the missed acpi_put_table() to fix memory leak")
627448e85c76 ("tpm: separate cmd_ready/go_idle from runtime_pm")
e2fb992d82c6 ("tpm: add retry logic")
65520d46a4ad ("tpm: tpm-interface: fix tpm_transmit/_cmd kdoc")
888d867df441 ("tpm: cmd_ready command can be issued only after granting locality")
b3e958ce4c58 ("tpm: Keep CLKRUN enabled throughout the duration of transmit_cmd()")
c382babccba2 ("tpm_tis: Move ilb_base_addr to tpm_tis_data")
fd3ec3663718 ("tpm: move tpm_eventlog.h outside of drivers folder")
cf151a9a44d5 ("tpm: reduce tpm polling delay in tpm_tis_core")
87cdfdd19aef ("tpm: move wait_for_tpm_stat() to respective driver files")
f5357413dbaa ("tpm/tpm_crb: Use start method value from ACPI table directly")
9f3fc7bcddcb ("tpm: replace msleep() with usleep_range() in TPM 1.2/2.0 generic drivers")
bc397085ca97 ("tpm_tis: make ilb_base_addr static")
5e572cab92f0 ("tpm: Enable CLKRUN protocol for Braswell systems")
f128480f3916 ("tpm/tpm_crb: fix priv->cmd_size initialisation")
e24dd9ee5399 ("Merge branch 'next' of git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security")
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From 37e90c374dd11cf4919c51e847c6d6ced0abc555 Mon Sep 17 00:00:00 2001
From: Hanjun Guo <guohanjun(a)huawei.com>
Date: Thu, 17 Nov 2022 19:23:41 +0800
Subject: [PATCH] tpm: tpm_crb: Add the missed acpi_put_table() to fix memory
leak
In crb_acpi_add(), we get the TPM2 table to retrieve information
like start method, and then assign them to the priv data, so the
TPM2 table is not used after the init, should be freed, call
acpi_put_table() to fix the memory leak.
Fixes: 30fc8d138e91 ("tpm: TPM 2.0 CRB Interface")
Cc: stable(a)vger.kernel.org
Signed-off-by: Hanjun Guo <guohanjun(a)huawei.com>
Reviewed-by: Jarkko Sakkinen <jarkko(a)kernel.org>
Signed-off-by: Jarkko Sakkinen <jarkko(a)kernel.org>
diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index 18606651d1aa..5bfb00fc19cf 100644
--- a/drivers/char/tpm/tpm_crb.c
+++ b/drivers/char/tpm/tpm_crb.c
@@ -676,12 +676,16 @@ static int crb_acpi_add(struct acpi_device *device)
/* Should the FIFO driver handle this? */
sm = buf->start_method;
- if (sm == ACPI_TPM2_MEMORY_MAPPED)
- return -ENODEV;
+ if (sm == ACPI_TPM2_MEMORY_MAPPED) {
+ rc = -ENODEV;
+ goto out;
+ }
priv = devm_kzalloc(dev, sizeof(struct crb_priv), GFP_KERNEL);
- if (!priv)
- return -ENOMEM;
+ if (!priv) {
+ rc = -ENOMEM;
+ goto out;
+ }
if (sm == ACPI_TPM2_COMMAND_BUFFER_WITH_ARM_SMC) {
if (buf->header.length < (sizeof(*buf) + sizeof(*crb_smc))) {
@@ -689,7 +693,8 @@ static int crb_acpi_add(struct acpi_device *device)
FW_BUG "TPM2 ACPI table has wrong size %u for start method type %d\n",
buf->header.length,
ACPI_TPM2_COMMAND_BUFFER_WITH_ARM_SMC);
- return -EINVAL;
+ rc = -EINVAL;
+ goto out;
}
crb_smc = ACPI_ADD_PTR(struct tpm2_crb_smc, buf, sizeof(*buf));
priv->smc_func_id = crb_smc->smc_func_id;
@@ -700,17 +705,23 @@ static int crb_acpi_add(struct acpi_device *device)
rc = crb_map_io(device, priv, buf);
if (rc)
- return rc;
+ goto out;
chip = tpmm_chip_alloc(dev, &tpm_crb);
- if (IS_ERR(chip))
- return PTR_ERR(chip);
+ if (IS_ERR(chip)) {
+ rc = PTR_ERR(chip);
+ goto out;
+ }
dev_set_drvdata(&chip->dev, priv);
chip->acpi_dev_handle = device->handle;
chip->flags = TPM_CHIP_FLAG_TPM2;
- return tpm_chip_register(chip);
+ rc = tpm_chip_register(chip);
+
+out:
+ acpi_put_table((struct acpi_table_header *)buf);
+ return rc;
}
static int crb_acpi_remove(struct acpi_device *device)