| Message ID | abef9f7ad2c6e6b8ae053225766d34f3fc930ddf.1647664114.git.william.xuanziyang@huawei.com (mailing list archive) |
|---|---|
| State | Deferred |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: ipvlan: fix potential UAF problem for phy_dev | expand |
On 3/19/22 02:52, Ziyang Xuan wrote: > There is a known scenario can trigger UAF problem for lower > netdevice as following: But this known scenario never happens. So maybe you should not add it 'in the future' > > Someone module puts the NETDEV_UNREGISTER event handler to a > work, and lower netdevice is accessed in the work handler. But > when the work is excuted, lower netdevice has been destroyed > because upper netdevice did not get reference to lower netdevice > correctly. Again, whoever would like to use a work queue would also need to hold a reference on the device. Regardless of ipvlan being used or not. > > Although it can not happen for ipvlan now because there is no > way to access phy_dev outside ipvlan. But it is necessary to > add the reference operation to phy_dev to avoid the potential > UAF problem in the future. > > Signed-off-by: Ziyang Xuan <william.xuanziyang@huawei.com> > --- Your patch makes no sense to me really.
diff --git a/drivers/net/ipvlan/ipvlan_main.c b/drivers/net/ipvlan/ipvlan_main.c index 696e245f6d00..dcdc01403f22 100644 --- a/drivers/net/ipvlan/ipvlan_main.c +++ b/drivers/net/ipvlan/ipvlan_main.c @@ -158,6 +158,10 @@ static int ipvlan_init(struct net_device *dev) } port = ipvlan_port_get_rtnl(phy_dev); port->count += 1; + + /* Get ipvlan's reference to phy_dev */ + dev_hold(phy_dev); + return 0; } @@ -665,6 +669,14 @@ void ipvlan_link_delete(struct net_device *dev, struct list_head *head) } EXPORT_SYMBOL_GPL(ipvlan_link_delete); +static void ipvlan_dev_free(struct net_device *dev) +{ + struct ipvl_dev *ipvlan = netdev_priv(dev); + + /* Get rid of the ipvlan's reference to phy_dev */ + dev_put(ipvlan->phy_dev); +} + void ipvlan_link_setup(struct net_device *dev) { ether_setup(dev); @@ -674,6 +686,7 @@ void ipvlan_link_setup(struct net_device *dev) dev->priv_flags |= IFF_UNICAST_FLT | IFF_NO_QUEUE; dev->netdev_ops = &ipvlan_netdev_ops; dev->needs_free_netdev = true; + dev->priv_destructor = ipvlan_dev_free; dev->header_ops = &ipvlan_header_ops; dev->ethtool_ops = &ipvlan_ethtool_ops; }
There is a known scenario can trigger UAF problem for lower netdevice as following: Someone module puts the NETDEV_UNREGISTER event handler to a work, and lower netdevice is accessed in the work handler. But when the work is excuted, lower netdevice has been destroyed because upper netdevice did not get reference to lower netdevice correctly. Although it can not happen for ipvlan now because there is no way to access phy_dev outside ipvlan. But it is necessary to add the reference operation to phy_dev to avoid the potential UAF problem in the future. Signed-off-by: Ziyang Xuan <william.xuanziyang@huawei.com> --- drivers/net/ipvlan/ipvlan_main.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)