On 2022/5/6 1:22 pm, D. Wythe wrote:
From: "D. Wythe" <alibuda@xxxxxxxxxxxxxxxxx>
The following scenarios exist:
lnk->refcnt=1;
smcr_link_put(lnk);
lnk->refcnt=0;
smcr_link_hold(lnk);
__smcr_link_clear(lnk);
do_xxx(lnk);
This patch try using refcount_inc_not_zero() instead refcount_inc()
to prevent this race condition. Therefore, we need to always check its
return value, and respond with an error when it fails.
Fixes: 20c9398d3309 ("net/smc: Resolve the race between SMC-R link access and clear")
Signed-off-by: D. Wythe <alibuda@xxxxxxxxxxxxxxxxx>
---
Thanks for your analysis.
1) Is the patch more appropriate to 'net' ?
2) The refcnt of smc link will be
- initilized to 1 in smcr_link_init();
- increased when connections assigned to the link;
eg. smc_conn_create() or smc_switch_link_and_count();
- decreased when connections removed from the link or link is cleared,
eg. smc_conn_free(), smc_switch_link_and_count(), smcr_link_clear().
I see the theoretical race between smcr_link_hold() and smcr_link_put(). Have you encountered this
issue in actual test, such as triggering WARN of refcount_inc()? Because IMHO the race window is small
(link state will turned to SMC_LNK_UNUSED after smcr_link_put() and connections will not be assigned to it).
3) Does the refcount of lgr (smc_lgr_hold(), smc_lgr_put()) has the similar problem?
