Am Dienstag, den 22.08.2017, 15:11 +0300 schrieb Anton Volkov:
> Hello.
>
> Judging by the code of cypress_m8.c some functions are considered to be
> capable of working concurrently with other functions, e.g. cypress_open.
> There are, however, entities that are protected by the locks at one
> place and not protected in another. Lines are given using the info from
> Linux kernel v4.12. Example:
>
> cypress_send
> spin_lock_irqsave
> priv->write_urb_in_use = 1;
> spin_lock_irqrestore
> (cypress_m8.c: lines 761-763)
> ...
> if (result) {
> priv->write_urb_in_use = 0; //without lock protection
> (cypress_m8.c: line 783)
> }
>
> Is it a bug?
Yes, but not of the kind you describe.
The transition from "not in use" to "in use" must be guarded by
a lock, because it may be contended.
But if that transition is properly guarded, you already know
that there can be only user. He can theoretically give up
the resource without locking.
Yet there is a bug:
^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700 702) spin_lock_irqsave(&priv->lock, flags);
^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700 703) if (priv->write_urb_in_use) {
441b62c1edb98 (Harvey Harrison 2008-03-03 16:08:34 -0800 704) dbg("%s - can't write, urb in use", __func__);
^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700 705) spin_unlock_irqrestore(&priv->lock, flags);
^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700 706) return;
^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700 707) }
^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700 708) spin_unlock_irqrestore(&priv->lock, flags);
The flag is checked is checked under a lock. But then the lock is dropped.
And here:
^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700 759) spin_lock_irqsave(&priv->lock, flags);
^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700 760) priv->write_urb_in_use = 1;
^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700 761) spin_unlock_irqrestore(&priv->lock, flags);
The flag is set under lock, but unconditionally.
The code just makes no sense.
In addition, when you drop the flag without a lock you need to worry
about memory ordering.
HTH
Oliver
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
