Re: the return values of dev_mc_add and dev_mc_delete

From: Randy.Dunlap
Date: Sat Feb 28 2004 - 01:34:05 EST


| Hello,
|
| In net/core/dev.c I found this lines in function dev_ifsioc:
|
| case SIOCADDMULTI:
| if (!dev->set_multicast_list ||
| ifr->ifr_hwaddr.sa_family != AF_UNSPEC)
| return -EINVAL;
| if (!netif_device_present(dev))
| return -ENODEV;
| --> dev_mc_add(dev, ifr->ifr_hwaddr.sa_data,
| dev->addr_len, 1);
| --> return 0;
|
| case SIOCDELMULTI:
| if (!dev->set_multicast_list ||
| ifr->ifr_hwaddr.sa_family != AF_UNSPEC)
| return -EINVAL;
| if (!netif_device_present(dev))
| return -ENODEV;
| --> dev_mc_delete(dev, ifr->ifr_hwaddr.sa_data,
| dev->addr_len, 1);
| --> return 0;
|
| As I understand functions dev_mc_add and dev_mc_delete return values
| including different errors. But this values are not checked in this code.
| This causes for example returning success when trying to delete a non-exiting
| muticast address. It could be fixed this way :
|
| case SIOCADDMULTI:
| if (!dev->set_multicast_list ||
| ifr->ifr_hwaddr.sa_family != AF_UNSPEC)
| return -EINVAL;
| if (!netif_device_present(dev))
| return -ENODEV;
| --> return dev_mc_add(dev, ifr->ifr_hwaddr.sa_data,
| dev->addr_len, 1);
|
| case SIOCDELMULTI:
| if (!dev->set_multicast_list ||
| ifr->ifr_hwaddr.sa_family != AF_UNSPEC)
| return -EINVAL;
| if (!netif_device_present(dev))
| return -ENODEV;
| --> return dev_mc_delete(dev, ifr->ifr_hwaddr.sa_data,
| dev->addr_len, 1);
|
| There are a few more functions in kernel (with dev_mc_add and dev_mc_delete)
| where you can find similar code. If the described situation is really error
| then this fuctions should be fixed too. I used 2.6.2 and 2.4.18 kernels.

I see. So why do all callers of dev_mc_add() and dev_mc_delete()
not check the return value of those functions?? Anyone?

I agree, it looks like dev_ifsioc() could/should return those
results.

--
~Randy
-
To unsubscribe from this list: send the line "unsubscribe linux-net" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html