[bug report] cfg80211: avoid holding the RTNL when calling the driver

[bug report] cfg80211: avoid holding the RTNL when calling the driver

[Dan Carpenter]

Hello Johannes Berg,

This is a semi-automatic email about new static checker warnings.

The patch 791daf8fc49a: "cfg80211: avoid holding the RTNL when 
calling the driver" from Jan 19, 2021, leads to the following Smatch 
complaint:

    net/wireless/nl80211.c:3242 nl80211_set_wiphy()
    error: we previously assumed 'rdev' could be null (see line 3222)

net/wireless/nl80211.c
  3221	
  3222		if (rdev)
                    ^^^^
The patch adds a NULL dereference

  3223			mutex_lock(&rdev->wiphy.mtx);
  3224		rtnl_unlock();
  3225	
  3226		/*
  3227		 * end workaround code, by now the rdev is available
  3228		 * and locked, and wdev may or may not be NULL.
  3229		 */
  3230	
  3231		if (info->attrs[NL80211_ATTR_WIPHY_NAME])
  3232			result = cfg80211_dev_rename(
  3233				rdev, nla_data(info->attrs[NL80211_ATTR_WIPHY_NAME]));
  3234	
  3235		if (result)
  3236			goto out;
  3237	
  3238		if (info->attrs[NL80211_ATTR_WIPHY_TXQ_PARAMS]) {
  3239			struct ieee80211_txq_params txq_params;
  3240			struct nlattr *tb[NL80211_TXQ_ATTR_MAX + 1];
  3241	
  3242			if (!rdev->ops->set_txq_params) {
                             ^^^^^^
But it's not checked here.

  3243				result = -EOPNOTSUPP;
  3244				goto out;

regards,
dan carpenter

Re: [bug report] cfg80211: avoid holding the RTNL when calling the driver

[Johannes Berg]

Hi,

On Mon, 2021-01-25 at 12:07 +0300, Dan Carpenter wrote:
> Hello Johannes Berg,
> 
> This is a semi-automatic email about new static checker warnings.
> 
> The patch 791daf8fc49a: "cfg80211: avoid holding the RTNL when 
> calling the driver" from Jan 19, 2021, leads to the following Smatch 
> complaint:
> 
>     net/wireless/nl80211.c:3242 nl80211_set_wiphy()
>     error: we previously assumed 'rdev' could be null (see line 3222)
> 
> net/wireless/nl80211.c
>   3221	
>   3222		if (rdev)
>                     ^^^^
> The patch adds a NULL dereference
> 
>   3223			mutex_lock(&rdev->wiphy.mtx);

Yeah. I realized this later, the "if" here is wrong. Current version has
this unconditional :-)

johannes