All of lore.kernel.org
 help / color / mirror / Atom feed
* drivers/net/can/usb/peak_usb/pcan_usb.c:523 pcan_usb_decode_error() error: we previously assumed 'cf' could be null (see line 503)
@ 2021-12-03 14:09 ` Dan Carpenter
  0 siblings, 0 replies; 11+ messages in thread
From: kernel test robot @ 2021-12-02 10:24 UTC (permalink / raw)
  To: kbuild

[-- Attachment #1: Type: text/plain, Size: 7578 bytes --]

CC: kbuild-all(a)lists.01.org
CC: linux-kernel(a)vger.kernel.org
TO: Stephane Grosjean <s.grosjean@peak-system.com>
CC: "Marc Kleine-Budde" <mkl@pengutronix.de>

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
head:   58e1100fdc5990b0cc0d4beaf2562a92e621ac7d
commit: c11dcee758302702a83c6e85e4c4c3d9af42d2b3 can: peak_usb: pcan_usb_decode_error(): upgrade handling of bus state changes
date:   4 months ago
:::::: branch date: 2 days ago
:::::: commit date: 4 months ago
config: x86_64-randconfig-m001-20211202 (https://download.01.org/0day-ci/archive/20211202/202112021833.wABxM5UN-lkp(a)intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

smatch warnings:
drivers/net/can/usb/peak_usb/pcan_usb.c:523 pcan_usb_decode_error() error: we previously assumed 'cf' could be null (see line 503)

vim +/cf +523 drivers/net/can/usb/peak_usb/pcan_usb.c

46be265d338833 Stephane Grosjean 2012-03-02  449  
46be265d338833 Stephane Grosjean 2012-03-02  450  static int pcan_usb_decode_error(struct pcan_usb_msg_context *mc, u8 n,
46be265d338833 Stephane Grosjean 2012-03-02  451  				 u8 status_len)
46be265d338833 Stephane Grosjean 2012-03-02  452  {
46be265d338833 Stephane Grosjean 2012-03-02  453  	struct sk_buff *skb;
46be265d338833 Stephane Grosjean 2012-03-02  454  	struct can_frame *cf;
c11dcee7583027 Stephane Grosjean 2021-07-15  455  	enum can_state new_state = CAN_STATE_ERROR_ACTIVE;
46be265d338833 Stephane Grosjean 2012-03-02  456  
46be265d338833 Stephane Grosjean 2012-03-02  457  	/* ignore this error until 1st ts received */
46be265d338833 Stephane Grosjean 2012-03-02  458  	if (n == PCAN_USB_ERROR_QOVR)
46be265d338833 Stephane Grosjean 2012-03-02  459  		if (!mc->pdev->time_ref.tick_count)
46be265d338833 Stephane Grosjean 2012-03-02  460  			return 0;
46be265d338833 Stephane Grosjean 2012-03-02  461  
c11dcee7583027 Stephane Grosjean 2021-07-15  462  	/* allocate an skb to store the error frame */
c11dcee7583027 Stephane Grosjean 2021-07-15  463  	skb = alloc_can_err_skb(mc->netdev, &cf);
46be265d338833 Stephane Grosjean 2012-03-02  464  
c11dcee7583027 Stephane Grosjean 2021-07-15  465  	if (n & PCAN_USB_ERROR_RXQOVR) {
c11dcee7583027 Stephane Grosjean 2021-07-15  466  		/* data overrun interrupt */
c11dcee7583027 Stephane Grosjean 2021-07-15  467  		netdev_dbg(mc->netdev, "data overrun interrupt\n");
c11dcee7583027 Stephane Grosjean 2021-07-15  468  		mc->netdev->stats.rx_over_errors++;
c11dcee7583027 Stephane Grosjean 2021-07-15  469  		mc->netdev->stats.rx_errors++;
c11dcee7583027 Stephane Grosjean 2021-07-15  470  		if (cf) {
c11dcee7583027 Stephane Grosjean 2021-07-15  471  			cf->can_id |= CAN_ERR_CRTL;
c11dcee7583027 Stephane Grosjean 2021-07-15  472  			cf->data[1] |= CAN_ERR_CRTL_RX_OVERFLOW;
46be265d338833 Stephane Grosjean 2012-03-02  473  		}
46be265d338833 Stephane Grosjean 2012-03-02  474  	}
46be265d338833 Stephane Grosjean 2012-03-02  475  
c11dcee7583027 Stephane Grosjean 2021-07-15  476  	if (n & PCAN_USB_ERROR_TXQFULL)
c11dcee7583027 Stephane Grosjean 2021-07-15  477  		netdev_dbg(mc->netdev, "device Tx queue full)\n");
c11dcee7583027 Stephane Grosjean 2021-07-15  478  
46be265d338833 Stephane Grosjean 2012-03-02  479  	if (n & PCAN_USB_ERROR_BUS_OFF) {
46be265d338833 Stephane Grosjean 2012-03-02  480  		new_state = CAN_STATE_BUS_OFF;
c11dcee7583027 Stephane Grosjean 2021-07-15  481  	} else if (n & PCAN_USB_ERROR_BUS_HEAVY) {
c11dcee7583027 Stephane Grosjean 2021-07-15  482  		new_state = ((mc->pdev->bec.txerr >= 128) ||
c11dcee7583027 Stephane Grosjean 2021-07-15  483  			     (mc->pdev->bec.rxerr >= 128)) ?
c11dcee7583027 Stephane Grosjean 2021-07-15  484  				CAN_STATE_ERROR_PASSIVE :
c11dcee7583027 Stephane Grosjean 2021-07-15  485  				CAN_STATE_ERROR_WARNING;
c11dcee7583027 Stephane Grosjean 2021-07-15  486  	} else {
c11dcee7583027 Stephane Grosjean 2021-07-15  487  		new_state = CAN_STATE_ERROR_ACTIVE;
46be265d338833 Stephane Grosjean 2012-03-02  488  	}
46be265d338833 Stephane Grosjean 2012-03-02  489  
c11dcee7583027 Stephane Grosjean 2021-07-15  490  	/* handle change of state */
c11dcee7583027 Stephane Grosjean 2021-07-15  491  	if (new_state != mc->pdev->dev.can.state) {
c11dcee7583027 Stephane Grosjean 2021-07-15  492  		enum can_state tx_state =
c11dcee7583027 Stephane Grosjean 2021-07-15  493  			(mc->pdev->bec.txerr >= mc->pdev->bec.rxerr) ?
c11dcee7583027 Stephane Grosjean 2021-07-15  494  				new_state : 0;
c11dcee7583027 Stephane Grosjean 2021-07-15  495  		enum can_state rx_state =
c11dcee7583027 Stephane Grosjean 2021-07-15  496  			(mc->pdev->bec.txerr <= mc->pdev->bec.rxerr) ?
c11dcee7583027 Stephane Grosjean 2021-07-15  497  				new_state : 0;
46be265d338833 Stephane Grosjean 2012-03-02  498  
c11dcee7583027 Stephane Grosjean 2021-07-15  499  		can_change_state(mc->netdev, cf, tx_state, rx_state);
46be265d338833 Stephane Grosjean 2012-03-02  500  
c11dcee7583027 Stephane Grosjean 2021-07-15  501  		if (new_state == CAN_STATE_BUS_OFF) {
46be265d338833 Stephane Grosjean 2012-03-02  502  			can_bus_off(mc->netdev);
c11dcee7583027 Stephane Grosjean 2021-07-15 @503  		} else if (cf && (cf->can_id & CAN_ERR_CRTL)) {
c11dcee7583027 Stephane Grosjean 2021-07-15  504  			/* Supply TX/RX error counters in case of
c11dcee7583027 Stephane Grosjean 2021-07-15  505  			 * controller error.
c11dcee7583027 Stephane Grosjean 2021-07-15  506  			 */
ea8b33bde76c8f Stephane Grosjean 2019-12-06  507  			cf->data[6] = mc->pdev->bec.txerr;
ea8b33bde76c8f Stephane Grosjean 2019-12-06  508  			cf->data[7] = mc->pdev->bec.rxerr;
ea8b33bde76c8f Stephane Grosjean 2019-12-06  509  		}
46be265d338833 Stephane Grosjean 2012-03-02  510  	}
46be265d338833 Stephane Grosjean 2012-03-02  511  
c11dcee7583027 Stephane Grosjean 2021-07-15  512  	if (!skb)
c11dcee7583027 Stephane Grosjean 2021-07-15  513  		return -ENOMEM;
46be265d338833 Stephane Grosjean 2012-03-02  514  
46be265d338833 Stephane Grosjean 2012-03-02  515  	if (status_len & PCAN_USB_STATUSLEN_TIMESTAMP) {
c9faaa09e2a133 Oliver Hartkopp   2012-11-21  516  		struct skb_shared_hwtstamps *hwts = skb_hwtstamps(skb);
c9faaa09e2a133 Oliver Hartkopp   2012-11-21  517  
d5888a1e75c799 Arnd Bergmann     2017-11-03  518  		peak_usb_get_ts_time(&mc->pdev->time_ref, mc->ts16,
d5888a1e75c799 Arnd Bergmann     2017-11-03  519  				     &hwts->hwtstamp);
46be265d338833 Stephane Grosjean 2012-03-02  520  	}
46be265d338833 Stephane Grosjean 2012-03-02  521  
46be265d338833 Stephane Grosjean 2012-03-02  522  	mc->netdev->stats.rx_packets++;
c7b74967799b1a Oliver Hartkopp   2020-11-20 @523  	mc->netdev->stats.rx_bytes += cf->len;
1c0ee046957648 Marc Kleine-Budde 2015-07-11  524  	netif_rx(skb);
46be265d338833 Stephane Grosjean 2012-03-02  525  
46be265d338833 Stephane Grosjean 2012-03-02  526  	return 0;
46be265d338833 Stephane Grosjean 2012-03-02  527  }
46be265d338833 Stephane Grosjean 2012-03-02  528  

:::::: The code at line 523 was first introduced by commit
:::::: c7b74967799b1af52b3045d69d4c26836b2d41de can: replace can_dlc as variable/element for payload length

:::::: TO: Oliver Hartkopp <socketcan@hartkopp.net>
:::::: CC: Marc Kleine-Budde <mkl@pengutronix.de>

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

^ permalink raw reply	[flat|nested] 11+ messages in thread

* drivers/net/can/usb/peak_usb/pcan_usb.c:523 pcan_usb_decode_error() error: we previously assumed 'cf' could be null (see line 503)
@ 2021-12-03 14:09 ` Dan Carpenter
  0 siblings, 0 replies; 11+ messages in thread
