Hello Mathias Nyman,
The patch c99b38c41234: "xhci: add support to allocate several
interrupters" from Jan 2, 2024 (linux-next), leads to the following
Smatch static checker warning:
drivers/usb/host/xhci-mem.c:2331 xhci_add_interrupter()
warn: array off by one? 'xhci->interrupters[intr_num]'
drivers/usb/host/xhci-mem.c
2318 static int
2319 xhci_add_interrupter(struct xhci_hcd *xhci, struct xhci_interrupter *ir,
2320 unsigned int intr_num)
2321 {
2322 u64 erst_base;
2323 u32 erst_size;
2324
2325 if (intr_num > xhci->max_interrupters) {
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This check predates your commit.
2326 xhci_warn(xhci, "Can't add interrupter %d, max interrupters %d\n",
2327 intr_num, xhci->max_interrupters);
2328 return -EINVAL;
2329 }
2330
--> 2331 if (xhci->interrupters[intr_num]) {
^^^^^^^^^^^^^^^^^^^^^^^^^^^^
But, yeah, it's an off by one. This is allocated in xhci_mem_init().
2332 xhci_warn(xhci, "Interrupter %d\n already set up", intr_num);
2333 return -EINVAL;
2334 }
2335
2336 xhci->interrupters[intr_num] = ir;
2337 ir->intr_num = intr_num;
2338 ir->ir_set = &xhci->run_regs->ir_set[intr_num];
However, potentially there was already a bug here? Normally "max" type
names are inclusive and "number" or "count" type names are the count.
So maybe > xhci->max_interrupters was correct and we should allocated 1
more element. But the code is kind of mixed with regards to whether
it's a max or a count and I can't be sure one way or the other.
2339
2340 /* set ERST count with the number of entries in the segment table */
2341 erst_size = readl(&ir->ir_set->erst_size);
2342 erst_size &= ERST_SIZE_MASK;
2343 erst_size |= ir->event_ring->num_segs;
2344 writel(erst_size, &ir->ir_set->erst_size);
2345
2346 erst_base = xhci_read_64(xhci, &ir->ir_set->erst_base);
2347 erst_base &= ERST_BASE_RSVDP;
2348 erst_base |= ir->erst.erst_dma_addr & ~ERST_BASE_RSVDP;
2349 xhci_write_64(xhci, erst_base, &ir->ir_set->erst_base);
2350
2351 /* Set the event ring dequeue address of this interrupter */
2352 xhci_set_hc_event_deq(xhci, ir);
2353
2354 return 0;
2355 }
regards,
dan carpenter
