netdev - Re: [PATCH bpf-next v4 2/3] net/smc: bpf: Introduce generic hook for handshake flow

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20251103091834.GA10875@j66a10360.sqa.eu95>
Date: Mon, 3 Nov 2025 17:18:34 +0800
From: "D. Wythe" <alibuda@...ux.alibaba.com    >
To: bot+bpf-ci@...nel.org
Cc: alibuda@...ux.alibaba.com, ast@...nel.org, daniel@...earbox.net,
	andrii@...nel.org, martin.lau@...ux.dev, pabeni@...hat.com,
	song@...nel.org, sdf@...gle.com, haoluo@...gle.com, yhs@...com,
	edumazet@...gle.com, john.fastabend@...il.com, kpsingh@...nel.org,
	jolsa@...nel.org, mjambigi@...ux.ibm.com, wenjia@...ux.ibm.com,
	wintera@...ux.ibm.com, dust.li@...ux.alibaba.com,
	tonylu@...ux.alibaba.com, guwen@...ux.alibaba.com,
	bpf@...r.kernel.org, davem@...emloft.net, kuba@...nel.org,
	netdev@...r.kernel.org, sidraya@...ux.ibm.com, jaka@...ux.ibm.com,
	martin.lau@...nel.org, eddyz87@...il.com, yonghong.song@...ux.dev,
	clm@...a.com, ihor.solodrai@...ux.dev
Subject: Re: [PATCH bpf-next v4 2/3] net/smc: bpf: Introduce generic hook for
 handshake flow

On Mon, Nov 03, 2025 at 07:55:35AM +0000, bot+bpf-ci@...nel.org wrote:
> 
> ```
> commit b661eee197ec1c841de9195f171863daef37de34
> Author: D. Wythe <alibuda@...ux.alibaba.com>
> 
> net/smc: bpf: Introduce generic hook for handshake flow
> 
> This patch introduces a BPF struct_ops mechanism that allows eBPF
> programs to control SMC protocol handshake decisions at runtime based on
> connection properties like IP addresses and ports.
> 
> > diff --git a/net/smc/smc_sysctl.c b/net/smc/smc_sysctl.c
> > index 2fab6456f..918516734 100644
> > --- a/net/smc/smc_sysctl.c
> > +++ b/net/smc/smc_sysctl.c
> 
> [ ... ]
> 
> > +#if IS_ENABLED(CONFIG_SMC_HS_CTRL_BPF)
> > +static int smc_net_replace_smc_hs_ctrl(struct net *net, const char *name)
> > +{
> > +	struct smc_hs_ctrl *ctrl = NULL;
> > +
> > +	rcu_read_lock();
> > +	/* null or empty name ask to clear current ctrl */
> > +	if (name && name[0]) {
> > +		ctrl = smc_hs_ctrl_find_by_name(name);
> > +		if (!ctrl) {
> > +			rcu_read_unlock();
> > +			return -EINVAL;
> > +		}
> > +		/* no change, just return */
> > +		if (ctrl == rcu_dereference(net->smc.hs_ctrl)) {
> > +			rcu_read_unlock();
> > +			return 0;
> > +		}
> > +		if (!bpf_try_module_get(ctrl, ctrl->owner)) {
> > +			rcu_read_unlock();
> > +			return -EBUSY;
> > +		}
> > +	}
> > +	/* xhcg old ctrl with the new one atomically */
> > +	ctrl = unrcu_pointer(xchg(&net->smc.hs_ctrl, RCU_INITIALIZER(ctrl)));
> > +	/* release old ctrl */
> > +	if (ctrl)
> > +		bpf_module_put(ctrl, ctrl->owner);
> > +
> > +	rcu_read_unlock();
> > +	return 0;
> > +}
> 
> Can concurrent sysctl writes race in smc_net_replace_smc_hs_ctrl? If
> two threads both write the same ctrl name to the hs_ctrl sysctl:
> 
> Thread A: finds ctrl, bpf_try_module_get(ctrl) succeeds (refcount++)
> Thread B: finds ctrl, bpf_try_module_get(ctrl) succeeds (refcount++)
> Thread A: xchg(&net->smc.hs_ctrl, ctrl) returns old_ctrl
> Thread B: xchg(&net->smc.hs_ctrl, ctrl) returns ctrl (from Thread A)
> Thread A: bpf_module_put(old_ctrl)
> Thread B: bpf_module_put(ctrl)  <- drops ref on ctrl still in use
> 
> This leaves ctrl with one less reference than needed while it's assigned
> to net->smc.hs_ctrl, and old_ctrl leaks a reference. The xchg is atomic
> but the overall get-xchg-put sequence is not. Would a mutex around the
> entire operation resolve this?
>

Hi claude,

I don't think this is a problem:

1. The first thread sets hs_ctrl = ctrl and keeps its reference.
2. The second thread sees hs_ctrl is already ctrl, so its xchg returns ctrl,
   and it drops its own redundant reference.
3. The final state is hs_ctrl = ctrl with refcount=1, which is correct.

In all cases:

- The reference held by hs_ctrl is the one acquired by the last writer.
- Other writers only drop their own temporary references.

D. Wythe