From: Dan Carpenter @ 2021-12-03 14:09 UTC (permalink / raw)
  To: kbuild, Stephane Grosjean
  Cc: lkp, kbuild-all, linux-kernel, Marc Kleine-Budde

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
head:   58e1100fdc5990b0cc0d4beaf2562a92e621ac7d
commit: c11dcee758302702a83c6e85e4c4c3d9af42d2b3 can: peak_usb: pcan_usb_decode_error(): upgrade handling of bus state changes
config: x86_64-randconfig-m001-20211202 (https://download.01.org/0day-ci/archive/20211202/202112021833.wABxM5UN-lkp@intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

smatch warnings:
drivers/net/can/usb/peak_usb/pcan_usb.c:523 pcan_usb_decode_error() error: we previously assumed 'cf' could be null (see line 503)

vim +/cf +523 drivers/net/can/usb/peak_usb/pcan_usb.c

46be265d338833 Stephane Grosjean 2012-03-02  450  static int pcan_usb_decode_error(struct pcan_usb_msg_context *mc, u8 n,
46be265d338833 Stephane Grosjean 2012-03-02  451  				 u8 status_len)
46be265d338833 Stephane Grosjean 2012-03-02  452  {
46be265d338833 Stephane Grosjean 2012-03-02  453  	struct sk_buff *skb;
46be265d338833 Stephane Grosjean 2012-03-02  454  	struct can_frame *cf;
c11dcee7583027 Stephane Grosjean 2021-07-15  455  	enum can_state new_state = CAN_STATE_ERROR_ACTIVE;
46be265d338833 Stephane Grosjean 2012-03-02  456  
46be265d338833 Stephane Grosjean 2012-03-02  457  	/* ignore this error until 1st ts received */
46be265d338833 Stephane Grosjean 2012-03-02  458  	if (n == PCAN_USB_ERROR_QOVR)
46be265d338833 Stephane Grosjean 2012-03-02  459  		if (!mc->pdev->time_ref.tick_count)
46be265d338833 Stephane Grosjean 2012-03-02  460  			return 0;
46be265d338833 Stephane Grosjean 2012-03-02  461  
c11dcee7583027 Stephane Grosjean 2021-07-15  462  	/* allocate an skb to store the error frame */
c11dcee7583027 Stephane Grosjean 2021-07-15  463  	skb = alloc_can_err_skb(mc->netdev, &cf);
46be265d338833 Stephane Grosjean 2012-03-02  464  
c11dcee7583027 Stephane Grosjean 2021-07-15  465  	if (n & PCAN_USB_ERROR_RXQOVR) {
c11dcee7583027 Stephane Grosjean 2021-07-15  466  		/* data overrun interrupt */
c11dcee7583027 Stephane Grosjean 2021-07-15  467  		netdev_dbg(mc->netdev, "data overrun interrupt\n");
c11dcee7583027 Stephane Grosjean 2021-07-15  468  		mc->netdev->stats.rx_over_errors++;
c11dcee7583027 Stephane Grosjean 2021-07-15  469  		mc->netdev->stats.rx_errors++;
c11dcee7583027 Stephane Grosjean 2021-07-15  470  		if (cf) {

Check for NULL

c11dcee7583027 Stephane Grosjean 2021-07-15  471  			cf->can_id |= CAN_ERR_CRTL;
c11dcee7583027 Stephane Grosjean 2021-07-15  472  			cf->data[1] |= CAN_ERR_CRTL_RX_OVERFLOW;
46be265d338833 Stephane Grosjean 2012-03-02  473  		}
46be265d338833 Stephane Grosjean 2012-03-02  474  	}
46be265d338833 Stephane Grosjean 2012-03-02  475  
c11dcee7583027 Stephane Grosjean 2021-07-15  476  	if (n & PCAN_USB_ERROR_TXQFULL)
c11dcee7583027 Stephane Grosjean 2021-07-15  477  		netdev_dbg(mc->netdev, "device Tx queue full)\n");
c11dcee7583027 Stephane Grosjean 2021-07-15  478  
46be265d338833 Stephane Grosjean 2012-03-02  479  	if (n & PCAN_USB_ERROR_BUS_OFF) {
46be265d338833 Stephane Grosjean 2012-03-02  480  		new_state = CAN_STATE_BUS_OFF;
c11dcee7583027 Stephane Grosjean 2021-07-15  481  	} else if (n & PCAN_USB_ERROR_BUS_HEAVY) {
c11dcee7583027 Stephane Grosjean 2021-07-15  482  		new_state = ((mc->pdev->bec.txerr >= 128) ||
c11dcee7583027 Stephane Grosjean 2021-07-15  483  			     (mc->pdev->bec.rxerr >= 128)) ?
c11dcee7583027 Stephane Grosjean 2021-07-15  484  				CAN_STATE_ERROR_PASSIVE :
c11dcee7583027 Stephane Grosjean 2021-07-15  485  				CAN_STATE_ERROR_WARNING;
c11dcee7583027 Stephane Grosjean 2021-07-15  486  	} else {
c11dcee7583027 Stephane Grosjean 2021-07-15  487  		new_state = CAN_STATE_ERROR_ACTIVE;
46be265d338833 Stephane Grosjean 2012-03-02  488  	}
46be265d338833 Stephane Grosjean 2012-03-02  489  
c11dcee7583027 Stephane Grosjean 2021-07-15  490  	/* handle change of state */
c11dcee7583027 Stephane Grosjean 2021-07-15  491  	if (new_state != mc->pdev->dev.can.state) {
c11dcee7583027 Stephane Grosjean 2021-07-15  492  		enum can_state tx_state =
c11dcee7583027 Stephane Grosjean 2021-07-15  493  			(mc->pdev->bec.txerr >= mc->pdev->bec.rxerr) ?
c11dcee7583027 Stephane Grosjean 2021-07-15  494  				new_state : 0;
c11dcee7583027 Stephane Grosjean 2021-07-15  495  		enum can_state rx_state =
c11dcee7583027 Stephane Grosjean 2021-07-15  496  			(mc->pdev->bec.txerr <= mc->pdev->bec.rxerr) ?
c11dcee7583027 Stephane Grosjean 2021-07-15  497  				new_state : 0;
46be265d338833 Stephane Grosjean 2012-03-02  498  
c11dcee7583027 Stephane Grosjean 2021-07-15  499  		can_change_state(mc->netdev, cf, tx_state, rx_state);
46be265d338833 Stephane Grosjean 2012-03-02  500  
c11dcee7583027 Stephane Grosjean 2021-07-15  501  		if (new_state == CAN_STATE_BUS_OFF) {
46be265d338833 Stephane Grosjean 2012-03-02  502  			can_bus_off(mc->netdev);
c11dcee7583027 Stephane Grosjean 2021-07-15 @503  		} else if (cf && (cf->can_id & CAN_ERR_CRTL)) {

Check for NULL

c11dcee7583027 Stephane Grosjean 2021-07-15  504  			/* Supply TX/RX error counters in case of
c11dcee7583027 Stephane Grosjean 2021-07-15  505  			 * controller error.
c11dcee7583027 Stephane Grosjean 2021-07-15  506  			 */
ea8b33bde76c8f Stephane Grosjean 2019-12-06  507  			cf->data[6] = mc->pdev->bec.txerr;
ea8b33bde76c8f Stephane Grosjean 2019-12-06  508  			cf->data[7] = mc->pdev->bec.rxerr;
ea8b33bde76c8f Stephane Grosjean 2019-12-06  509  		}
46be265d338833 Stephane Grosjean 2012-03-02  510  	}
46be265d338833 Stephane Grosjean 2012-03-02  511  
c11dcee7583027 Stephane Grosjean 2021-07-15  512  	if (!skb)
c11dcee7583027 Stephane Grosjean 2021-07-15  513  		return -ENOMEM;
46be265d338833 Stephane Grosjean 2012-03-02  514  
46be265d338833 Stephane Grosjean 2012-03-02  515  	if (status_len & PCAN_USB_STATUSLEN_TIMESTAMP) {
c9faaa09e2a133 Oliver Hartkopp   2012-11-21  516  		struct skb_shared_hwtstamps *hwts = skb_hwtstamps(skb);
c9faaa09e2a133 Oliver Hartkopp   2012-11-21  517  
d5888a1e75c799 Arnd Bergmann     2017-11-03  518  		peak_usb_get_ts_time(&mc->pdev->time_ref, mc->ts16,
d5888a1e75c799 Arnd Bergmann     2017-11-03  519  				     &hwts->hwtstamp);
46be265d338833 Stephane Grosjean 2012-03-02  520  	}
46be265d338833 Stephane Grosjean 2012-03-02  521  
46be265d338833 Stephane Grosjean 2012-03-02  522  	mc->netdev->stats.rx_packets++;
c7b74967799b1a Oliver Hartkopp   2020-11-20 @523  	mc->netdev->stats.rx_bytes += cf->len;
                                                                                      ^^^^^^^^
No check for NULL.

1c0ee046957648 Marc Kleine-Budde 2015-07-11  524  	netif_rx(skb);
46be265d338833 Stephane Grosjean 2012-03-02  525  
46be265d338833 Stephane Grosjean 2012-03-02  526  	return 0;
46be265d338833 Stephane Grosjean 2012-03-02  527  }

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org



^ permalink raw reply	[flat|nested] 11+ messages in thread

* drivers/net/can/usb/peak_usb/pcan_usb.c:523 pcan_usb_decode_error() error: we previously assumed 'cf' could be null (see line 503)
@ 2021-12-03 14:09 ` Dan Carpenter
  0 siblings, 0 replies; 11+ messages in thread
From: Dan Carpenter @ 2021-12-03 14:09 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 7102 bytes --]

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
head:   58e1100fdc5990b0cc0d4beaf2562a92e621ac7d
commit: c11dcee758302702a83c6e85e4c4c3d9af42d2b3 can: peak_usb: pcan_usb_decode_error(): upgrade handling of bus state changes
config: x86_64-randconfig-m001-20211202 (https://download.01.org/0day-ci/archive/20211202/202112021833.wABxM5UN-lkp(a)intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

smatch warnings:
drivers/net/can/usb/peak_usb/pcan_usb.c:523 pcan_usb_decode_error() error: we previously assumed 'cf' could be null (see line 503)

vim +/cf +523 drivers/net/can/usb/peak_usb/pcan_usb.c

46be265d338833 Stephane Grosjean 2012-03-02  450  static int pcan_usb_decode_error(struct pcan_usb_msg_context *mc, u8 n,
46be265d338833 Stephane Grosjean 2012-03-02  451  				 u8 status_len)
46be265d338833 Stephane Grosjean 2012-03-02  452  {
46be265d338833 Stephane Grosjean 2012-03-02  453  	struct sk_buff *skb;
46be265d338833 Stephane Grosjean 2012-03-02  454  	struct can_frame *cf;
c11dcee7583027 Stephane Grosjean 2021-07-15  455  	enum can_state new_state = CAN_STATE_ERROR_ACTIVE;
46be265d338833 Stephane Grosjean 2012-03-02  456  
46be265d338833 Stephane Grosjean 2012-03-02  457  	/* ignore this error until 1st ts received */
46be265d338833 Stephane Grosjean 2012-03-02  458  	if (n == PCAN_USB_ERROR_QOVR)
46be265d338833 Stephane Grosjean 2012-03-02  459  		if (!mc->pdev->time_ref.tick_count)
46be265d338833 Stephane Grosjean 2012-03-02  460  			return 0;
46be265d338833 Stephane Grosjean 2012-03-02  461  
c11dcee7583027 Stephane Grosjean 2021-07-15  462  	/* allocate an skb to store the error frame */
c11dcee7583027 Stephane Grosjean 2021-07-15  463  	skb = alloc_can_err_skb(mc->netdev, &cf);
46be265d338833 Stephane Grosjean 2012-03-02  464  
c11dcee7583027 Stephane Grosjean 2021-07-15  465  	if (n & PCAN_USB_ERROR_RXQOVR) {
c11dcee7583027 Stephane Grosjean 2021-07-15  466  		/* data overrun interrupt */
c11dcee7583027 Stephane Grosjean 2021-07-15  467  		netdev_dbg(mc->netdev, "data overrun interrupt\n");
c11dcee7583027 Stephane Grosjean 2021-07-15  468  		mc->netdev->stats.rx_over_errors++;
c11dcee7583027 Stephane Grosjean 2021-07-15  469  		mc->netdev->stats.rx_errors++;
c11dcee7583027 Stephane Grosjean 2021-07-15  470  		if (cf) {

Check for NULL

c11dcee7583027 Stephane Grosjean 2021-07-15  471  			cf->can_id |= CAN_ERR_CRTL;
c11dcee7583027 Stephane Grosjean 2021-07-15  472  			cf->data[1] |= CAN_ERR_CRTL_RX_OVERFLOW;
46be265d338833 Stephane Grosjean 2012-03-02  473  		}
46be265d338833 Stephane Grosjean 2012-03-02  474  	}
46be265d338833 Stephane Grosjean 2012-03-02  475  
c11dcee7583027 Stephane Grosjean 2021-07-15  476  	if (n & PCAN_USB_ERROR_TXQFULL)
c11dcee7583027 Stephane Grosjean 2021-07-15  477  		netdev_dbg(mc->netdev, "device Tx queue full)\n");
c11dcee7583027 Stephane Grosjean 2021-07-15  478  
46be265d338833 Stephane Grosjean 2012-03-02  479  	if (n & PCAN_USB_ERROR_BUS_OFF) {
46be265d338833 Stephane Grosjean 2012-03-02  480  		new_state = CAN_STATE_BUS_OFF;
c11dcee7583027 Stephane Grosjean 2021-07-15  481  	} else if (n & PCAN_USB_ERROR_BUS_HEAVY) {
c11dcee7583027 Stephane Grosjean 2021-07-15  482  		new_state = ((mc->pdev->bec.txerr >= 128) ||
c11dcee7583027 Stephane Grosjean 2021-07-15  483  			     (mc->pdev->bec.rxerr >= 128)) ?
c11dcee7583027 Stephane Grosjean 2021-07-15  484  				CAN_STATE_ERROR_PASSIVE :
c11dcee7583027 Stephane Grosjean 2021-07-15  485  				CAN_STATE_ERROR_WARNING;
c11dcee7583027 Stephane Grosjean 2021-07-15  486  	} else {
c11dcee7583027 Stephane Grosjean 2021-07-15  487  		new_state = CAN_STATE_ERROR_ACTIVE;
46be265d338833 Stephane Grosjean 2012-03-02  488  	}
46be265d338833 Stephane Grosjean 2012-03-02  489  
c11dcee7583027 Stephane Grosjean 2021-07-15  490  	/* handle change of state */
c11dcee7583027 Stephane Grosjean 2021-07-15  491  	if (new_state != mc->pdev->dev.can.state) {
c11dcee7583027 Stephane Grosjean 2021-07-15  492  		enum can_state tx_state =
c11dcee7583027 Stephane Grosjean 2021-07-15  493  			(mc->pdev->bec.txerr >= mc->pdev->bec.rxerr) ?
c11dcee7583027 Stephane Grosjean 2021-07-15  494  				new_state : 0;
c11dcee7583027 Stephane Grosjean 2021-07-15  495  		enum can_state rx_state =
c11dcee7583027 Stephane Grosjean 2021-07-15  496  			(mc->pdev->bec.txerr <= mc->pdev->bec.rxerr) ?
c11dcee7583027 Stephane Grosjean 2021-07-15  497  				new_state : 0;
46be265d338833 Stephane Grosjean 2012-03-02  498  
c11dcee7583027 Stephane Grosjean 2021-07-15  499  		can_change_state(mc->netdev, cf, tx_state, rx_state);
46be265d338833 Stephane Grosjean 2012-03-02  500  
c11dcee7583027 Stephane Grosjean 2021-07-15  501  		if (new_state == CAN_STATE_BUS_OFF) {
46be265d338833 Stephane Grosjean 2012-03-02  502  			can_bus_off(mc->netdev);
c11dcee7583027 Stephane Grosjean 2021-07-15 @503  		} else if (cf && (cf->can_id & CAN_ERR_CRTL)) {

Check for NULL

c11dcee7583027 Stephane Grosjean 2021-07-15  504  			/* Supply TX/RX error counters in case of
c11dcee7583027 Stephane Grosjean 2021-07-15  505  			 * controller error.
c11dcee7583027 Stephane Grosjean 2021-07-15  506  			 */
ea8b33bde76c8f Stephane Grosjean 2019-12-06  507  			cf->data[6] = mc->pdev->bec.txerr;
ea8b33bde76c8f Stephane Grosjean 2019-12-06  508  			cf->data[7] = mc->pdev->bec.rxerr;
ea8b33bde76c8f Stephane Grosjean 2019-12-06  509  		}
46be265d338833 Stephane Grosjean 2012-03-02  510  	}
46be265d338833 Stephane Grosjean 2012-03-02  511  
c11dcee7583027 Stephane Grosjean 2021-07-15  512  	if (!skb)
c11dcee7583027 Stephane Grosjean 2021-07-15  513  		return -ENOMEM;
46be265d338833 Stephane Grosjean 2012-03-02  514  
46be265d338833 Stephane Grosjean 2012-03-02  515  	if (status_len & PCAN_USB_STATUSLEN_TIMESTAMP) {
c9faaa09e2a133 Oliver Hartkopp   2012-11-21  516  		struct skb_shared_hwtstamps *hwts = skb_hwtstamps(skb);
c9faaa09e2a133 Oliver Hartkopp   2012-11-21  517  
d5888a1e75c799 Arnd Bergmann     2017-11-03  518  		peak_usb_get_ts_time(&mc->pdev->time_ref, mc->ts16,
d5888a1e75c799 Arnd Bergmann     2017-11-03  519  				     &hwts->hwtstamp);
46be265d338833 Stephane Grosjean 2012-03-02  520  	}
46be265d338833 Stephane Grosjean 2012-03-02  521  
46be265d338833 Stephane Grosjean 2012-03-02  522  	mc->netdev->stats.rx_packets++;
c7b74967799b1a Oliver Hartkopp   2020-11-20 @523  	mc->netdev->stats.rx_bytes += cf->len;
                                                                                      ^^^^^^^^
No check for NULL.

1c0ee046957648 Marc Kleine-Budde 2015-07-11  524  	netif_rx(skb);
46be265d338833 Stephane Grosjean 2012-03-02  525  
46be265d338833 Stephane Grosjean 2012-03-02  526  	return 0;
46be265d338833 Stephane Grosjean 2012-03-02  527  }

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: drivers/net/can/usb/peak_usb/pcan_usb.c:523 pcan_usb_decode_error() error: we previously assumed 'cf' could be null (see line 503)
  2021-12-03 14:09 ` Dan Carpenter
  (?)
@ 2021-12-03 14:58   ` Marc Kleine-Budde
  -1 siblings, 0 replies; 11+ messages in thread
From: Marc Kleine-Budde @ 2021-12-03 14:58 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: kbuild, Stephane Grosjean, lkp, kbuild-all, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 7504 bytes --]

On 03.12.2021 17:09:55, Dan Carpenter wrote:
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head:   58e1100fdc5990b0cc0d4beaf2562a92e621ac7d
> commit: c11dcee758302702a83c6e85e4c4c3d9af42d2b3 can: peak_usb: pcan_usb_decode_error(): upgrade handling of bus state changes
> config: x86_64-randconfig-m001-20211202 (https://download.01.org/0day-ci/archive/20211202/202112021833.wABxM5UN-lkp@intel.com/config)
> compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> smatch warnings:
> drivers/net/can/usb/peak_usb/pcan_usb.c:523 pcan_usb_decode_error() error: we previously assumed 'cf' could be null (see line 503)
> 
> vim +/cf +523 drivers/net/can/usb/peak_usb/pcan_usb.c
> 
> 46be265d338833 Stephane Grosjean 2012-03-02  450  static int pcan_usb_decode_error(struct pcan_usb_msg_context *mc, u8 n,
> 46be265d338833 Stephane Grosjean 2012-03-02  451  				 u8 status_len)
> 46be265d338833 Stephane Grosjean 2012-03-02  452  {
> 46be265d338833 Stephane Grosjean 2012-03-02  453  	struct sk_buff *skb;
> 46be265d338833 Stephane Grosjean 2012-03-02  454  	struct can_frame *cf;
> c11dcee7583027 Stephane Grosjean 2021-07-15  455  	enum can_state new_state = CAN_STATE_ERROR_ACTIVE;
> 46be265d338833 Stephane Grosjean 2012-03-02  456  
> 46be265d338833 Stephane Grosjean 2012-03-02  457  	/* ignore this error until 1st ts received */
> 46be265d338833 Stephane Grosjean 2012-03-02  458  	if (n == PCAN_USB_ERROR_QOVR)
> 46be265d338833 Stephane Grosjean 2012-03-02  459  		if (!mc->pdev->time_ref.tick_count)
> 46be265d338833 Stephane Grosjean 2012-03-02  460  			return 0;
> 46be265d338833 Stephane Grosjean 2012-03-02  461  
> c11dcee7583027 Stephane Grosjean 2021-07-15  462  	/* allocate an skb to store the error frame */
> c11dcee7583027 Stephane Grosjean 2021-07-15  463  	skb = alloc_can_err_skb(mc->netdev, &cf);

alloc_can_err_skb() ->
alloc_canfd_skb()

https://elixir.bootlin.com/linux/v5.15/source/drivers/net/can/dev/skb.c#L210

If skb is NULL, cf is set to NULL, too.

> 46be265d338833 Stephane Grosjean 2012-03-02  464  
> c11dcee7583027 Stephane Grosjean 2021-07-15  465  	if (n & PCAN_USB_ERROR_RXQOVR) {
> c11dcee7583027 Stephane Grosjean 2021-07-15  466  		/* data overrun interrupt */
> c11dcee7583027 Stephane Grosjean 2021-07-15  467  		netdev_dbg(mc->netdev, "data overrun interrupt\n");
> c11dcee7583027 Stephane Grosjean 2021-07-15  468  		mc->netdev->stats.rx_over_errors++;
> c11dcee7583027 Stephane Grosjean 2021-07-15  469  		mc->netdev->stats.rx_errors++;
> c11dcee7583027 Stephane Grosjean 2021-07-15  470  		if (cf) {
> 
> Check for NULL
> 
> c11dcee7583027 Stephane Grosjean 2021-07-15  471  			cf->can_id |= CAN_ERR_CRTL;
> c11dcee7583027 Stephane Grosjean 2021-07-15  472  			cf->data[1] |= CAN_ERR_CRTL_RX_OVERFLOW;
> 46be265d338833 Stephane Grosjean 2012-03-02  473  		}
> 46be265d338833 Stephane Grosjean 2012-03-02  474  	}
> 46be265d338833 Stephane Grosjean 2012-03-02  475  
> c11dcee7583027 Stephane Grosjean 2021-07-15  476  	if (n & PCAN_USB_ERROR_TXQFULL)
> c11dcee7583027 Stephane Grosjean 2021-07-15  477  		netdev_dbg(mc->netdev, "device Tx queue full)\n");
> c11dcee7583027 Stephane Grosjean 2021-07-15  478  
> 46be265d338833 Stephane Grosjean 2012-03-02  479  	if (n & PCAN_USB_ERROR_BUS_OFF) {
> 46be265d338833 Stephane Grosjean 2012-03-02  480  		new_state = CAN_STATE_BUS_OFF;
> c11dcee7583027 Stephane Grosjean 2021-07-15  481  	} else if (n & PCAN_USB_ERROR_BUS_HEAVY) {
> c11dcee7583027 Stephane Grosjean 2021-07-15  482  		new_state = ((mc->pdev->bec.txerr >= 128) ||
> c11dcee7583027 Stephane Grosjean 2021-07-15  483  			     (mc->pdev->bec.rxerr >= 128)) ?
> c11dcee7583027 Stephane Grosjean 2021-07-15  484  				CAN_STATE_ERROR_PASSIVE :
> c11dcee7583027 Stephane Grosjean 2021-07-15  485  				CAN_STATE_ERROR_WARNING;
> c11dcee7583027 Stephane Grosjean 2021-07-15  486  	} else {
> c11dcee7583027 Stephane Grosjean 2021-07-15  487  		new_state = CAN_STATE_ERROR_ACTIVE;
> 46be265d338833 Stephane Grosjean 2012-03-02  488  	}
> 46be265d338833 Stephane Grosjean 2012-03-02  489  
> c11dcee7583027 Stephane Grosjean 2021-07-15  490  	/* handle change of state */
> c11dcee7583027 Stephane Grosjean 2021-07-15  491  	if (new_state != mc->pdev->dev.can.state) {
> c11dcee7583027 Stephane Grosjean 2021-07-15  492  		enum can_state tx_state =
> c11dcee7583027 Stephane Grosjean 2021-07-15  493  			(mc->pdev->bec.txerr >= mc->pdev->bec.rxerr) ?
> c11dcee7583027 Stephane Grosjean 2021-07-15  494  				new_state : 0;
> c11dcee7583027 Stephane Grosjean 2021-07-15  495  		enum can_state rx_state =
> c11dcee7583027 Stephane Grosjean 2021-07-15  496  			(mc->pdev->bec.txerr <= mc->pdev->bec.rxerr) ?
> c11dcee7583027 Stephane Grosjean 2021-07-15  497  				new_state : 0;
> 46be265d338833 Stephane Grosjean 2012-03-02  498  
> c11dcee7583027 Stephane Grosjean 2021-07-15  499  		can_change_state(mc->netdev, cf, tx_state, rx_state);
> 46be265d338833 Stephane Grosjean 2012-03-02  500  
> c11dcee7583027 Stephane Grosjean 2021-07-15  501  		if (new_state == CAN_STATE_BUS_OFF) {
> 46be265d338833 Stephane Grosjean 2012-03-02  502  			can_bus_off(mc->netdev);
> c11dcee7583027 Stephane Grosjean 2021-07-15 @503  		} else if (cf && (cf->can_id & CAN_ERR_CRTL)) {
> 
> Check for NULL
> 
> c11dcee7583027 Stephane Grosjean 2021-07-15  504  			/* Supply TX/RX error counters in case of
> c11dcee7583027 Stephane Grosjean 2021-07-15  505  			 * controller error.
> c11dcee7583027 Stephane Grosjean 2021-07-15  506  			 */
> ea8b33bde76c8f Stephane Grosjean 2019-12-06  507  			cf->data[6] = mc->pdev->bec.txerr;
> ea8b33bde76c8f Stephane Grosjean 2019-12-06  508  			cf->data[7] = mc->pdev->bec.rxerr;
> ea8b33bde76c8f Stephane Grosjean 2019-12-06  509  		}
> 46be265d338833 Stephane Grosjean 2012-03-02  510  	}
> 46be265d338833 Stephane Grosjean 2012-03-02  511  
> c11dcee7583027 Stephane Grosjean 2021-07-15  512  	if (!skb)
> c11dcee7583027 Stephane Grosjean 2021-07-15  513  		return -ENOMEM;

If cf is NULL, so is skb....

> 46be265d338833 Stephane Grosjean 2012-03-02  514  
> 46be265d338833 Stephane Grosjean 2012-03-02  515  	if (status_len & PCAN_USB_STATUSLEN_TIMESTAMP) {
> c9faaa09e2a133 Oliver Hartkopp   2012-11-21  516  		struct skb_shared_hwtstamps *hwts = skb_hwtstamps(skb);
> c9faaa09e2a133 Oliver Hartkopp   2012-11-21  517  
> d5888a1e75c799 Arnd Bergmann     2017-11-03  518  		peak_usb_get_ts_time(&mc->pdev->time_ref, mc->ts16,
> d5888a1e75c799 Arnd Bergmann     2017-11-03  519  				     &hwts->hwtstamp);
> 46be265d338833 Stephane Grosjean 2012-03-02  520  	}
> 46be265d338833 Stephane Grosjean 2012-03-02  521  
> 46be265d338833 Stephane Grosjean 2012-03-02  522  	mc->netdev->stats.rx_packets++;
> c7b74967799b1a Oliver Hartkopp   2020-11-20 @523  	mc->netdev->stats.rx_bytes += cf->len;
>                                                                                       ^^^^^^^^
> No check for NULL.

...then this code is not reached.

Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde           |
Embedded Linux                   | https://www.pengutronix.de  |
Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: drivers/net/can/usb/peak_usb/pcan_usb.c:523 pcan_usb_decode_error() error: we previously assumed 'cf' could be null (see line 503)
@ 2021-12-03 14:58   ` Marc Kleine-Budde
  0 siblings, 0 replies; 11+ messages in thread
From: Marc Kleine-Budde @ 2021-12-03 14:58 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 7506 bytes --]

