Hello Dexuan Cui,
The patch ca9c54d2d6a5: "net: mana: Add a driver for Microsoft Azure
Network Adapter (MANA)" from Apr 16, 2021, leads to the following
static checker warning:
drivers/net/ethernet/microsoft/mana/hw_channel.c:292 mana_hwc_comp_event()
warn: 'comp_read' unsigned <= 0
drivers/net/ethernet/microsoft/mana/hw_channel.c
281 static void mana_hwc_comp_event(void *ctx, struct gdma_queue *q_self)
282 {
283 struct hwc_rx_oob comp_data = {};
284 struct gdma_comp *completions;
285 struct hwc_cq *hwc_cq = ctx;
286 u32 comp_read, i;
^^^^^^^^^^^^^^^^^
These should be int. I think GCC is encouraging everyone to use u32 but
the u32 type is really a terrible default type to use. It causes a lot
of bugs. This is my second or third signedness bug to deal with today.
Normally int is the best type to use. In the kernel we're mostly using
small numbers where int is fine. In filesystems etc then we're dealing
with huge numbers so u64 is the right type. But it's a very narrow band
between 2 and 4 million where u32 is appropriate.
287
288 WARN_ON_ONCE(hwc_cq->gdma_cq != q_self);
289
290 completions = hwc_cq->comp_buf;
291 comp_read = mana_gd_poll_cq(q_self, completions, hwc_cq->queue_depth);
mana_gd_poll_cq() returns int. It returns -1 on error.
292 WARN_ON_ONCE(comp_read <= 0 || comp_read > hwc_cq->queue_depth);
comp_read can't be less than zero because it's unsigned but the warning
for "comp_read > hwc_cq->queue_depth" will trigger.
293
294 for (i = 0; i < comp_read; ++i) {
^^^^^^^^^^^^^
If "comp_read" was declared as an int then this loop would be a harmless
no-op but because it's UINT_MAX on error then this will crash the system.
295 comp_data = *(struct hwc_rx_oob *)completions[i].cqe_data;
296
297 if (completions[i].is_sq)
298 hwc_cq->tx_event_handler(hwc_cq->tx_event_ctx,
299 completions[i].wq_num,
300 &comp_data);
301 else
302 hwc_cq->rx_event_handler(hwc_cq->rx_event_ctx,
303 completions[i].wq_num,
304 &comp_data);
305 }
306
307 mana_gd_arm_cq(q_self);
308 }
regards,
dan carpenter
