2020-08-13 19:41:36

by kernel test robot

[permalink] [raw]
Subject: drivers/greybus/es2.c:439 message_send() error: double unlocked 'es2->cport_out_urb_lock' (orig line 417)

Hi Greg,

First bad commit (maybe != root cause):

tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
head: dc06fe51d26efc100ac74121607c01a454867c91
commit: b81beec9cb2d586412c7166c893894930f19965e staging: greybus: move es2 to drivers/greybus/
date: 12 months ago
config: x86_64-randconfig-m001-20200813 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>

New smatch warnings:
drivers/greybus/es2.c:439 message_send() error: double unlocked 'es2->cport_out_urb_lock' (orig line 417)
drivers/greybus/es2.c:879 cport_out_callback() error: double unlocked 'es2->cport_out_urb_lock' (orig line 871)
drivers/greybus/es2.c:1018 arpc_sync() error: double unlocked 'es2->arpc_lock' (orig line 992)

Old smatch warnings:
drivers/greybus/es2.c:441 message_send() error: double unlocked 'es2->cport_out_urb_lock' (orig line 439)

vim +439 drivers/greybus/es2.c

d29b3d631e57240 drivers/staging/greybus/es2.c Alex Elder 2015-06-13 385
f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-01-21 386 /*
3e136cc9e05e1a3 drivers/staging/greybus/es2.c Johan Hovold 2015-07-01 387 * Returns zero if the message was successfully queued, or a negative errno
3e136cc9e05e1a3 drivers/staging/greybus/es2.c Johan Hovold 2015-07-01 388 * otherwise.
f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-01-21 389 */
2537636abae5b81 drivers/staging/greybus/es2.c Johan Hovold 2015-11-03 390 static int message_send(struct gb_host_device *hd, u16 cport_id,
7cf7bca9ec5659e drivers/staging/greybus/es2.c Johan Hovold 2015-04-07 391 struct gb_message *message, gfp_t gfp_mask)
f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-01-21 392 {
4b1d82047ebbb10 drivers/staging/greybus/es2.c Alex Elder 2015-10-27 393 struct es2_ap_dev *es2 = hd_to_es2(hd);
4b1d82047ebbb10 drivers/staging/greybus/es2.c Alex Elder 2015-10-27 394 struct usb_device *udev = es2->usb_dev;
7cf7bca9ec5659e drivers/staging/greybus/es2.c Johan Hovold 2015-04-07 395 size_t buffer_size;
f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-01-21 396 int retval;
f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-01-21 397 struct urb *urb;
3e136cc9e05e1a3 drivers/staging/greybus/es2.c Johan Hovold 2015-07-01 398 unsigned long flags;
f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-01-21 399
f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-01-21 400 /*
f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-01-21 401 * The data actually transferred will include an indication
f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-01-21 402 * of where the data should be sent. Do one last check of
f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-01-21 403 * the target CPort id before filling it in.
f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-01-21 404 */
144670c2ae3f9b4 drivers/staging/greybus/es2.c Fabien Parent 2015-09-02 405 if (!cport_id_valid(hd, cport_id)) {
100e90000840741 drivers/staging/greybus/es2.c Johan Hovold 2015-12-07 406 dev_err(&udev->dev, "invalid cport %u\n", cport_id);
3e136cc9e05e1a3 drivers/staging/greybus/es2.c Johan Hovold 2015-07-01 407 return -EINVAL;
f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-01-21 408 }
f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-01-21 409
f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-01-21 410 /* Find a free urb */
4b1d82047ebbb10 drivers/staging/greybus/es2.c Alex Elder 2015-10-27 411 urb = next_free_urb(es2, gfp_mask);
f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-01-21 412 if (!urb)
3e136cc9e05e1a3 drivers/staging/greybus/es2.c Johan Hovold 2015-07-01 413 return -ENOMEM;
3e136cc9e05e1a3 drivers/staging/greybus/es2.c Johan Hovold 2015-07-01 414
4b1d82047ebbb10 drivers/staging/greybus/es2.c Alex Elder 2015-10-27 415 spin_lock_irqsave(&es2->cport_out_urb_lock, flags);
3e136cc9e05e1a3 drivers/staging/greybus/es2.c Johan Hovold 2015-07-01 416 message->hcpriv = urb;
4b1d82047ebbb10 drivers/staging/greybus/es2.c Alex Elder 2015-10-27 @417 spin_unlock_irqrestore(&es2->cport_out_urb_lock, flags);
f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-01-21 418
d29b3d631e57240 drivers/staging/greybus/es2.c Alex Elder 2015-06-13 419 /* Pack the cport id into the message header */
d29b3d631e57240 drivers/staging/greybus/es2.c Alex Elder 2015-06-13 420 gb_message_cport_pack(message->header, cport_id);
491e60d63fde0a9 drivers/staging/greybus/es2.c Johan Hovold 2015-04-07 421
821c620afa1ad29 drivers/staging/greybus/es2.c Alex Elder 2015-06-13 422 buffer_size = sizeof(*message->header) + message->payload_size;
491e60d63fde0a9 drivers/staging/greybus/es2.c Johan Hovold 2015-04-07 423
f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-01-21 424 usb_fill_bulk_urb(urb, udev,
606addd2847ccc5 drivers/staging/greybus/es2.c Alexandre Bailon 2015-06-15 425 usb_sndbulkpipe(udev,
403074b50b66f1a drivers/staging/greybus/es2.c Greg Kroah-Hartman 2016-08-17 426 es2->cport_out_endpoint),
821c620afa1ad29 drivers/staging/greybus/es2.c Alex Elder 2015-06-13 427 message->buffer, buffer_size,
7cf7bca9ec5659e drivers/staging/greybus/es2.c Johan Hovold 2015-04-07 428 cport_out_callback, message);
977e209ab41073d drivers/staging/greybus/es2.c Alexandre Bailon 2015-08-31 429 urb->transfer_flags |= URB_ZERO_PACKET;
495787a792ac498 drivers/staging/greybus/es2.c Alex Elder 2016-06-03 430
495787a792ac498 drivers/staging/greybus/es2.c Alex Elder 2016-06-03 431 trace_gb_message_submit(message);
495787a792ac498 drivers/staging/greybus/es2.c Alex Elder 2016-06-03 432
f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-01-21 433 retval = usb_submit_urb(urb, gfp_mask);
f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-01-21 434 if (retval) {
05e3095563670ab drivers/staging/greybus/es2.c Viresh Kumar 2016-06-23 435 dev_err(&udev->dev, "failed to submit out-urb: %d\n", retval);
3e136cc9e05e1a3 drivers/staging/greybus/es2.c Johan Hovold 2015-07-01 436
4b1d82047ebbb10 drivers/staging/greybus/es2.c Alex Elder 2015-10-27 437 spin_lock_irqsave(&es2->cport_out_urb_lock, flags);
3e136cc9e05e1a3 drivers/staging/greybus/es2.c Johan Hovold 2015-07-01 438 message->hcpriv = NULL;
4b1d82047ebbb10 drivers/staging/greybus/es2.c Alex Elder 2015-10-27 @439 spin_unlock_irqrestore(&es2->cport_out_urb_lock, flags);
3e136cc9e05e1a3 drivers/staging/greybus/es2.c Johan Hovold 2015-07-01 440
4b1d82047ebbb10 drivers/staging/greybus/es2.c Alex Elder 2015-10-27 441 free_urb(es2, urb);
d29b3d631e57240 drivers/staging/greybus/es2.c Alex Elder 2015-06-13 442 gb_message_cport_clear(message->header);
3e136cc9e05e1a3 drivers/staging/greybus/es2.c Johan Hovold 2015-07-01 443
3e136cc9e05e1a3 drivers/staging/greybus/es2.c Johan Hovold 2015-07-01 444 return retval;
f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-01-21 445 }
f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-01-21 446
3e136cc9e05e1a3 drivers/staging/greybus/es2.c Johan Hovold 2015-07-01 447 return 0;
f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-01-21 448 }
f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-01-21 449

