linux-hyperv.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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

             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).