All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: andreyknvl@google.com
Cc: Andrey Konovalov <andreyknvl@google.com>, linux-usb@vger.kernel.org
Subject: [bug report] usb: gadget: add raw-gadget interface
Date: Mon, 6 Apr 2020 13:55:45 +0300	[thread overview]
Message-ID: <20200406105545.GA35744@mwanda> (raw)

Hello Andrey Konovalov,

The patch f2c2e717642c: "usb: gadget: add raw-gadget interface" from
Feb 24, 2020, leads to the following static checker warning:

    drivers/usb/gadget/legacy/raw_gadget.c:102 raw_event_queue_fetch() warn: inconsistent returns 'queue->sema'.
      Locked on  : 96,102
      Unlocked on: 93

drivers/usb/gadget/legacy/raw_gadget.c
    81  static struct usb_raw_event *raw_event_queue_fetch(
    82                                  struct raw_event_queue *queue)
    83  {
    84          unsigned long flags;
    85          struct usb_raw_event *event;
    86  
    87          /*
    88           * This function can be called concurrently. We first check that
    89           * there's at least one event queued by decrementing the semaphore,
    90           * and then take the lock to protect queue struct fields.
    91           */
    92          if (down_interruptible(&queue->sema))
    93                  return NULL;
    94          spin_lock_irqsave(&queue->lock, flags);
    95          if (WARN_ON(!queue->size))
    96                  return NULL;

I'm going to have investigate to see why Smatch doesn't complain that
we have disabled IRQs on this path...  Anyway, this doesn't seem like it
can be correct.  I get that this is a WARN_ON() path, but we're leaving
the computer in a very screwed up state we don't at least enable IRQs.

    97          event = queue->events[0];
    98          queue->size--;
    99          memmove(&queue->events[0], &queue->events[1],
   100                          queue->size * sizeof(queue->events[0]));
   101          spin_unlock_irqrestore(&queue->lock, flags);
   102          return event;
   103  }

regards,
dan carpenter

             reply	other threads:[~2020-04-06 10:55 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-06 10:55 Dan Carpenter [this message]
2020-04-06 13:06 ` [bug report] usb: gadget: add raw-gadget interface Andrey Konovalov
2020-04-06 14:07   ` Dan Carpenter
2020-04-06 16:04     ` Andrey Konovalov

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=20200406105545.GA35744@mwanda \
    --to=dan.carpenter@oracle.com \
    --cc=andreyknvl@google.com \
    --cc=linux-usb@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.