Re: [PATCH net-next v2 3/3] dinghai: add async event queue for firmware notifications
From: Simon Horman <horms@kernel.org>
Date: 2026-09-14 07:21:11
Also in:
lkml
This is an AI-generated review of your patch. The human sending this email has considered the AI review valid, or at least plausible. Full review at: https://sashiko.dev/#/patchset/20260911115929435mXJzcTF08xlUAdjnT8qqm@zte.com.cn?part=3 --- dinghai: add async event queue for firmware notifications This commit adds an event queue table and an async event queue for firmware notifications. The queue claims an MSI-X vector from the async pool and maps firmware events to notifier chains.
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/ethernet/zte/dinghai/en_pf.c b/drivers/net/ethernet/zte/dinghai/en_pf.c --- a/drivers/net/ethernet/zte/dinghai/en_pf.c +++ b/drivers/net/ethernet/zte/dinghai/en_pf.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -612,12 +612,26 @@ static int zxdh_pf_probe(struct pci_dev *pdev, const struct pci_device_id *id) goto err_irq_table; } + ret = zxdh_pf_eq_table_init(zxdh_dev); + if (ret) { + dev_err(&pdev->dev, "zxdh_pf_eq_table_init failed: %d\n", ret); + goto err_eq_table; + } + + ret = zxdh_pf_eq_table_create(zxdh_dev); + if (ret) { + dev_err(&pdev->dev, "zxdh_pf_eq_table_create failed: %d\n", ret); + goto err_eq_table; + } + devlink_register(devlink); return 0; +err_eq_table: + zxdh_pf_eq_table_destroy(zxdh_dev); err_irq_table: - kvfree(zxdh_dev->irq_table.priv); + zxdh_pf_irq_table_destroy(zxdh_dev); err_cfg_init: zxdh_pf_pci_close(zxdh_dev);
[Severity: Medium] Does this error path leak the modern configuration MMIO mappings? The zxdh_pf_modern_cfg_init() function successfully maps several capabilities. If a subsequent step fails (such as zxdh_pf_eq_table_init() or other device initialization checks), the code jumps to err_eq_table and falls through to err_cfg_init. However, this label only calls zxdh_pf_pci_close() and misses a call to zxdh_pf_modern_cfg_uninit(), leaving the memory mapped.
err_pci_init:
[ ... ]
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/ethernet/zte/dinghai/zxdh_eq.c b/drivers/net/ethernet/zte/dinghai/zxdh_eq.c --- /dev/null +++ b/drivers/net/ethernet/zte/dinghai/zxdh_eq.c
[ ... ]
+static u16 zxdh_eq_event_id_get(struct zxdh_core_dev *zxdh_dev)
+{
+ struct zxdh_pf_dev *pf_dev = zxdh_dev->priv;
+ void __iomem *subchan;
+
+ subchan = pf_dev->pci_ioremap_addr[0] + ZXDH_BAR_MSG_SUBCHAN_RECV;
+
+ return ioread32(subchan) >> 16;
+}[Severity: High] Is there a risk of an out-of-bounds MMIO read here? The device blindly maps BAR 0 based on pci_resource_len() during zxdh_pf_pci_init(). If the hardware or an emulated environment exposes a truncated BAR 0 length that is smaller than 0x2804 bytes, reading from ZXDH_BAR_MSG_SUBCHAN_RECV (offset 0x2800) could exceed the mapped boundaries and cause a page fault or oops. [ ... ]
+int zxdh_pf_eq_table_create(struct zxdh_core_dev *zxdh_dev)
+{
+ struct zxdh_pf_eq_table *pf_eq_table = zxdh_dev->eq_table.priv;
+ struct zxdh_eq_table *table = &zxdh_dev->eq_table;
+ struct zxdh_eq_async *eq = &pf_eq_table->riscv_eq;
+ int err;
+
+ mutex_lock(&table->lock);
+
+ eq->priv = zxdh_dev;
+ eq->irq = zxdh_pf_async_irq_request(zxdh_dev);
+ if (IS_ERR(eq->irq)) {
+ err = PTR_ERR(eq->irq);
+ goto unlock;
+ }
[Severity: High]
Could this lead to an invalid pointer dereference on the cleanup path?
If zxdh_pf_async_irq_request() fails (e.g., due to memory exhaustion), it
returns an error pointer like ERR_PTR(-ENOMEM). This error pointer is
assigned to eq->irq but is not cleared to NULL before jumping to unlock.
When the error propagates, zxdh_pf_probe() jumps to the err_eq_table label
and calls zxdh_pf_eq_table_destroy().
Because eq->irq contains an error pointer, the cleanup check in
zxdh_pf_eq_table_destroy():
if (eq->irq) {
zxdh_irq_detach_nb(eq->irq, &eq->irq_nb);
...
will evaluate to true, and zxdh_irq_detach_nb() will attempt to dereference
the error pointer, causing a kernel panic.
+ + eq->irq_nb.notifier_call = zxdh_eq_async_riscv_int; + err = zxdh_irq_attach_nb(eq->irq, &eq->irq_nb);
[ ... ]