:::::: The code at line 439 was first introduced by commit
:::::: 4b1d82047ebbb108e9961fffd51f8ccd54459221 greybus: es2: rename es2 data structures

:::::: TO: Alex Elder <[email protected]>
:::::: CC: Greg Kroah-Hartman <[email protected]>

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]


Attachments:
(No filename) (8.44 kB)
.config.gz (27.57 kB)
Download all attachments

2020-08-24 09:54:47

by Johan Hovold

[permalink] [raw]
Subject: Re: drivers/greybus/es2.c:439 message_send() error: double unlocked 'es2->cport_out_urb_lock' (orig line 417)

On Fri, Aug 14, 2020 at 03:26:27AM +0800, kernel test robot wrote:
> Hi Greg,
>
> First bad commit (maybe != root cause):
>
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head: dc06fe51d26efc100ac74121607c01a454867c91
> commit: b81beec9cb2d586412c7166c893894930f19965e staging: greybus: move es2 to drivers/greybus/
> date: 12 months ago
> config: x86_64-randconfig-m001-20200813 (attached as .config)
> compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <[email protected]>
>
> New smatch warnings:
> drivers/greybus/es2.c:439 message_send() error: double unlocked 'es2->cport_out_urb_lock' (orig line 417)
> drivers/greybus/es2.c:879 cport_out_callback() error: double unlocked 'es2->cport_out_urb_lock' (orig line 871)
> drivers/greybus/es2.c:1018 arpc_sync() error: double unlocked 'es2->arpc_lock' (orig line 992)
>
> Old smatch warnings:
> drivers/greybus/es2.c:441 message_send() error: double unlocked 'es2->cport_out_urb_lock' (orig line 439)

