Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Mon, 24 Aug 2020 11:53:42 +0200
From:   Johan Hovold <johan@kernel.org>
To:     kernel test robot <lkp@intel.com>
Cc:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        kbuild-all@lists.01.org, linux-kernel@vger.kernel.org
Subject: Re: drivers/greybus/es2.c:439 message_send() error: double unlocked
 'es2->cport_out_urb_lock' (orig line 417)
Message-ID: <20200824095342.GF21288@localhost>
References: <202008140324.LLig4g2M%lkp@intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <202008140324.LLig4g2M%lkp@intel.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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 <lkp@intel.com>
> 
> 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 <elder@linaro.org>
> :::::: CC: Greg Kroah-Hartman <gregkh@google.com>
> 
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

Johan