From: Dan Carpenter <dan.carpenter@oracle.com>
To: decui@microsoft.com
Cc: linux-hyperv@vger.kernel.org
Subject: [bug report] net: mana: Add a driver for Microsoft Azure Network Adapter (MANA)
Date: Wed, 21 Apr 2021 13:06:49 +0300 [thread overview]
Message-ID: <YH/5OQSEbCBvH9ju@mwanda> (raw)
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
next reply other threads:[~2021-04-21 10:07 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-21 10:06 Dan Carpenter [this message]
2021-04-22 8:06 ` [bug report] net: mana: Add a driver for Microsoft Azure Network Adapter (MANA) Dexuan Cui
2021-04-22 9:35 ` Dan Carpenter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YH/5OQSEbCBvH9ju@mwanda \
--to=dan.carpenter@oracle.com \
--cc=decui@microsoft.com \
--cc=linux-hyperv@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).