Re: [PATCH net v2 7/7] ipv6: reset position for force_forwarding sysctl restart

[Ido Schimmel <idosch@nvidia.com>]

On Sat, Jun 20, 2026 at 06:18:50PM +0200, Fernando Fernandez Mancera wrote:
> When handling proxy_ndp, if rtnl_net_trylock() fails, the operation is

s/proxy_ndp/force_forwarding/

> retried but the position pointer was already advanced meaning that the
> restarted sysctl will read from an incorrect offset.
> 
> Fix this by restoring the original position pointer before restarting
> the syscall.
> 
> In addition, remove the redundant position pointer restoration at the
> end of the function.
> 
> Fixes: f24987ef6959 ("ipv6: add `force_forwarding` sysctl to enable per-interface forwarding")
> Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
> ---
>  net/ipv6/addrconf.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index cbe681de3818..8c0741e9dfcc 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -6825,8 +6825,10 @@ static int addrconf_sysctl_force_forwarding(const struct ctl_table *ctl, int wri
>  	ret = proc_douintvec_minmax(&tmp_ctl, write, buffer, lenp, ppos);
>  
>  	if (write && old_val != new_val) {
> -		if (!rtnl_net_trylock(net))
> +		if (!rtnl_net_trylock(net)) {
> +			*ppos = pos;
>  			return restart_syscall();
> +		}

Are you sure that this is needed?

AFAICT, the position pointer is only advanced if the return value is
positive. From new_sync_write():

kiocb.ki_pos = (ppos ? *ppos : 0);
[...]
ret = filp->f_op->write_iter(&kiocb, &iter);
[...]
if (ret > 0 && ppos)
        *ppos = kiocb.ki_pos;

And restart_syscall() returns '-ERESTARTNOINTR'.

>  
>  		WRITE_ONCE(*valp, new_val);
>  
> @@ -6851,8 +6853,6 @@ static int addrconf_sysctl_force_forwarding(const struct ctl_table *ctl, int wri
>  		rtnl_net_unlock(net);
>  	}
>  
> -	if (ret)
> -		*ppos = pos;
>  	return ret;
>  }
>  
> -- 
> 2.54.0
>