On Wed, Feb 09, 2022 at 10:10:53PM +0800, Wen Gu wrote:
> The callback functions of clcsock will be saved and replaced during
> the fallback. But if the fallback happens more than once, then the
> copies of these callback functions will be overwritten incorrectly,
> resulting in a loop call issue:
>
> clcsk->sk_error_report
> |- smc_fback_error_report() <------------------------------|
> |- smc_fback_forward_wakeup() | (loop)
> |- clcsock_callback() (incorrectly overwritten) |
> |- smc->clcsk_error_report() ------------------|
>
> So this patch fixes the issue by saving these function pointers only
> once in the fallback and avoiding overwriting.
>
> Reported-by: syzbot+4de3c0e8a263e1e499bc@xxxxxxxxxxxxxxxxxxxxxxxxx
> Fixes: 341adeec9ada ("net/smc: Forward wakeup to smc socket waitqueue after fallback")
> Link: https://lore.kernel.org/r/0000000000006d045e05d78776f6@xxxxxxxxxx
> Signed-off-by: Wen Gu <guwen@xxxxxxxxxxxxxxxxx>
> ---
> net/smc/af_smc.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
> index 8c89d0b..306d9e8c 100644
> --- a/net/smc/af_smc.c
> +++ b/net/smc/af_smc.c
> @@ -667,14 +667,17 @@ static void smc_fback_error_report(struct sock *clcsk)
> static int smc_switch_to_fallback(struct smc_sock *smc, int reason_code)
> {
> struct sock *clcsk;
> + int rc = 0;
>
> mutex_lock(&smc->clcsock_release_lock);
> if (!smc->clcsock) {
> - mutex_unlock(&smc->clcsock_release_lock);
> - return -EBADF;
> + rc = -EBADF;
> + goto out;
> }
> clcsk = smc->clcsock->sk;
>
> + if (smc->use_fallback)
> + goto out;
> smc->use_fallback = true;
I am wondering that there is a potential racing. If ->use_fallback is
setted to true, but the rest of replacing process is on the way, others
who tested and passed ->use_fallback, they would get old value before
replacing.
Thanks,
Tony Lu