None of these warnings make any sense. Is the robot drunk?

> vim +439 drivers/greybus/es2.c
>
> d29b3d631e57240 drivers/staging/greybus/es2.c Alex Elder 2015-06-13 385
> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-01-21 386 /*
> 3e136cc9e05e1a3 drivers/staging/greybus/es2.c Johan Hovold 2015-07-01 387 * Returns zero if the message was successfully queued, or a negative errno
> 3e136cc9e05e1a3 drivers/staging/greybus/es2.c Johan Hovold 2015-07-01 388 * otherwise.
> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-01-21 389 */
> 2537636abae5b81 drivers/staging/greybus/es2.c Johan Hovold 2015-11-03 390 static int message_send(struct gb_host_device *hd, u16 cport_id,
> 7cf7bca9ec5659e drivers/staging/greybus/es2.c Johan Hovold 2015-04-07 391 struct gb_message *message, gfp_t gfp_mask)
> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-01-21 392 {
> 4b1d82047ebbb10 drivers/staging/greybus/es2.c Alex Elder 2015-10-27 393 struct es2_ap_dev *es2 = hd_to_es2(hd);
> 4b1d82047ebbb10 drivers/staging/greybus/es2.c Alex Elder 2015-10-27 394 struct usb_device *udev = es2->usb_dev;
> 7cf7bca9ec5659e drivers/staging/greybus/es2.c Johan Hovold 2015-04-07 395 size_t buffer_size;
> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-01-21 396 int retval;
> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-01-21 397 struct urb *urb;
> 3e136cc9e05e1a3 drivers/staging/greybus/es2.c Johan Hovold 2015-07-01 398 unsigned long flags;
> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-01-21 399
> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-01-21 400 /*
> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-01-21 401 * The data actually transferred will include an indication
> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-01-21 402 * of where the data should be sent. Do one last check of
> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-01-21 403 * the target CPort id before filling it in.
> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-01-21 404 */
> 144670c2ae3f9b4 drivers/staging/greybus/es2.c Fabien Parent 2015-09-02 405 if (!cport_id_valid(hd, cport_id)) {
> 100e90000840741 drivers/staging/greybus/es2.c Johan Hovold 2015-12-07 406 dev_err(&udev->dev, "invalid cport %u\n", cport_id);
> 3e136cc9e05e1a3 drivers/staging/greybus/es2.c Johan Hovold 2015-07-01 407 return -EINVAL;
> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-01-21 408 }
> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-01-21 409
> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-01-21 410 /* Find a free urb */
> 4b1d82047ebbb10 drivers/staging/greybus/es2.c Alex Elder 2015-10-27 411 urb = next_free_urb(es2, gfp_mask);
> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-01-21 412 if (!urb)
> 3e136cc9e05e1a3 drivers/staging/greybus/es2.c Johan Hovold 2015-07-01 413 return -ENOMEM;
> 3e136cc9e05e1a3 drivers/staging/greybus/es2.c Johan Hovold 2015-07-01 414
> 4b1d82047ebbb10 drivers/staging/greybus/es2.c Alex Elder 2015-10-27 415 spin_lock_irqsave(&es2->cport_out_urb_lock, flags);
> 3e136cc9e05e1a3 drivers/staging/greybus/es2.c Johan Hovold 2015-07-01 416 message->hcpriv = urb;
> 4b1d82047ebbb10 drivers/staging/greybus/es2.c Alex Elder 2015-10-27 @417 spin_unlock_irqrestore(&es2->cport_out_urb_lock, flags);

How is this a double unlock?

> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-01-21 418
> d29b3d631e57240 drivers/staging/greybus/es2.c Alex Elder 2015-06-13 419 /* Pack the cport id into the message header */
> d29b3d631e57240 drivers/staging/greybus/es2.c Alex Elder 2015-06-13 420 gb_message_cport_pack(message->header, cport_id);
> 491e60d63fde0a9 drivers/staging/greybus/es2.c Johan Hovold 2015-04-07 421
> 821c620afa1ad29 drivers/staging/greybus/es2.c Alex Elder 2015-06-13 422 buffer_size = sizeof(*message->header) + message->payload_size;
> 491e60d63fde0a9 drivers/staging/greybus/es2.c Johan Hovold 2015-04-07 423
> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-01-21 424 usb_fill_bulk_urb(urb, udev,
> 606addd2847ccc5 drivers/staging/greybus/es2.c Alexandre Bailon 2015-06-15 425 usb_sndbulkpipe(udev,
> 403074b50b66f1a drivers/staging/greybus/es2.c Greg Kroah-Hartman 2016-08-17 426 es2->cport_out_endpoint),
> 821c620afa1ad29 drivers/staging/greybus/es2.c Alex Elder 2015-06-13 427 message->buffer, buffer_size,
> 7cf7bca9ec5659e drivers/staging/greybus/es2.c Johan Hovold 2015-04-07 428 cport_out_callback, message);
> 977e209ab41073d drivers/staging/greybus/es2.c Alexandre Bailon 2015-08-31 429 urb->transfer_flags |= URB_ZERO_PACKET;
> 495787a792ac498 drivers/staging/greybus/es2.c Alex Elder 2016-06-03 430
> 495787a792ac498 drivers/staging/greybus/es2.c Alex Elder 2016-06-03 431 trace_gb_message_submit(message);
> 495787a792ac498 drivers/staging/greybus/es2.c Alex Elder 2016-06-03 432
> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-01-21 433 retval = usb_submit_urb(urb, gfp_mask);
> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-01-21 434 if (retval) {
> 05e3095563670ab drivers/staging/greybus/es2.c Viresh Kumar 2016-06-23 435 dev_err(&udev->dev, "failed to submit out-urb: %d\n", retval);
> 3e136cc9e05e1a3 drivers/staging/greybus/es2.c Johan Hovold 2015-07-01 436
> 4b1d82047ebbb10 drivers/staging/greybus/es2.c Alex Elder 2015-10-27 437 spin_lock_irqsave(&es2->cport_out_urb_lock, flags);
> 3e136cc9e05e1a3 drivers/staging/greybus/es2.c Johan Hovold 2015-07-01 438 message->hcpriv = NULL;
> 4b1d82047ebbb10 drivers/staging/greybus/es2.c Alex Elder 2015-10-27 @439 spin_unlock_irqrestore(&es2->cport_out_urb_lock, flags);
> 3e136cc9e05e1a3 drivers/staging/greybus/es2.c Johan Hovold 2015-07-01 440
> 4b1d82047ebbb10 drivers/staging/greybus/es2.c Alex Elder 2015-10-27 441 free_urb(es2, urb);
> d29b3d631e57240 drivers/staging/greybus/es2.c Alex Elder 2015-06-13 442 gb_message_cport_clear(message->header);
> 3e136cc9e05e1a3 drivers/staging/greybus/es2.c Johan Hovold 2015-07-01 443
> 3e136cc9e05e1a3 drivers/staging/greybus/es2.c Johan Hovold 2015-07-01 444 return retval;
> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-01-21 445 }
> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-01-21 446
> 3e136cc9e05e1a3 drivers/staging/greybus/es2.c Johan Hovold 2015-07-01 447 return 0;
> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-01-21 448 }
> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-01-21 449
>
> :::::: The code at line 439 was first introduced by commit
> :::::: 4b1d82047ebbb108e9961fffd51f8ccd54459221 greybus: es2: rename es2 data structures
>
> :::::: TO: Alex Elder <[email protected]>
> :::::: CC: Greg Kroah-Hartman <[email protected]>
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/[email protected]