On 03.12.2021 17:09:55, Dan Carpenter wrote:
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head:   58e1100fdc5990b0cc0d4beaf2562a92e621ac7d
> commit: c11dcee758302702a83c6e85e4c4c3d9af42d2b3 can: peak_usb: pcan_usb_decode_error(): upgrade handling of bus state changes
> config: x86_64-randconfig-m001-20211202 (https://download.01.org/0day-ci/archive/20211202/202112021833.wABxM5UN-lkp(a)intel.com/config)
> compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> smatch warnings:
> drivers/net/can/usb/peak_usb/pcan_usb.c:523 pcan_usb_decode_error() error: we previously assumed 'cf' could be null (see line 503)
> 
> vim +/cf +523 drivers/net/can/usb/peak_usb/pcan_usb.c
> 
> 46be265d338833 Stephane Grosjean 2012-03-02  450  static int pcan_usb_decode_error(struct pcan_usb_msg_context *mc, u8 n,
> 46be265d338833 Stephane Grosjean 2012-03-02  451  				 u8 status_len)
> 46be265d338833 Stephane Grosjean 2012-03-02  452  {
> 46be265d338833 Stephane Grosjean 2012-03-02  453  	struct sk_buff *skb;
> 46be265d338833 Stephane Grosjean 2012-03-02  454  	struct can_frame *cf;
> c11dcee7583027 Stephane Grosjean 2021-07-15  455  	enum can_state new_state = CAN_STATE_ERROR_ACTIVE;
> 46be265d338833 Stephane Grosjean 2012-03-02  456  
> 46be265d338833 Stephane Grosjean 2012-03-02  457  	/* ignore this error until 1st ts received */
> 46be265d338833 Stephane Grosjean 2012-03-02  458  	if (n == PCAN_USB_ERROR_QOVR)
> 46be265d338833 Stephane Grosjean 2012-03-02  459  		if (!mc->pdev->time_ref.tick_count)
> 46be265d338833 Stephane Grosjean 2012-03-02  460  			return 0;
> 46be265d338833 Stephane Grosjean 2012-03-02  461  
> c11dcee7583027 Stephane Grosjean 2021-07-15  462  	/* allocate an skb to store the error frame */
> c11dcee7583027 Stephane Grosjean 2021-07-15  463  	skb = alloc_can_err_skb(mc->netdev, &cf);

alloc_can_err_skb() ->
alloc_canfd_skb()

https://elixir.bootlin.com/linux/v5.15/source/drivers/net/can/dev/skb.c#L210

If skb is NULL, cf is set to NULL, too.

> 46be265d338833 Stephane Grosjean 2012-03-02  464  
> c11dcee7583027 Stephane Grosjean 2021-07-15  465  	if (n & PCAN_USB_ERROR_RXQOVR) {
> c11dcee7583027 Stephane Grosjean 2021-07-15  466  		/* data overrun interrupt */
> c11dcee7583027 Stephane Grosjean 2021-07-15  467  		netdev_dbg(mc->netdev, "data overrun interrupt\n");
> c11dcee7583027 Stephane Grosjean 2021-07-15  468  		mc->netdev->stats.rx_over_errors++;
> c11dcee7583027 Stephane Grosjean 2021-07-15  469  		mc->netdev->stats.rx_errors++;
> c11dcee7583027 Stephane Grosjean 2021-07-15  470  		if (cf) {
> 
> Check for NULL
> 
> c11dcee7583027 Stephane Grosjean 2021-07-15  471  			cf->can_id |= CAN_ERR_CRTL;
> c11dcee7583027 Stephane Grosjean 2021-07-15  472  			cf->data[1] |= CAN_ERR_CRTL_RX_OVERFLOW;
> 46be265d338833 Stephane Grosjean 2012-03-02  473  		}
> 46be265d338833 Stephane Grosjean 2012-03-02  474  	}
> 46be265d338833 Stephane Grosjean 2012-03-02  475  
> c11dcee7583027 Stephane Grosjean 2021-07-15  476  	if (n & PCAN_USB_ERROR_TXQFULL)
> c11dcee7583027 Stephane Grosjean 2021-07-15  477  		netdev_dbg(mc->netdev, "device Tx queue full)\n");
> c11dcee7583027 Stephane Grosjean 2021-07-15  478  
> 46be265d338833 Stephane Grosjean 2012-03-02  479  	if (n & PCAN_USB_ERROR_BUS_OFF) {
> 46be265d338833 Stephane Grosjean 2012-03-02  480  		new_state = CAN_STATE_BUS_OFF;
> c11dcee7583027 Stephane Grosjean 2021-07-15  481  	} else if (n & PCAN_USB_ERROR_BUS_HEAVY) {
> c11dcee7583027 Stephane Grosjean 2021-07-15  482  		new_state = ((mc->pdev->bec.txerr >= 128) ||
> c11dcee7583027 Stephane Grosjean 2021-07-15  483  			     (mc->pdev->bec.rxerr >= 128)) ?
> c11dcee7583027 Stephane Grosjean 2021-07-15  484  				CAN_STATE_ERROR_PASSIVE :
> c11dcee7583027 Stephane Grosjean 2021-07-15  485  				CAN_STATE_ERROR_WARNING;
> c11dcee7583027 Stephane Grosjean 2021-07-15  486  	} else {
> c11dcee7583027 Stephane Grosjean 2021-07-15  487  		new_state = CAN_STATE_ERROR_ACTIVE;
> 46be265d338833 Stephane Grosjean 2012-03-02  488  	}
> 46be265d338833 Stephane Grosjean 2012-03-02  489  
> c11dcee7583027 Stephane Grosjean 2021-07-15  490  	/* handle change of state */
> c11dcee7583027 Stephane Grosjean 2021-07-15  491  	if (new_state != mc->pdev->dev.can.state) {
> c11dcee7583027 Stephane Grosjean 2021-07-15  492  		enum can_state tx_state =
> c11dcee7583027 Stephane Grosjean 2021-07-15  493  			(mc->pdev->bec.txerr >= mc->pdev->bec.rxerr) ?
> c11dcee7583027 Stephane Grosjean 2021-07-15  494  				new_state : 0;
> c11dcee7583027 Stephane Grosjean 2021-07-15  495  		enum can_state rx_state =
> c11dcee7583027 Stephane Grosjean 2021-07-15  496  			(mc->pdev->bec.txerr <= mc->pdev->bec.rxerr) ?
> c11dcee7583027 Stephane Grosjean 2021-07-15  497  				new_state : 0;
> 46be265d338833 Stephane Grosjean 2012-03-02  498  
> c11dcee7583027 Stephane Grosjean 2021-07-15  499  		can_change_state(mc->netdev, cf, tx_state, rx_state);
> 46be265d338833 Stephane Grosjean 2012-03-02  500  
> c11dcee7583027 Stephane Grosjean 2021-07-15  501  		if (new_state == CAN_STATE_BUS_OFF) {
> 46be265d338833 Stephane Grosjean 2012-03-02  502  			can_bus_off(mc->netdev);
> c11dcee7583027 Stephane Grosjean 2021-07-15 @503  		} else if (cf && (cf->can_id & CAN_ERR_CRTL)) {
> 
> Check for NULL
> 
> c11dcee7583027 Stephane Grosjean 2021-07-15  504  			/* Supply TX/RX error counters in case of
> c11dcee7583027 Stephane Grosjean 2021-07-15  505  			 * controller error.
> c11dcee7583027 Stephane Grosjean 2021-07-15  506  			 */
> ea8b33bde76c8f Stephane Grosjean 2019-12-06  507  			cf->data[6] = mc->pdev->bec.txerr;
> ea8b33bde76c8f Stephane Grosjean 2019-12-06  508  			cf->data[7] = mc->pdev->bec.rxerr;
> ea8b33bde76c8f Stephane Grosjean 2019-12-06  509  		}
> 46be265d338833 Stephane Grosjean 2012-03-02  510  	}
> 46be265d338833 Stephane Grosjean 2012-03-02  511  
> c11dcee7583027 Stephane Grosjean 2021-07-15  512  	if (!skb)
> c11dcee7583027 Stephane Grosjean 2021-07-15  513  		return -ENOMEM;

If cf is NULL, so is skb....

> 46be265d338833 Stephane Grosjean 2012-03-02  514  
> 46be265d338833 Stephane Grosjean 2012-03-02  515  	if (status_len & PCAN_USB_STATUSLEN_TIMESTAMP) {
> c9faaa09e2a133 Oliver Hartkopp   2012-11-21  516  		struct skb_shared_hwtstamps *hwts = skb_hwtstamps(skb);
> c9faaa09e2a133 Oliver Hartkopp   2012-11-21  517  
> d5888a1e75c799 Arnd Bergmann     2017-11-03  518  		peak_usb_get_ts_time(&mc->pdev->time_ref, mc->ts16,
> d5888a1e75c799 Arnd Bergmann     2017-11-03  519  				     &hwts->hwtstamp);
> 46be265d338833 Stephane Grosjean 2012-03-02  520  	}
> 46be265d338833 Stephane Grosjean 2012-03-02  521  
> 46be265d338833 Stephane Grosjean 2012-03-02  522  	mc->netdev->stats.rx_packets++;
> c7b74967799b1a Oliver Hartkopp   2020-11-20 @523  	mc->netdev->stats.rx_bytes += cf->len;
>                                                                                       ^^^^^^^^
> No check for NULL.

...then this code is not reached.

Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde           |
Embedded Linux                   | https://www.pengutronix.de  |
Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: drivers/net/can/usb/peak_usb/pcan_usb.c:523 pcan_usb_decode_error() error: we previously assumed 'cf' could be null (see line 503)
@ 2021-12-03 14:58   ` Marc Kleine-Budde
  0 siblings, 0 replies; 11+ messages in thread
From: Marc Kleine-Budde @ 2021-12-03 14:58 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: kbuild, Stephane Grosjean, lkp, kbuild-all, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 7504 bytes --]

On 03.12.2021 17:09:55, Dan Carpenter wrote:
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head:   58e1100fdc5990b0cc0d4beaf2562a92e621ac7d
> commit: c11dcee758302702a83c6e85e4c4c3d9af42d2b3 can: peak_usb: pcan_usb_decode_error(): upgrade handling of bus state changes
> config: x86_64-randconfig-m001-20211202 (https://download.01.org/0day-ci/archive/20211202/202112021833.wABxM5UN-lkp@intel.com/config)
> compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> smatch warnings:
> drivers/net/can/usb/peak_usb/pcan_usb.c:523 pcan_usb_decode_error() error: we previously assumed 'cf' could be null (see line 503)
> 
> vim +/cf +523 drivers/net/can/usb/peak_usb/pcan_usb.c
> 
> 46be265d338833 Stephane Grosjean 2012-03-02  450  static int pcan_usb_decode_error(struct pcan_usb_msg_context *mc, u8 n,
> 46be265d338833 Stephane Grosjean 2012-03-02  451  				 u8 status_len)
> 46be265d338833 Stephane Grosjean 2012-03-02  452  {
> 46be265d338833 Stephane Grosjean 2012-03-02  453  	struct sk_buff *skb;
> 46be265d338833 Stephane Grosjean 2012-03-02  454  	struct can_frame *cf;
> c11dcee7583027 Stephane Grosjean 2021-07-15  455  	enum can_state new_state = CAN_STATE_ERROR_ACTIVE;
> 46be265d338833 Stephane Grosjean 2012-03-02  456  
> 46be265d338833 Stephane Grosjean 2012-03-02  457  	/* ignore this error until 1st ts received */
> 46be265d338833 Stephane Grosjean 2012-03-02  458  	if (n == PCAN_USB_ERROR_QOVR)
> 46be265d338833 Stephane Grosjean 2012-03-02  459  		if (!mc->pdev->time_ref.tick_count)
> 46be265d338833 Stephane Grosjean 2012-03-02  460  			return 0;
> 46be265d338833 Stephane Grosjean 2012-03-02  461  
> c11dcee7583027 Stephane Grosjean 2021-07-15  462  	/* allocate an skb to store the error frame */
> c11dcee7583027 Stephane Grosjean 2021-07-15  463  	skb = alloc_can_err_skb(mc->netdev, &cf);

alloc_can_err_skb() ->
alloc_canfd_skb()

https://elixir.bootlin.com/linux/v5.15/source/drivers/net/can/dev/skb.c#L210

If skb is NULL, cf is set to NULL, too.

> 46be265d338833 Stephane Grosjean 2012-03-02  464  
> c11dcee7583027 Stephane Grosjean 2021-07-15  465  	if (n & PCAN_USB_ERROR_RXQOVR) {
> c11dcee7583027 Stephane Grosjean 2021-07-15  466  		/* data overrun interrupt */
> c11dcee7583027 Stephane Grosjean 2021-07-15  467  		netdev_dbg(mc->netdev, "data overrun interrupt\n");
> c11dcee7583027 Stephane Grosjean 2021-07-15  468  		mc->netdev->stats.rx_over_errors++;
> c11dcee7583027 Stephane Grosjean 2021-07-15  469  		mc->netdev->stats.rx_errors++;
> c11dcee7583027 Stephane Grosjean 2021-07-15  470  		if (cf) {
> 
> Check for NULL
> 
> c11dcee7583027 Stephane Grosjean 2021-07-15  471  			cf->can_id |= CAN_ERR_CRTL;
> c11dcee7583027 Stephane Grosjean 2021-07-15  472  			cf->data[1] |= CAN_ERR_CRTL_RX_OVERFLOW;
> 46be265d338833 Stephane Grosjean 2012-03-02  473  		}
> 46be265d338833 Stephane Grosjean 2012-03-02  474  	}
> 46be265d338833 Stephane Grosjean 2012-03-02  475  
> c11dcee7583027 Stephane Grosjean 2021-07-15  476  	if (n & PCAN_USB_ERROR_TXQFULL)
> c11dcee7583027 Stephane Grosjean 2021-07-15  477  		netdev_dbg(mc->netdev, "device Tx queue full)\n");
> c11dcee7583027 Stephane Grosjean 2021-07-15  478  
> 46be265d338833 Stephane Grosjean 2012-03-02  479  	if (n & PCAN_USB_ERROR_BUS_OFF) {
> 46be265d338833 Stephane Grosjean 2012-03-02  480  		new_state = CAN_STATE_BUS_OFF;
> c11dcee7583027 Stephane Grosjean 2021-07-15  481  	} else if (n & PCAN_USB_ERROR_BUS_HEAVY) {
> c11dcee7583027 Stephane Grosjean 2021-07-15  482  		new_state = ((mc->pdev->bec.txerr >= 128) ||
> c11dcee7583027 Stephane Grosjean 2021-07-15  483  			     (mc->pdev->bec.rxerr >= 128)) ?
> c11dcee7583027 Stephane Grosjean 2021-07-15  484  				CAN_STATE_ERROR_PASSIVE :
> c11dcee7583027 Stephane Grosjean 2021-07-15  485  				CAN_STATE_ERROR_WARNING;
> c11dcee7583027 Stephane Grosjean 2021-07-15  486  	} else {
> c11dcee7583027 Stephane Grosjean 2021-07-15  487  		new_state = CAN_STATE_ERROR_ACTIVE;
> 46be265d338833 Stephane Grosjean 2012-03-02  488  	}
> 46be265d338833 Stephane Grosjean 2012-03-02  489  
> c11dcee7583027 Stephane Grosjean 2021-07-15  490  	/* handle change of state */
> c11dcee7583027 Stephane Grosjean 2021-07-15  491  	if (new_state != mc->pdev->dev.can.state) {
> c11dcee7583027 Stephane Grosjean 2021-07-15  492  		enum can_state tx_state =
> c11dcee7583027 Stephane Grosjean 2021-07-15  493  			(mc->pdev->bec.txerr >= mc->pdev->bec.rxerr) ?
> c11dcee7583027 Stephane Grosjean 2021-07-15  494  				new_state : 0;
> c11dcee7583027 Stephane Grosjean 2021-07-15  495  		enum can_state rx_state =
> c11dcee7583027 Stephane Grosjean 2021-07-15  496  			(mc->pdev->bec.txerr <= mc->pdev->bec.rxerr) ?
> c11dcee7583027 Stephane Grosjean 2021-07-15  497  				new_state : 0;
> 46be265d338833 Stephane Grosjean 2012-03-02  498  
> c11dcee7583027 Stephane Grosjean 2021-07-15  499  		can_change_state(mc->netdev, cf, tx_state, rx_state);
> 46be265d338833 Stephane Grosjean 2012-03-02  500  
> c11dcee7583027 Stephane Grosjean 2021-07-15  501  		if (new_state == CAN_STATE_BUS_OFF) {
> 46be265d338833 Stephane Grosjean 2012-03-02  502  			can_bus_off(mc->netdev);
> c11dcee7583027 Stephane Grosjean 2021-07-15 @503  		} else if (cf && (cf->can_id & CAN_ERR_CRTL)) {
> 
> Check for NULL
> 
> c11dcee7583027 Stephane Grosjean 2021-07-15  504  			/* Supply TX/RX error counters in case of
> c11dcee7583027 Stephane Grosjean 2021-07-15  505  			 * controller error.
> c11dcee7583027 Stephane Grosjean 2021-07-15  506  			 */
> ea8b33bde76c8f Stephane Grosjean 2019-12-06  507  			cf->data[6] = mc->pdev->bec.txerr;
> ea8b33bde76c8f Stephane Grosjean 2019-12-06  508  			cf->data[7] = mc->pdev->bec.rxerr;
> ea8b33bde76c8f Stephane Grosjean 2019-12-06  509  		}
> 46be265d338833 Stephane Grosjean 2012-03-02  510  	}
> 46be265d338833 Stephane Grosjean 2012-03-02  511  
> c11dcee7583027 Stephane Grosjean 2021-07-15  512  	if (!skb)
> c11dcee7583027 Stephane Grosjean 2021-07-15  513  		return -ENOMEM;

If cf is NULL, so is skb....

> 46be265d338833 Stephane Grosjean 2012-03-02  514  
> 46be265d338833 Stephane Grosjean 2012-03-02  515  	if (status_len & PCAN_USB_STATUSLEN_TIMESTAMP) {
> c9faaa09e2a133 Oliver Hartkopp   2012-11-21  516  		struct skb_shared_hwtstamps *hwts = skb_hwtstamps(skb);
> c9faaa09e2a133 Oliver Hartkopp   2012-11-21  517  
> d5888a1e75c799 Arnd Bergmann     2017-11-03  518  		peak_usb_get_ts_time(&mc->pdev->time_ref, mc->ts16,
> d5888a1e75c799 Arnd Bergmann     2017-11-03  519  				     &hwts->hwtstamp);
> 46be265d338833 Stephane Grosjean 2012-03-02  520  	}
> 46be265d338833 Stephane Grosjean 2012-03-02  521  
> 46be265d338833 Stephane Grosjean 2012-03-02  522  	mc->netdev->stats.rx_packets++;
> c7b74967799b1a Oliver Hartkopp   2020-11-20 @523  	mc->netdev->stats.rx_bytes += cf->len;
>                                                                                       ^^^^^^^^
> No check for NULL.

...then this code is not reached.

Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde           |
Embedded Linux                   | https://www.pengutronix.de  |
Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: drivers/net/can/usb/peak_usb/pcan_usb.c:523 pcan_usb_decode_error() error: we previously assumed 'cf' could be null (see line 503)
  2021-12-03 14:58   ` Marc Kleine-Budde
@ 2021-12-03 15:01     ` Marc Kleine-Budde
  -1 siblings, 0 replies; 11+ messages in thread
From: Marc Kleine-Budde @ 2021-12-03 15:01 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: kbuild, Stephane Grosjean, lkp, kbuild-all, linux-kernel, linux-can

[-- Attachment #1: Type: text/plain, Size: 2600 bytes --]

On 03.12.2021 15:58:52, Marc Kleine-Budde wrote:
> On 03.12.2021 17:09:55, Dan Carpenter wrote:
> > tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> > head:   58e1100fdc5990b0cc0d4beaf2562a92e621ac7d
> > commit: c11dcee758302702a83c6e85e4c4c3d9af42d2b3 can: peak_usb: pcan_usb_decode_error(): upgrade handling of bus state changes
> > config: x86_64-randconfig-m001-20211202 (https://download.01.org/0day-ci/archive/20211202/202112021833.wABxM5UN-lkp@intel.com/config)
> > compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
> > 
> > If you fix the issue, kindly add following tag as appropriate
> > Reported-by: kernel test robot <lkp@intel.com>
> > Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> > 
> > smatch warnings:
> > drivers/net/can/usb/peak_usb/pcan_usb.c:523 pcan_usb_decode_error() error: we previously assumed 'cf' could be null (see line 503)
> > 
> > vim +/cf +523 drivers/net/can/usb/peak_usb/pcan_usb.c
> > 
> > 46be265d338833 Stephane Grosjean 2012-03-02  450  static int pcan_usb_decode_error(struct pcan_usb_msg_context *mc, u8 n,
> > 46be265d338833 Stephane Grosjean 2012-03-02  451  				 u8 status_len)
> > 46be265d338833 Stephane Grosjean 2012-03-02  452  {
> > 46be265d338833 Stephane Grosjean 2012-03-02  453  	struct sk_buff *skb;
> > 46be265d338833 Stephane Grosjean 2012-03-02  454  	struct can_frame *cf;
> > c11dcee7583027 Stephane Grosjean 2021-07-15  455  	enum can_state new_state = CAN_STATE_ERROR_ACTIVE;
> > 46be265d338833 Stephane Grosjean 2012-03-02  456  
> > 46be265d338833 Stephane Grosjean 2012-03-02  457  	/* ignore this error until 1st ts received */
> > 46be265d338833 Stephane Grosjean 2012-03-02  458  	if (n == PCAN_USB_ERROR_QOVR)
> > 46be265d338833 Stephane Grosjean 2012-03-02  459  		if (!mc->pdev->time_ref.tick_count)
> > 46be265d338833 Stephane Grosjean 2012-03-02  460  			return 0;
> > 46be265d338833 Stephane Grosjean 2012-03-02  461  
> > c11dcee7583027 Stephane Grosjean 2021-07-15  462  	/* allocate an skb to store the error frame */
> > c11dcee7583027 Stephane Grosjean 2021-07-15  463  	skb = alloc_can_err_skb(mc->netdev, &cf);
> 
> alloc_can_err_skb() ->
> alloc_canfd_skb()

Doh! It calls alloc_can_skb()

https://elixir.bootlin.com/linux/v5.15/source/drivers/net/can/dev/skb.c#L180

Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde           |
Embedded Linux                   | https://www.pengutronix.de  |
Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: drivers/net/can/usb/peak_usb/pcan_usb.c:523 pcan_usb_decode_error() error: we previously assumed 'cf' could be null (see line 503)
@ 2021-12-03 15:01     ` Marc Kleine-Budde
  0 siblings, 0 replies; 11+ messages in thread
From: Marc Kleine-Budde @ 2021-12-03 15:01 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 2602 bytes --]

On 03.12.2021 15:58:52, Marc Kleine-Budde wrote:
> On 03.12.2021 17:09:55, Dan Carpenter wrote:
> > tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> > head:   58e1100fdc5990b0cc0d4beaf2562a92e621ac7d
> > commit: c11dcee758302702a83c6e85e4c4c3d9af42d2b3 can: peak_usb: pcan_usb_decode_error(): upgrade handling of bus state changes
> > config: x86_64-randconfig-m001-20211202 (https://download.01.org/0day-ci/archive/20211202/202112021833.wABxM5UN-lkp(a)intel.com/config)
> > compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
> > 
> > If you fix the issue, kindly add following tag as appropriate
> > Reported-by: kernel test robot <lkp@intel.com>
> > Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> > 
> > smatch warnings:
> > drivers/net/can/usb/peak_usb/pcan_usb.c:523 pcan_usb_decode_error() error: we previously assumed 'cf' could be null (see line 503)
> > 
> > vim +/cf +523 drivers/net/can/usb/peak_usb/pcan_usb.c
> > 
> > 46be265d338833 Stephane Grosjean 2012-03-02  450  static int pcan_usb_decode_error(struct pcan_usb_msg_context *mc, u8 n,
> > 46be265d338833 Stephane Grosjean 2012-03-02  451  				 u8 status_len)
> > 46be265d338833 Stephane Grosjean 2012-03-02  452  {
> > 46be265d338833 Stephane Grosjean 2012-03-02  453  	struct sk_buff *skb;
> > 46be265d338833 Stephane Grosjean 2012-03-02  454  	struct can_frame *cf;
> > c11dcee7583027 Stephane Grosjean 2021-07-15  455  	enum can_state new_state = CAN_STATE_ERROR_ACTIVE;
> > 46be265d338833 Stephane Grosjean 2012-03-02  456  
> > 46be265d338833 Stephane Grosjean 2012-03-02  457  	/* ignore this error until 1st ts received */
> > 46be265d338833 Stephane Grosjean 2012-03-02  458  	if (n == PCAN_USB_ERROR_QOVR)
> > 46be265d338833 Stephane Grosjean 2012-03-02  459  		if (!mc->pdev->time_ref.tick_count)
> > 46be265d338833 Stephane Grosjean 2012-03-02  460  			return 0;
> > 46be265d338833 Stephane Grosjean 2012-03-02  461  
> > c11dcee7583027 Stephane Grosjean 2021-07-15  462  	/* allocate an skb to store the error frame */
> > c11dcee7583027 Stephane Grosjean 2021-07-15  463  	skb = alloc_can_err_skb(mc->netdev, &cf);
> 
> alloc_can_err_skb() ->
> alloc_canfd_skb()

Doh! It calls alloc_can_skb()

https://elixir.bootlin.com/linux/v5.15/source/drivers/net/can/dev/skb.c#L180

Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde           |
Embedded Linux                   | https://www.pengutronix.de  |
Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: drivers/net/can/usb/peak_usb/pcan_usb.c:523 pcan_usb_decode_error() error: we previously assumed 'cf' could be null (see line 503)
  2021-12-03 14:58   ` Marc Kleine-Budde
  (?)
@ 2021-12-03 15:14     ` Dan Carpenter
  -1 siblings, 0 replies; 11+ messages in thread
From: Dan Carpenter @ 2021-12-03 15:14 UTC (permalink / raw)
  To: Marc Kleine-Budde
  Cc: kbuild, Stephane Grosjean, lkp, kbuild-all, linux-kernel

On Fri, Dec 03, 2021 at 03:58:51PM +0100, Marc Kleine-Budde wrote:
> > 
> > 46be265d338833 Stephane Grosjean 2012-03-02  450  static int pcan_usb_decode_error(struct pcan_usb_msg_context *mc, u8 n,
> > 46be265d338833 Stephane Grosjean 2012-03-02  451  				 u8 status_len)
> > 46be265d338833 Stephane Grosjean 2012-03-02  452  {
> > 46be265d338833 Stephane Grosjean 2012-03-02  453  	struct sk_buff *skb;
> > 46be265d338833 Stephane Grosjean 2012-03-02  454  	struct can_frame *cf;
> > c11dcee7583027 Stephane Grosjean 2021-07-15  455  	enum can_state new_state = CAN_STATE_ERROR_ACTIVE;
> > 46be265d338833 Stephane Grosjean 2012-03-02  456  
> > 46be265d338833 Stephane Grosjean 2012-03-02  457  	/* ignore this error until 1st ts received */
> > 46be265d338833 Stephane Grosjean 2012-03-02  458  	if (n == PCAN_USB_ERROR_QOVR)
> > 46be265d338833 Stephane Grosjean 2012-03-02  459  		if (!mc->pdev->time_ref.tick_count)
> > 46be265d338833 Stephane Grosjean 2012-03-02  460  			return 0;
> > 46be265d338833 Stephane Grosjean 2012-03-02  461  
> > c11dcee7583027 Stephane Grosjean 2021-07-15  462  	/* allocate an skb to store the error frame */
> > c11dcee7583027 Stephane Grosjean 2021-07-15  463  	skb = alloc_can_err_skb(mc->netdev, &cf);
> 
> alloc_can_err_skb() ->
> alloc_canfd_skb()
> 
> https://elixir.bootlin.com/linux/v5.15/source/drivers/net/can/dev/skb.c#L210
> 
> If skb is NULL, cf is set to NULL, too.

Yeah.  Sorry about that.  The kbuild-bot doesn't do much cross function
analysis (none for functions that are not inline).  I saw that this
patch was old and could easily have checked it on my system which has
the cross function DB but I was in a rush.  :/  My bad.  Will try be a
more diligent person next time.

regards,
dan carpenter



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: drivers/net/can/usb/peak_usb/pcan_usb.c:523 pcan_usb_decode_error() error: we previously assumed 'cf' could be null (see line 503)
@ 2021-12-03 15:14     ` Dan Carpenter
  0 siblings, 0 replies; 11+ messages in thread
From: Dan Carpenter @ 2021-12-03 15:14 UTC (permalink / raw)
  To: kbuild

[-- Attachment #1: Type: text/plain, Size: 1786 bytes --]

On Fri, Dec 03, 2021 at 03:58:51PM +0100, Marc Kleine-Budde wrote:
> > 
> > 46be265d338833 Stephane Grosjean 2012-03-02  450  static int pcan_usb_decode_error(struct pcan_usb_msg_context *mc, u8 n,
> > 46be265d338833 Stephane Grosjean 2012-03-02  451  				 u8 status_len)
> > 46be265d338833 Stephane Grosjean 2012-03-02  452  {
> > 46be265d338833 Stephane Grosjean 2012-03-02  453  	struct sk_buff *skb;
> > 46be265d338833 Stephane Grosjean 2012-03-02  454  	struct can_frame *cf;
> > c11dcee7583027 Stephane Grosjean 2021-07-15  455  	enum can_state new_state = CAN_STATE_ERROR_ACTIVE;
> > 46be265d338833 Stephane Grosjean 2012-03-02  456  
> > 46be265d338833 Stephane Grosjean 2012-03-02  457  	/* ignore this error until 1st ts received */
> > 46be265d338833 Stephane Grosjean 2012-03-02  458  	if (n == PCAN_USB_ERROR_QOVR)
> > 46be265d338833 Stephane Grosjean 2012-03-02  459  		if (!mc->pdev->time_ref.tick_count)
> > 46be265d338833 Stephane Grosjean 2012-03-02  460  			return 0;
> > 46be265d338833 Stephane Grosjean 2012-03-02  461  
> > c11dcee7583027 Stephane Grosjean 2021-07-15  462  	/* allocate an skb to store the error frame */
> > c11dcee7583027 Stephane Grosjean 2021-07-15  463  	skb = alloc_can_err_skb(mc->netdev, &cf);
> 
> alloc_can_err_skb() ->
> alloc_canfd_skb()
> 
> https://elixir.bootlin.com/linux/v5.15/source/drivers/net/can/dev/skb.c#L210
> 
> If skb is NULL, cf is set to NULL, too.

Yeah.  Sorry about that.  The kbuild-bot doesn't do much cross function
analysis (none for functions that are not inline).  I saw that this
patch was old and could easily have checked it on my system which has
the cross function DB but I was in a rush.  :/  My bad.  Will try be a
more diligent person next time.

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: drivers/net/can/usb/peak_usb/pcan_usb.c:523 pcan_usb_decode_error() error: we previously assumed 'cf' could be null (see line 503)
@ 2021-12-03 15:14     ` Dan Carpenter
  0 siblings, 0 replies; 11+ messages in thread
From: Dan Carpenter @ 2021-12-03 15:14 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 1786 bytes --]

On Fri, Dec 03, 2021 at 03:58:51PM +0100, Marc Kleine-Budde wrote:
> > 
> > 46be265d338833 Stephane Grosjean 2012-03-02  450  static int pcan_usb_decode_error(struct pcan_usb_msg_context *mc, u8 n,
> > 46be265d338833 Stephane Grosjean 2012-03-02  451  				 u8 status_len)
> > 46be265d338833 Stephane Grosjean 2012-03-02  452  {
> > 46be265d338833 Stephane Grosjean 2012-03-02  453  	struct sk_buff *skb;
> > 46be265d338833 Stephane Grosjean 2012-03-02  454  	struct can_frame *cf;
> > c11dcee7583027 Stephane Grosjean 2021-07-15  455  	enum can_state new_state = CAN_STATE_ERROR_ACTIVE;
> > 46be265d338833 Stephane Grosjean 2012-03-02  456  
> > 46be265d338833 Stephane Grosjean 2012-03-02  457  	/* ignore this error until 1st ts received */
> > 46be265d338833 Stephane Grosjean 2012-03-02  458  	if (n == PCAN_USB_ERROR_QOVR)
> > 46be265d338833 Stephane Grosjean 2012-03-02  459  		if (!mc->pdev->time_ref.tick_count)
> > 46be265d338833 Stephane Grosjean 2012-03-02  460  			return 0;
> > 46be265d338833 Stephane Grosjean 2012-03-02  461  
> > c11dcee7583027 Stephane Grosjean 2021-07-15  462  	/* allocate an skb to store the error frame */
> > c11dcee7583027 Stephane Grosjean 2021-07-15  463  	skb = alloc_can_err_skb(mc->netdev, &cf);
> 
> alloc_can_err_skb() ->
> alloc_canfd_skb()
> 
> https://elixir.bootlin.com/linux/v5.15/source/drivers/net/can/dev/skb.c#L210
> 
> If skb is NULL, cf is set to NULL, too.

Yeah.  Sorry about that.  The kbuild-bot doesn't do much cross function
analysis (none for functions that are not inline).  I saw that this
patch was old and could easily have checked it on my system which has
the cross function DB but I was in a rush.  :/  My bad.  Will try be a
more diligent person next time.

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2021-12-03 15:16 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-02 10:24 drivers/net/can/usb/peak_usb/pcan_usb.c:523 pcan_usb_decode_error() error: we previously assumed 'cf' could be null (see line 503) kernel test robot
2021-12-03 14:09 ` Dan Carpenter
2021-12-03 14:09 ` Dan Carpenter
2021-12-03 14:58 ` Marc Kleine-Budde
2021-12-03 14:58   ` Marc Kleine-Budde
2021-12-03 14:58   ` Marc Kleine-Budde
2021-12-03 15:01   ` Marc Kleine-Budde
2021-12-03 15:01     ` Marc Kleine-Budde
2021-12-03 15:14   ` Dan Carpenter
2021-12-03 15:14     ` Dan Carpenter
2021-12-03 15:14     ` Dan Carpenter

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.