[PATCH v3] usb: atm: cxacru: fix NULL deref of atm_dev on sysfs writes
From: Jie Wang
Date: Mon Sep 21 2026 - 21:44:22 EST
The adsl_state and adsl_config sysfs attributes are created via
dev_groups, so the driver core exposes them as soon as ->probe()
returns. At that point instance->atm_dev can still be NULL:
usbatm_heavy_init() waits only for the heavy-init kthread to start, and
atm_dev is assigned later, in usbatm_atm_init(). A write in that window
passes the "instance == NULL" check and then dereferences the NULL
atm_dev: atm_err()/atm_info() expand to
instance->usbatm->atm_dev->number, and a poll/start request also calls
cxacru_poll_status() directly, which touches atm_dev via
atm_dev_signal_change() and atm_dev->link_rate.
Log via usb_err()/usb_info() instead; they use the USB interface
device, which stays valid for the whole write. Reject a poll/start
request with -ENODEV before poll_state is advanced to CXPOLL_POLLING,
so that cxacru_atm_start() still starts polling once atm_dev is ready.
Fixes: e605c30977bb ("USB: atm: cxacru: convert to use dev_groups")
Assisted-by: Gemini:gemini-3.6-flash Gemini:gemini-3.1-pro-preview syzbot
Reported-by: syzbot+9b195c4f412ea5c4e56a@xxxxxxxxxxxxxxxxxxxxxxxxx
Closes: https://syzkaller.appspot.com/bug?extid=9b195c4f412ea5c4e56a
Tested-by: syzbot+9b195c4f412ea5c4e56a@xxxxxxxxxxxxxxxxxxxxxxxxx
Signed-off-by: Jie Wang <jie.wang@xxxxxxxxx>
---
v3:
- Guard in adsl_state_store() instead of cxacru_poll_status(); the v2
guard left poll_state wedged at CXPOLL_POLLING with no work queued, so
polling never restarted (Sashiko).
- Restore the Assisted-by tag.
v2:
- Drop the ->atm_dev entry gate (an unsynchronized TOCTOU); remove the
->atm_dev dereference from the logging paths instead.
v1: https://lore.kernel.org/all/f3ef0a67-79d7-47da-be6e-7e82d92e68fe@xxxxxxxxxxxxxxx/
v2: https://lore.kernel.org/all/20260908150203.2119844-1-jie.wang@xxxxxxxxx/
drivers/usb/atm/cxacru.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/atm/cxacru.c b/drivers/usb/atm/cxacru.c
index 636f7886fc260..501a51b27e466 100644
--- a/drivers/usb/atm/cxacru.c
+++ b/drivers/usb/atm/cxacru.c
@@ -356,7 +356,7 @@ static ssize_t adsl_state_store(struct device *dev,
if (!strcmp(str_cmd, "stop") || !strcmp(str_cmd, "restart")) {
ret = cxacru_cm(instance, CM_REQUEST_CHIP_ADSL_LINE_STOP, NULL, 0, NULL, 0);
if (ret < 0) {
- atm_err(instance->usbatm, "change adsl state:"
+ usb_err(instance->usbatm, "change adsl state:"
" CHIP_ADSL_LINE_STOP returned %d\n", ret);
ret = -EIO;
@@ -376,7 +376,7 @@ static ssize_t adsl_state_store(struct device *dev,
if (!strcmp(str_cmd, "start") || !strcmp(str_cmd, "restart")) {
ret = cxacru_cm(instance, CM_REQUEST_CHIP_ADSL_LINE_START, NULL, 0, NULL, 0);
if (ret < 0) {
- atm_err(instance->usbatm, "change adsl state:"
+ usb_err(instance->usbatm, "change adsl state:"
" CHIP_ADSL_LINE_START returned %d\n", ret);
ret = -EIO;
@@ -396,6 +396,15 @@ static ssize_t adsl_state_store(struct device *dev,
poll = -1;
}
+ /* cxacru_poll_status() below dereferences atm_dev, which may not be
+ * set up yet; reject before poll_state is advanced so that
+ * cxacru_atm_start() can still start polling once it is.
+ */
+ if (poll == CXPOLL_POLLING && !instance->usbatm->atm_dev) {
+ ret = -ENODEV;
+ poll = -1;
+ }
+
if (poll == CXPOLL_POLLING) {
mutex_lock(&instance->poll_state_serialize);
switch (instance->poll_state) {
@@ -481,7 +490,7 @@ static ssize_t adsl_config_store(struct device *dev,
ret = cxacru_cm(instance, CM_REQUEST_CARD_DATA_SET,
(u8 *) data, 4 + num * 8, NULL, 0);
if (ret < 0) {
- atm_err(instance->usbatm,
+ usb_err(instance->usbatm,
"set card data returned %d\n", ret);
return -EIO;
}
@@ -490,7 +499,7 @@ static ssize_t adsl_config_store(struct device *dev,
snprintf(log + tmp*12, 13, " %02x=%08x",
le32_to_cpu(data[tmp * 2 + 1]),
le32_to_cpu(data[tmp * 2 + 2]));
- atm_info(instance->usbatm, "config%s\n", log);
+ usb_info(instance->usbatm, "config%s\n", log);
num = 0;
}
}
--
2.43.0