Johan

2020-08-25 02:41:21

by Xia, Hui

[permalink] [raw]
Subject: RE: [kbuild-all] Re: drivers/greybus/es2.c:439 message_send() error: double unlocked 'es2->cport_out_urb_lock' (orig line 417)



>-----Original Message-----
>From: Johan Hovold <[email protected]>
>Sent: 2020$BG/(B8$B7n(B24$BF|(B 17:54
>To: kernel test robot <[email protected]>
>Cc: Greg Kroah-Hartman <[email protected]>; [email protected];
>[email protected]
>Subject: [kbuild-all] Re: drivers/greybus/es2.c:439 message_send() error: double
>unlocked 'es2->cport_out_urb_lock' (orig line 417)
>
>On Fri, Aug 14, 2020 at 03:26:27AM +0800, kernel test robot wrote:
>> Hi Greg,
>>
>> First bad commit (maybe != root cause):
>>
>> tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
>> head: dc06fe51d26efc100ac74121607c01a454867c91
>> commit: b81beec9cb2d586412c7166c893894930f19965e staging: greybus:
>move es2 to drivers/greybus/
>> date: 12 months ago
>> config: x86_64-randconfig-m001-20200813 (attached as .config)
>> compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
>>
>> If you fix the issue, kindly add following tag as appropriate
>> Reported-by: kernel test robot <[email protected]>
>>
>> New smatch warnings:
>> drivers/greybus/es2.c:439 message_send() error: double unlocked
>> 'es2->cport_out_urb_lock' (orig line 417)
>> drivers/greybus/es2.c:879 cport_out_callback() error: double unlocked
>> 'es2->cport_out_urb_lock' (orig line 871)
>> drivers/greybus/es2.c:1018 arpc_sync() error: double unlocked
>> 'es2->arpc_lock' (orig line 992)
>>
>> Old smatch warnings:
>> drivers/greybus/es2.c:441 message_send() error: double unlocked
>> 'es2->cport_out_urb_lock' (orig line 439)
>
>None of these warnings make any sense. Is the robot drunk?
Hi Johan,

It is a false positive. Please kindly ignore.
There was a bug in robot that sent unconfident reports out directly and has been fixed. Sorry for inconvenient.

Best Regards,
Hui
>
>> vim +439 drivers/greybus/es2.c
>>
>> d29b3d631e57240 drivers/staging/greybus/es2.c Alex Elder 2015-06-13
>385
>> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-
>01-21 386 /*
>> 3e136cc9e05e1a3 drivers/staging/greybus/es2.c Johan Hovold 2015-07-01
>387 * Returns zero if the message was successfully queued, or a negative errno
>> 3e136cc9e05e1a3 drivers/staging/greybus/es2.c Johan Hovold 2015-07-01
>388 * otherwise.
>> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-
>01-21 389 */
>> 2537636abae5b81 drivers/staging/greybus/es2.c Johan Hovold 2015-11-
>03 390 static int message_send(struct gb_host_device *hd, u16 cport_id,
>> 7cf7bca9ec5659e drivers/staging/greybus/es2.c Johan Hovold 2015-04-07
>391 struct gb_message *message, gfp_t gfp_mask)
>> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-
>01-21 392 {
>> 4b1d82047ebbb10 drivers/staging/greybus/es2.c Alex Elder 2015-10-27
>393 struct es2_ap_dev *es2 = hd_to_es2(hd);
>> 4b1d82047ebbb10 drivers/staging/greybus/es2.c Alex Elder 2015-10-27
>394 struct usb_device *udev = es2->usb_dev;
>> 7cf7bca9ec5659e drivers/staging/greybus/es2.c Johan Hovold 2015-04-07
>395 size_t buffer_size;
>> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-
>01-21 396 int retval;
>> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-
>01-21 397 struct urb *urb;
>> 3e136cc9e05e1a3 drivers/staging/greybus/es2.c Johan Hovold 2015-07-01
>398 unsigned long flags;
>> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-
>01-21 399
>> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-
>01-21 400 /*
>> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-
>01-21 401 * The data actually transferred will include an indication
>> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-
>01-21 402 * of where the data should be sent. Do one last check of
>> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-
>01-21 403 * the target CPort id before filling it in.
>> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-
>01-21 404 */
>> 144670c2ae3f9b4 drivers/staging/greybus/es2.c Fabien Parent 2015-09-02
>405 if (!cport_id_valid(hd, cport_id)) {
>> 100e90000840741 drivers/staging/greybus/es2.c Johan Hovold 2015-12-
>07 406 dev_err(&udev->dev, "invalid cport %u\n", cport_id);
>> 3e136cc9e05e1a3 drivers/staging/greybus/es2.c Johan Hovold 2015-07-01
>407 return -EINVAL;
>> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-
>01-21 408 }
>> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-
>01-21 409
>> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-
>01-21 410 /* Find a free urb */
>> 4b1d82047ebbb10 drivers/staging/greybus/es2.c Alex Elder 2015-10-27
>411 urb = next_free_urb(es2, gfp_mask);
>> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-
>01-21 412 if (!urb)
>> 3e136cc9e05e1a3 drivers/staging/greybus/es2.c Johan Hovold 2015-07-01
>413 return -ENOMEM;
>> 3e136cc9e05e1a3 drivers/staging/greybus/es2.c Johan Hovold 2015-07-01
>414
>> 4b1d82047ebbb10 drivers/staging/greybus/es2.c Alex Elder 2015-10-27
>415 spin_lock_irqsave(&es2->cport_out_urb_lock, flags);
>> 3e136cc9e05e1a3 drivers/staging/greybus/es2.c Johan Hovold 2015-07-01
>416 message->hcpriv = urb;
>> 4b1d82047ebbb10 drivers/staging/greybus/es2.c Alex Elder 2015-10-27
>@417 spin_unlock_irqrestore(&es2->cport_out_urb_lock, flags);
>
>How is this a double unlock?
>
>> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-
>01-21 418
>> d29b3d631e57240 drivers/staging/greybus/es2.c Alex Elder 2015-06-13
>419 /* Pack the cport id into the message header */
>> d29b3d631e57240 drivers/staging/greybus/es2.c Alex Elder 2015-06-13
>420 gb_message_cport_pack(message->header, cport_id);
>> 491e60d63fde0a9 drivers/staging/greybus/es2.c Johan Hovold 2015-04-07
>421
>> 821c620afa1ad29 drivers/staging/greybus/es2.c Alex Elder 2015-06-13
>422 buffer_size = sizeof(*message->header) + message->payload_size;
>> 491e60d63fde0a9 drivers/staging/greybus/es2.c Johan Hovold 2015-04-07
>423
>> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-
>01-21 424 usb_fill_bulk_urb(urb, udev,
>> 606addd2847ccc5 drivers/staging/greybus/es2.c Alexandre Bailon 2015-06-
>15 425 usb_sndbulkpipe(udev,
>> 403074b50b66f1a drivers/staging/greybus/es2.c Greg Kroah-Hartman 2016-
>08-17 426 es2->cport_out_endpoint),
>> 821c620afa1ad29 drivers/staging/greybus/es2.c Alex Elder 2015-06-13
>427 message->buffer, buffer_size,
>> 7cf7bca9ec5659e drivers/staging/greybus/es2.c Johan Hovold 2015-04-07
>428 cport_out_callback, message);
>> 977e209ab41073d drivers/staging/greybus/es2.c Alexandre Bailon 2015-08-
>31 429 urb->transfer_flags |= URB_ZERO_PACKET;
>> 495787a792ac498 drivers/staging/greybus/es2.c Alex Elder 2016-06-03
>430
>> 495787a792ac498 drivers/staging/greybus/es2.c Alex Elder 2016-06-03
>431 trace_gb_message_submit(message);
>> 495787a792ac498 drivers/staging/greybus/es2.c Alex Elder 2016-06-03
>432
>> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-
>01-21 433 retval = usb_submit_urb(urb, gfp_mask);
>> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-
>01-21 434 if (retval) {
>> 05e3095563670ab drivers/staging/greybus/es2.c Viresh Kumar 2016-06-23
>435 dev_err(&udev->dev, "failed to submit out-urb: %d\n", retval);
>> 3e136cc9e05e1a3 drivers/staging/greybus/es2.c Johan Hovold 2015-07-01
>436
>> 4b1d82047ebbb10 drivers/staging/greybus/es2.c Alex Elder 2015-10-27
>437 spin_lock_irqsave(&es2->cport_out_urb_lock, flags);
>> 3e136cc9e05e1a3 drivers/staging/greybus/es2.c Johan Hovold 2015-07-01
>438 message->hcpriv = NULL;
>> 4b1d82047ebbb10 drivers/staging/greybus/es2.c Alex Elder 2015-10-27
>@439 spin_unlock_irqrestore(&es2->cport_out_urb_lock, flags);
>> 3e136cc9e05e1a3 drivers/staging/greybus/es2.c Johan Hovold 2015-07-01
>440
>> 4b1d82047ebbb10 drivers/staging/greybus/es2.c Alex Elder 2015-10-27
>441 free_urb(es2, urb);
>> d29b3d631e57240 drivers/staging/greybus/es2.c Alex Elder 2015-06-13
>442 gb_message_cport_clear(message->header);
>> 3e136cc9e05e1a3 drivers/staging/greybus/es2.c Johan Hovold 2015-07-01
>443
>> 3e136cc9e05e1a3 drivers/staging/greybus/es2.c Johan Hovold 2015-07-01
>444 return retval;
>> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-
>01-21 445 }
>> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-
>01-21 446
>> 3e136cc9e05e1a3 drivers/staging/greybus/es2.c Johan Hovold 2015-07-01
>447 return 0;
>> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman
>> 2015-01-21 448 }
>> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman
>> 2015-01-21 449
>>
>> :::::: The code at line 439 was first introduced by commit
>> :::::: 4b1d82047ebbb108e9961fffd51f8ccd54459221 greybus: es2: rename
>> es2 data structures
>>
>> :::::: TO: Alex Elder <[email protected]>
>> :::::: CC: Greg Kroah-Hartman <[email protected]>
>>
>> ---
>> 0-DAY CI Kernel Test Service, Intel Corporation
>> https://lists.01.org/hyperkitty/list/[email protected]
>
>Johan
>_______________________________________________
>kbuild-all mailing list -- [email protected] To unsubscribe send an email to
>[email protected]

2020-08-25 07:14:21

by Johan Hovold

[permalink] [raw]
Subject: Re: [kbuild-all] Re: drivers/greybus/es2.c:439 message_send() error: double unlocked 'es2->cport_out_urb_lock' (orig line 417)

On Tue, Aug 25, 2020 at 02:37:04AM +0000, Xia, Hui wrote:
> >-----Original Message-----
> >From: Johan Hovold <[email protected]>
> >Sent: 2020年8月24日 17:54
> >To: kernel test robot <[email protected]>
> >Cc: Greg Kroah-Hartman <[email protected]>; [email protected];
> >[email protected]
> >Subject: [kbuild-all] Re: drivers/greybus/es2.c:439 message_send() error: double
> >unlocked 'es2->cport_out_urb_lock' (orig line 417)
> >
> >On Fri, Aug 14, 2020 at 03:26:27AM +0800, kernel test robot wrote:
> >> Hi Greg,
> >>
> >> First bad commit (maybe != root cause):
> >>
> >> tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> >> head: dc06fe51d26efc100ac74121607c01a454867c91
> >> commit: b81beec9cb2d586412c7166c893894930f19965e staging: greybus:
> >move es2 to drivers/greybus/
> >> date: 12 months ago
> >> config: x86_64-randconfig-m001-20200813 (attached as .config)
> >> compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
> >>
> >> If you fix the issue, kindly add following tag as appropriate
> >> Reported-by: kernel test robot <[email protected]>
> >>
> >> New smatch warnings:
> >> drivers/greybus/es2.c:439 message_send() error: double unlocked
> >> 'es2->cport_out_urb_lock' (orig line 417)
> >> drivers/greybus/es2.c:879 cport_out_callback() error: double unlocked
> >> 'es2->cport_out_urb_lock' (orig line 871)
> >> drivers/greybus/es2.c:1018 arpc_sync() error: double unlocked
> >> 'es2->arpc_lock' (orig line 992)
> >>
> >> Old smatch warnings:
> >> drivers/greybus/es2.c:441 message_send() error: double unlocked
> >> 'es2->cport_out_urb_lock' (orig line 439)
> >
> >None of these warnings make any sense. Is the robot drunk?

> It is a false positive. Please kindly ignore.
> There was a bug in robot that sent unconfident reports out directly
> and has been fixed. Sorry for inconvenient.

Ok, thanks for confirming.

Johan