[PATCH net v2] ipvlan: update port flags only after ipvlan_set_port_mode() succeeds

[PATCH net v2] ipvlan: update port flags only after ipvlan_set_port_mode() succeeds

[Wenbo Sun]

In ipvlan_link_new(), port->flags is updated before calling
ipvlan_set_port_mode(). If ipvlan_set_port_mode() fails (for example,
when switching to L3S mode without CONFIG_IPVLAN_L3S), error unwinding
calls unregister_netdevice(dev) via unlink_netdev.

While unregister_netdevice(dev) destroys the port when creating the first
ipvlan device on a lower interface, struct ipvl_port is shared across all
ipvlan interfaces on the same lower device (tracked via port->count). If
another ipvlan interface already exists, unregister_netdevice(dev) only
drops the extra reference and leaves the shared port alive with the
updated port->flags, unintentionally altering the flags of existing
ipvlan interfaces.

Defer updating port->flags until after ipvlan_set_port_mode() succeeds,
after which ipvlan_link_new() has no remaining failure paths. This avoids
modifying port->flags on failure without needing to save and restore it,
matching ipvlan_nl_changelink().

This issue was identified with the assistance of an LLM-based code
review tool, confirmed in a User-Mode Linux (UML) environment by creating
a second ipvlan interface that fails in ipvlan_set_port_mode(), and
verified in UML that the existing interface's port flags remain
unchanged upon failure.

Fixes: a190d04db937 ("ipvlan: introduce 'private' attribute for all existing modes.")
Signed-off-by: Wenbo Sun <sunwenbo@cmss.chinamobile.com>
---
v2:
  - Clarify in the commit message that when creating a second or
    subsequent ipvlan interface on an already-active port,
    unregister_netdevice() only drops the extra reference rather than
    destroying the shared port, and explain why deferring the update
    avoids needing to save and restore port->flags (Hangbin Liu).
  - Soften and clarify the wording in the commit message.
v1: https://lore.kernel.org/netdev/20261111064655.233265-1-sunwenbo@cmss.chinamobile.com/

 drivers/net/ipvlan/ipvlan_main.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ipvlan/ipvlan_main.c b/drivers/net/ipvlan/ipvlan_main.c
index f29864db662a..5732b167c5b3 100644
--- a/drivers/net/ipvlan/ipvlan_main.c
+++ b/drivers/net/ipvlan/ipvlan_main.c
@@ -678,12 +678,6 @@ int ipvlan_link_new(struct net_device *dev, struct rtnl_newlink_params *params,
 	if (err)
 		goto remove_ida;
 
-	/* Flags are per port and latest update overrides. User has
-	 * to be consistent in setting it just like the mode attribute.
-	 */
-	if (data && data[IFLA_IPVLAN_FLAGS])
-		port->flags = nla_get_u16(data[IFLA_IPVLAN_FLAGS]);
-
 	if (data && data[IFLA_IPVLAN_MODE])
 		mode = nla_get_u16(data[IFLA_IPVLAN_MODE]);
 
@@ -691,6 +685,12 @@ int ipvlan_link_new(struct net_device *dev, struct rtnl_newlink_params *params,
 	if (err)
 		goto unlink_netdev;
 
+	/* Flags are per port and latest update overrides. User has
+	 * to be consistent in setting it just like the mode attribute.
+	 */
+	if (data && data[IFLA_IPVLAN_FLAGS])
+		port->flags = nla_get_u16(data[IFLA_IPVLAN_FLAGS]);
+
 	mutex_lock(&port->pnodes_lock);
 	list_add_tail_rcu(&ipvlan->pnode, &port->ipvlans);
 	mutex_unlock(&port->pnodes_lock);
-- 
2.33.0

Re: [PATCH net v2] ipvlan: update port flags only after ipvlan_set_port_mode() succeeds

[Hangbin Liu]

On Thu, Nov 12, 2026 at 02:48:21PM +0800, Wenbo Sun wrote:
> In ipvlan_link_new(), port->flags is updated before calling
> ipvlan_set_port_mode(). If ipvlan_set_port_mode() fails (for example,
> when switching to L3S mode without CONFIG_IPVLAN_L3S), error unwinding
> calls unregister_netdevice(dev) via unlink_netdev.
> 
> While unregister_netdevice(dev) destroys the port when creating the first
> ipvlan device on a lower interface, struct ipvl_port is shared across all
> ipvlan interfaces on the same lower device (tracked via port->count). If
> another ipvlan interface already exists, unregister_netdevice(dev) only
> drops the extra reference and leaves the shared port alive with the
> updated port->flags, unintentionally altering the flags of existing
> ipvlan interfaces.
> 
> Defer updating port->flags until after ipvlan_set_port_mode() succeeds,
> after which ipvlan_link_new() has no remaining failure paths. This avoids
> modifying port->flags on failure without needing to save and restore it,
> matching ipvlan_nl_changelink().
> 
> This issue was identified with the assistance of an LLM-based code
> review tool, confirmed in a User-Mode Linux (UML) environment by creating
> a second ipvlan interface that fails in ipvlan_set_port_mode(), and
> verified in UML that the existing interface's port flags remain
> unchanged upon failure.
> 
> Fixes: a190d04db937 ("ipvlan: introduce 'private' attribute for all existing modes.")
> Signed-off-by: Wenbo Sun <sunwenbo@cmss.chinamobile.com>
> ---
> v2:
>   - Clarify in the commit message that when creating a second or
>     subsequent ipvlan interface on an already-active port,
>     unregister_netdevice() only drops the extra reference rather than
>     destroying the shared port, and explain why deferring the update
>     avoids needing to save and restore port->flags (Hangbin Liu).
>   - Soften and clarify the wording in the commit message.
> v1: https://lore.kernel.org/netdev/20261111064655.233265-1-sunwenbo@cmss.chinamobile.com/
> 
>  drivers/net/ipvlan/ipvlan_main.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/ipvlan/ipvlan_main.c b/drivers/net/ipvlan/ipvlan_main.c
> index f29864db662a..5732b167c5b3 100644
> --- a/drivers/net/ipvlan/ipvlan_main.c
> +++ b/drivers/net/ipvlan/ipvlan_main.c
> @@ -678,12 +678,6 @@ int ipvlan_link_new(struct net_device *dev, struct rtnl_newlink_params *params,
>  	if (err)
>  		goto remove_ida;
>  
> -	/* Flags are per port and latest update overrides. User has
> -	 * to be consistent in setting it just like the mode attribute.
> -	 */
> -	if (data && data[IFLA_IPVLAN_FLAGS])
> -		port->flags = nla_get_u16(data[IFLA_IPVLAN_FLAGS]);
> -
>  	if (data && data[IFLA_IPVLAN_MODE])
>  		mode = nla_get_u16(data[IFLA_IPVLAN_MODE]);
>  
> @@ -691,6 +685,12 @@ int ipvlan_link_new(struct net_device *dev, struct rtnl_newlink_params *params,
>  	if (err)
>  		goto unlink_netdev;
>  
> +	/* Flags are per port and latest update overrides. User has
> +	 * to be consistent in setting it just like the mode attribute.
> +	 */
> +	if (data && data[IFLA_IPVLAN_FLAGS])
> +		port->flags = nla_get_u16(data[IFLA_IPVLAN_FLAGS]);
> +
>  	mutex_lock(&port->pnodes_lock);
>  	list_add_tail_rcu(&ipvlan->pnode, &port->ipvlans);
>  	mutex_unlock(&port->pnodes_lock);
> -- 
> 2.33.0
> 

Reviewed-by: Hangbin Liu <liuhangbin@kylinos.cn>