Re: [driver-core:driver-core-testing 12/18] drivers/base/core.c:286 fw_devlink_refresh_fwnode() error: we previously assumed 'dev' could be null (see line 270)

[Herve Codina <herve.codina@bootlin.com>]

Hi Dan,

On Sat, 23 May 2026 12:46:14 +0300
Dan Carpenter <error27@gmail.com> wrote:

> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/driver-core/driver-core.git driver-core-testing
> head:   024480bf8d75bd16894c5b0eb6082b6e6dae4970
> commit: 81e7c6befa36cecdcbf7244393bd67e8f8c59bf5 [12/18] of: dynamic: Fix overlayed devices not probing because of fw_devlink
> config: arm-randconfig-r071-20260523 (https://download.01.org/0day-ci/archive/20260523/202605230211.567ddMEp-lkp@intel.com/config)
> compiler: clang version 23.0.0git (https://github.com/llvm/llvm-project 5bac06718f502014fade905512f1d26d578a18f3)
> smatch: v0.5.0-9185-gbcc58b9c
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Reported-by: Dan Carpenter <error27@gmail.com>
> | Closes: https://lore.kernel.org/r/202605230211.567ddMEp-lkp@intel.com/
> 
> New smatch warnings:
> drivers/base/core.c:286 fw_devlink_refresh_fwnode() error: we previously assumed 'dev' could be null (see line 270)
> 
> vim +/dev +286 drivers/base/core.c
> 
> 81e7c6befa36ce Saravana Kannan 2026-05-11  256  void fw_devlink_refresh_fwnode(struct fwnode_handle *fwnode)
> 81e7c6befa36ce Saravana Kannan 2026-05-11  257  {
> 81e7c6befa36ce Saravana Kannan 2026-05-11  258  	struct device *dev;
> 81e7c6befa36ce Saravana Kannan 2026-05-11  259  
> 81e7c6befa36ce Saravana Kannan 2026-05-11  260  	/*
> 81e7c6befa36ce Saravana Kannan 2026-05-11  261  	 * Find the closest ancestor fwnode that has been converted to a device
> 81e7c6befa36ce Saravana Kannan 2026-05-11  262  	 * that can bind to a driver (bus device).
> 81e7c6befa36ce Saravana Kannan 2026-05-11  263  	 */
> 81e7c6befa36ce Saravana Kannan 2026-05-11  264  	fwnode_handle_get(fwnode);
> 81e7c6befa36ce Saravana Kannan 2026-05-11  265  	do {
> 81e7c6befa36ce Saravana Kannan 2026-05-11  266  		if (fwnode_test_flag(fwnode, FWNODE_FLAG_NOT_DEVICE))
> 81e7c6befa36ce Saravana Kannan 2026-05-11  267  			continue;
> 81e7c6befa36ce Saravana Kannan 2026-05-11  268  
> 81e7c6befa36ce Saravana Kannan 2026-05-11  269  		dev = get_dev_from_fwnode(fwnode);
> 81e7c6befa36ce Saravana Kannan 2026-05-11 @270  		if (!dev)
> 81e7c6befa36ce Saravana Kannan 2026-05-11  271  			continue;
> 
> Smatch doesn't like how this code tests for NULL and but then later
> we dereference "dev" without testing.
> 
> get_dev_from_fwnode() can only return NULL if "fwnode" is NULL which
> it can't be.  We can just remove this test.

No so sure.
  --- 8< ---
  struct device *get_dev_from_fwnode(struct fwnode_handle *fwnode)
  {
	return get_device((fwnode)->dev);
  }
  EXPORT_SYMBOL_GPL(get_dev_from_fwnode);
  --- 8< ---

get_dev_from_fwnode() returns NULL if no device is attached to the
given fwnode. This is what we test here.

At line 286, when dev is derefenced (dev->links.status), it cannot be NULL.

> 
> 81e7c6befa36ce Saravana Kannan 2026-05-11  272  
> 81e7c6befa36ce Saravana Kannan 2026-05-11  273  		if (dev->bus)
> 81e7c6befa36ce Saravana Kannan 2026-05-11  274  			break;
> 81e7c6befa36ce Saravana Kannan 2026-05-11  275  
> 81e7c6befa36ce Saravana Kannan 2026-05-11  276  		put_device(dev);
> 81e7c6befa36ce Saravana Kannan 2026-05-11  277  	} while ((fwnode = fwnode_get_next_parent(fwnode)));

We go out of the while() loop here either :
 - because dev->bus is not NULL (the break in the loop) and so dev is not
   NULL in that case (tested just before)
or
 - because fwnode is NULL

> 81e7c6befa36ce Saravana Kannan 2026-05-11  278  
> 81e7c6befa36ce Saravana Kannan 2026-05-11  279  	/*
> 81e7c6befa36ce Saravana Kannan 2026-05-11  280  	 * If none of the ancestor fwnodes have (yet) been converted to a device
> 81e7c6befa36ce Saravana Kannan 2026-05-11  281  	 * that can bind to a driver, there's nothing to fix up.
> 81e7c6befa36ce Saravana Kannan 2026-05-11  282  	 */
> 81e7c6befa36ce Saravana Kannan 2026-05-11  283  	if (!fwnode)
> 81e7c6befa36ce Saravana Kannan 2026-05-11  284  		return;

The case fwnode == NULL is handled here.
At this point dev cannot be NULL. Indeed, the loop has been exited because
dev->bus was not NULL (the break).

> 81e7c6befa36ce Saravana Kannan 2026-05-11  285  
> 81e7c6befa36ce Saravana Kannan 2026-05-11 @286  	WARN(device_is_bound(dev) && dev->links.status != DL_DEV_DRIVER_BOUND,
>                                                                                      ^^^^^^^^^^^^^^^^^
> 
> 81e7c6befa36ce Saravana Kannan 2026-05-11  287  	     "Don't multithread overlaying and probing the same device!\n");
> 81e7c6befa36ce Saravana Kannan 2026-05-11  288  
> 81e7c6befa36ce Saravana Kannan 2026-05-11  289  	/*
> 81e7c6befa36ce Saravana Kannan 2026-05-11  290  	 * If the device has already bound to a driver, then we need to redo
> 
> --
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki
> 

I don't understand why smatch reported a problem.
Do I miss something?

Best regards,
Hervé