Skip to contentSkip to conversation

cfg80211: avoid holding the RTNL when calling the driver

bug report Dan Carpenter 2 participants 2 messages Jan 25, 2021 – Jan 26, 2021

lore
Share & tools
lore
#0 lore raw

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

#1 lore raw

Hi,

Quoted 11 lines · 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)

Quoted 7 lines

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