[bug report] net: fix dev_ifsioc_locked() race condition

Hello Cong Wang,

The patch 3b23a32a6321: "net: fix dev_ifsioc_locked() race condition"
from Feb 11, 2021, leads to the following static checker warning:

	drivers/net/tap.c:1095 tap_ioctl()
	warn: check that 'sa.sa_family' doesn't leak information

drivers/net/tap.c
  1084  
  1085          case SIOCGIFHWADDR:
  1086                  rtnl_lock();
  1087                  tap = tap_get_tap_dev(q);
  1088                  if (!tap) {
  1089                          rtnl_unlock();
  1090                          return -ENOLINK;
  1091                  }
  1092                  ret = 0;
  1093                  dev_get_mac_address(&sa, dev_net(tap->dev), tap->dev->name);

How do you want to handle errors from dev_get_mac_address()?

  1094                  if (copy_to_user(&ifr->ifr_name, tap->dev->name, IFNAMSIZ) ||
  1095                      copy_to_user(&ifr->ifr_hwaddr, &sa, sizeof(sa)))
  1096                          ret = -EFAULT;
  1097                  tap_put_tap_dev(tap);
  1098                  rtnl_unlock();
  1099                  return ret;
  1100  

regards,
dan carpenter

On Tue, Feb 16, 2021 at 11:40 AM Dan Carpenter [off-list ref] wrote:

Hello Cong Wang,

The patch 3b23a32a6321: "net: fix dev_ifsioc_locked() race condition"
from Feb 11, 2021, leads to the following static checker warning:

        drivers/net/tap.c:1095 tap_ioctl()
        warn: check that 'sa.sa_family' doesn't leak information

drivers/net/tap.c
  1084
  1085          case SIOCGIFHWADDR:
  1086                  rtnl_lock();
  1087                  tap = tap_get_tap_dev(q);
  1088                  if (!tap) {
  1089                          rtnl_unlock();
  1090                          return -ENOLINK;
  1091                  }
  1092                  ret = 0;
  1093                  dev_get_mac_address(&sa, dev_net(tap->dev), tap->dev->name);

How do you want to handle errors from dev_get_mac_address()?

I did have a check there but it turns out unnecessary, because the only
error can happen is when the device name is not found, but here
we already have tap->dev, therefore its name is always there and valid.

Thanks.