Re: [PATCH RFC v2] ipvlan: keep lower device alive until private destruction
Krystian Kaniewski <[email protected]> Sun, 19 Jul 2026 14:09:32 +0200
| Newsgroups | dev.linux.lists.syzbot |
|---|---|
| Message-ID | <[email protected]> |
Please generate a message-only v3 for syzbot issue 5fe14f2ff4ccbace9a26,
based on AI patch v2 from job ecee4f7f-c35a-40b5-85ed-47bebd5cb0d1 and
kernel commit 8cdeaa50eae8dad34885515f62559ee83e7e8dda.
Keep the code diff unchanged. The implementation correctly gives each
struct ipvl_dev its own tracked reference to phy_dev, takes it at the
successful end of ipvlan_init(), and releases it from the
ipvlan_dev_free() private destructor. This also covers ipvtap. Keep the
current subject and all existing tags unchanged:
ipvlan: keep lower device alive until private destruction
The v2 commit message still contains two incorrect lifetime statements.
First, do not say that the lower-device reference owned by struct
ipvl_port is released whenever an ipvlan device is unregistered. The
port is shared. Its reference is released when the last ipvlan_uninit()
reduces port->count to zero and calls ipvlan_port_destroy(). An
individual ipvlan netdev can remain alive after that ndo_uninit because
RXE still owns a reference to it.
Second, do not say that ipvlan_dev_free() runs after the ipvlan netdev
has been freed or after its reference count reaches zero.
netdev_run_todo() waits until netdev_refcnt_read(dev) is 1, invokes
priv_destructor, and only then performs the final kobject_put(). State
that ipvlan_dev_free() runs after outstanding external references to the
ipvlan netdev have drained and before final private teardown and object
release.
Describe RXE explicitly as the asynchronous owner involved in this
report. RXE queues RDMA device removal on NETDEV_UNREGISTER, so it can
retain the ipvlan netdev after ndo_uninit. A later SMC port query can
still reach ipvlan->phy_dev through ib_query_port(), rxe_query_port(),
ib_get_eth_speed(), and ipvlan_ethtool_get_link_ksettings().
The revised explanation should make this sequence clear:
1. The shared struct ipvl_port owns one reference to the lower net_device.
2. The last ipvlan_uninit() destroys the shared port and drops that
reference.
3. RXE can still retain the individual ipvlan netdev while its queued
unregister work is pending.
4. Without a per-ipvlan reference, phy_dev can point to a lower netdev
that has reached final release.
5. The new per-device hold keeps phy_dev alive until external references
to the ipvlan netdev drain and ipvlan_dev_free() drops the hold before
final object release.
Return the complete replacement patch email, including the unchanged
diff. Do not introduce code changes or unrelated cleanup. Verify the
destructor ordering against netdev_run_todo() before describing it in
the commit message.
On 7/19/2026 12:28 PM, syzbot wrote:
> Commit 40b9d1ab63f5 ("ipvlan: hold lower dev to avoid possible
> use-after-free") added a reference to the lower net_device owned by struct
> ipvl_port. However, this reference is released when the ipvlan device is
> unregistered, which can happen before the ipvlan device is actually freed.
>
> If a stacked network device configuration is created (e.g., a lower
> net_device, an ipvlan device on top, and another device on top of ipvlan),
> the upper device holds a reference to the ipvlan device. When the lower
> net_device is unregistered, the ipvlan device is unregistered as well. This
> triggers unregistration of the upper device, which queues asynchronous work
> to drop its reference to the ipvlan device.
>
> During this process, netdev_run_todo() waits for the refcounts of both the
> lower net_device and the ipvlan device to drop to 1. Since the ipvlan
> device's refcount is elevated by the pending upper device unregistration,
> it is kept alive. However, because the lower net_device's reference held by
> the ipvlan port is released during unregistration, the lower net_device's
> refcount drops to 1, and netdev_run_todo() proceeds to free it. This leaves
> ipvlan->phy_dev as a dangling pointer.
>
> If an operation (like querying port attributes via ethtool) is performed on
> the ipvlan device while it is still alive, it can access the freed lower
> net_device, triggering a use-after-free:
>
> Call Trace:
> <TASK>
> netdev_need_ops_lock include/net/netdev_lock.h:30 [inline]
> netdev_lock_ops include/net/netdev_lock.h:41 [inline]
> __ethtool_get_link_ksettings+0x230/0x250 net/ethtool/ioctl.c:463
> __ethtool_get_link_ksettings+0x11f/0x250 net/ethtool/ioctl.c:464
> ib_get_eth_speed+0x180/0x7f0 drivers/infiniband/core/verbs.c:2052
> rxe_query_port+0x93/0x3d0 drivers/infiniband/sw/rxe/rxe_verbs.c:56
> __ib_query_port drivers/infiniband/core/device.c:2129 [inline]
> ib_query_port+0x16e/0x830 drivers/infiniband/core/device.c:2161
> smc_ib_remember_port_attr net/smc/smc_ib.c:364 [inline]
> smc_ib_port_event_work+0x147/0x920 net/smc/smc_ib.c:388
> </TASK>
>
> Fix this by holding a reference to the lower net_device using a
> netdevice_tracker in struct ipvl_dev. The reference is acquired in
> ipvlan_init() and released in the priv_destructor callback
> (ipvlan_dev_free). Releasing the reference in priv_destructor guarantees
> that the lower net_device is held until the ipvlan device is actually
> freed, after its refcount has dropped to 0. This mirrors the behavior of
> other stacked devices like macvlan and vlan, and safely covers ipvtap
> devices as well.
>
> Fixes: 2ad7bf363841 ("ipvlan: Initial check-in of the IPVLAN driver.")
> Assisted-by: Gemini:gemini-3.5-flash Gemini:gemini-3.1-pro-preview syzbot
> Reported-by: [email protected]
> Closes: https://syzkaller.appspot.com/bug?extid=5fe14f2ff4ccbace9a26
> Link: https://syzkaller.appspot.com/ai_job?id=ecee4f7f-c35a-40b5-85ed-47bebd5cb0d1
> To: "Andrew Lunn" <[email protected]>
> To: "David S. Miller" <[email protected]>
> To: "Eric Dumazet" <[email protected]>
> To: "Jakub Kicinski" <[email protected]>
> To: <[email protected]>
> To: "Paolo Abeni" <[email protected]>
> Cc: "Dmitry Skorodumov" <[email protected]>
> Cc: "Kees Cook" <[email protected]>
> Cc: <[email protected]>
>
> ---
> v2:
> - Updated the patch subject to "ipvlan: keep lower device alive until private destruction"
> - Rewrote the commit message to clarify that commit 40b9d1ab63f5 added a lower-device reference owned by struct ipvl_port
> - Described the lower device generically as a lower net_device
> - Replaced the full KASAN report with only the relevant call chain
> - Corrected the description of priv_destructor
>
> v1:
> https://lore.kernel.org/all/[email protected]/T/
> ---
> diff --git a/drivers/net/ipvlan/ipvlan.h b/drivers/net/ipvlan/ipvlan.h
> index 80f84fc87..13cdad002 100644
> --- a/drivers/net/ipvlan/ipvlan.h
> +++ b/drivers/net/ipvlan/ipvlan.h
> @@ -64,6 +64,7 @@ struct ipvl_dev {
> struct list_head pnode;
> struct ipvl_port *port;
> struct net_device *phy_dev;
> + netdevice_tracker dev_tracker;
> struct list_head addrs;
> struct ipvl_pcpu_stats __percpu *pcpu_stats;
> DECLARE_BITMAP(mac_filters, IPVLAN_MAC_FILTER_SIZE);
> diff --git a/drivers/net/ipvlan/ipvlan_main.c b/drivers/net/ipvlan/ipvlan_main.c
> index ed46439a9..b1435296a 100644
> --- a/drivers/net/ipvlan/ipvlan_main.c
> +++ b/drivers/net/ipvlan/ipvlan_main.c
> @@ -162,6 +162,9 @@ static int ipvlan_init(struct net_device *dev)
> }
> port = ipvlan_port_get_rtnl(phy_dev);
> port->count += 1;
> +
> + netdev_hold(phy_dev, &ipvlan->dev_tracker, GFP_KERNEL);
> +
> return 0;
> }
>
> @@ -673,6 +676,13 @@ 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);
> +
> + netdev_put(ipvlan->phy_dev, &ipvlan->dev_tracker);
> +}
> +
> void ipvlan_link_setup(struct net_device *dev)
> {
> ether_setup(dev);
> @@ -682,6 +692,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;
> }
>
>
> base-commit: 8cdeaa50eae8dad34885515f62559ee83e7e8dda