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

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

[Wenbo Sun]

In ipvlan_link_new(), the shared port->flags is updated before
calling ipvlan_set_port_mode(). If ipvlan_set_port_mode() fails (e.g.
switching to L3S mode without CONFIG_IPVLAN_L3S), the error unwinds via
unlink_netdev, but the shared port->flags has already been modified and
is never restored.

As a result, subsequent operations on existing ipvlan interfaces on the
same lower device operate with corrupted port flags (e.g., VEPA or
PRIVATE flags inadvertently set or cleared).

Fix this by deferring the update of port->flags until after
ipvlan_set_port_mode() returns successfully, matching the behaviour of
ipvlan_nl_changelink().

This issue was identified with the assistance of an LLM-based code
review tool, confirmed by reproducing the state corruption in a
User-Mode Linux (UML) environment when ipvlan_set_port_mode() fails,
and verified in UML that the shared 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>
---
 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] ipvlan: update port flags only after ipvlan_set_port_mode() succeeds

[Hangbin Liu]

Hi Wenbo,
On Wed, Nov 11, 2026 at 02:46:55PM +0800, Wenbo Sun wrote:
> In ipvlan_link_new(), the shared port->flags is updated before
> calling ipvlan_set_port_mode(). If ipvlan_set_port_mode() fails (e.g.
> switching to L3S mode without CONFIG_IPVLAN_L3S), the error unwinds via
> unlink_netdev, but the shared port->flags has already been modified and
> is never restored.

If ipvlan_set_port_mode() failed, the unregister_netdevice(dev) will be
called. Does port->flags still need to restore?

Thanks
Hangbin
> 
> As a result, subsequent operations on existing ipvlan interfaces on the
> same lower device operate with corrupted port flags (e.g., VEPA or
> PRIVATE flags inadvertently set or cleared).
> 
> Fix this by deferring the update of port->flags until after
> ipvlan_set_port_mode() returns successfully, matching the behaviour of
> ipvlan_nl_changelink().
> 
> This issue was identified with the assistance of an LLM-based code
> review tool, confirmed by reproducing the state corruption in a
> User-Mode Linux (UML) environment when ipvlan_set_port_mode() fails,
> and verified in UML that the shared 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>
> ---
>  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] ipvlan: update port flags only after ipvlan_set_port_mode() succeeds

[Wenbo Sun]

Hi Hangbin,

On Wed, Nov 11, 2026 at 09:02:17PM +0800, Hangbin Liu wrote:
> If ipvlan_set_port_mode() failed, the unregister_netdevice(dev) will be
> called. Does port->flags still need to restore?

Thank you very much for taking the time to review this patch and for
pointing out the question about the commit description.

You are completely right that when creating the first ipvlan device on a
lower device, unregister_netdevice(dev) drops the last reference to the
port in ipvlan_uninit() -> ipvlan_port_put(), destroying the port so no
flag restoration is needed.

The issue happens when one or more ipvlan interfaces already exist on the
same lower device:

1. Because struct ipvl_port is shared across all ipvlan interfaces on a
   lower device, ipvlan_init() takes an additional reference via
   refcount_inc(&port->count) when creating a new ipvlan interface.
2. If ipvlan_link_new() writes port->flags before calling
   ipvlan_set_port_mode(), and ipvlan_set_port_mode() fails (for
   instance, requesting L3S mode when CONFIG_IPVLAN_L3S is disabled),
   unregister_netdevice(dev) only decrements port->count back down to at
   least 1. Since ipvlan_port_destroy() is not called, the shared port
   remains active for the existing ipvlan interfaces while still carrying
   the newly written port->flags.

Rather than saving and restoring port->flags on the error path, moving
the port->flags assignment after ipvlan_set_port_mode() succeeds avoids
modifying it on failure altogether, as there are no remaining failure
paths afterward in ipvlan_link_new() (matching ipvlan_nl_changelink()).

I see that the v1 commit message phrased this awkwardly ("never restored")
and did not clearly distinguish the shared-port case with existing
interfaces. If you agree with this explanation and approach, I will be
happy to send a v2 with a clearer and more precise commit message once the
24-hour resend window passes.

Thanks again for the review!

Best regards,
Wenbo

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

[Hangbin Liu]

On Thu, Nov 12, 2026 at 11:51:52AM +0800, Wenbo Sun wrote:
> Hi Hangbin,
> 
> On Wed, Nov 11, 2026 at 09:02:17PM +0800, Hangbin Liu wrote:
> > If ipvlan_set_port_mode() failed, the unregister_netdevice(dev) will be
> > called. Does port->flags still need to restore?
> 
> Thank you very much for taking the time to review this patch and for
> pointing out the question about the commit description.
> 
> You are completely right that when creating the first ipvlan device on a
> lower device, unregister_netdevice(dev) drops the last reference to the
> port in ipvlan_uninit() -> ipvlan_port_put(), destroying the port so no
> flag restoration is needed.
> 
> The issue happens when one or more ipvlan interfaces already exist on the
> same lower device:
> 
> 1. Because struct ipvl_port is shared across all ipvlan interfaces on a
>    lower device, ipvlan_init() takes an additional reference via
>    refcount_inc(&port->count) when creating a new ipvlan interface.
> 2. If ipvlan_link_new() writes port->flags before calling
>    ipvlan_set_port_mode(), and ipvlan_set_port_mode() fails (for
>    instance, requesting L3S mode when CONFIG_IPVLAN_L3S is disabled),
>    unregister_netdevice(dev) only decrements port->count back down to at
>    least 1. Since ipvlan_port_destroy() is not called, the shared port
>    remains active for the existing ipvlan interfaces while still carrying
>    the newly written port->flags.
> 
> Rather than saving and restoring port->flags on the error path, moving
> the port->flags assignment after ipvlan_set_port_mode() succeeds avoids
> modifying it on failure altogether, as there are no remaining failure
> paths afterward in ipvlan_link_new() (matching ipvlan_nl_changelink()).

Thanks for the explanation. The change makes sense to me now.

Regards
Hangbin