restart_grace() still use hardcoded init_net.
It can cause to "list_add double add" in following scenario:
1) nfsd and lockd was started in several net namespaces
2) nfsd in init_net was stopped (lockd was not stopped because
it have users from another net namespaces)
3) lockd got signal, called restart_grace() -> set_grace_period()
and enabled lock_manager in hardcoded init_net.
4) nfsd in init_net is started again,
its lockd_up() calls set_grace_period() and tries to add lock_manager
into init_net 2nd time.
I do not understand how to fix this problem correctly.
We can:
1) remove per-netns calls in restart_grace().
However it was worked for init-net-only case for ages,
and I afraid users of this functionality will object.
2) we can somehow provide reference to net namespace of process submitted handled signal
and handle pointed net namespace only.
However this time it isn't clear for me how to do it.
3) we can call per-netns operations for all existing net namespaces
However I'm not sure is it expected behaviour.
Also I afraid it can be racy with creation/destroy of net namespaces.
4) we can make lockd kernel thraad per net_ns, like nfsd.
However this solution looks too heavy for this problem.
Could you please advise some other solution?
Thank you,
Vasily Averin
On Mon, 2017-10-30 at 18:20 +0300, Vasily Averin wrote:
> restart_grace() still use hardcoded init_net.
> It can cause to "list_add double add" in following scenario:
>
> 1) nfsd and lockd was started in several net namespaces
> 2) nfsd in init_net was stopped (lockd was not stopped because
> it have users from another net namespaces)
> 3) lockd got signal, called restart_grace() -> set_grace_period()
> and enabled lock_manager in hardcoded init_net.
> 4) nfsd in init_net is started again,
> its lockd_up() calls set_grace_period() and tries to add lock_manager
> into init_net 2nd time.
>
> I do not understand how to fix this problem correctly.
>
> We can:
> 1) remove per-netns calls in restart_grace().
> However it was worked for init-net-only case for ages,
> and I afraid users of this functionality will object.
>
> 2) we can somehow provide reference to net namespace of process submitted handled signal
> and handle pointed net namespace only.
> However this time it isn't clear for me how to do it.
>
> 3) we can call per-netns operations for all existing net namespaces
> However I'm not sure is it expected behaviour.
> Also I afraid it can be racy with creation/destroy of net namespaces.
>
> 4) we can make lockd kernel thraad per net_ns, like nfsd.
> However this solution looks too heavy for this problem.
>
> Could you please advise some other solution?
>
> Thank you,
> Vasily Averin
The problem seems to be that list_add in locks_start_grace. It seems
like we only want to do that list_add if list_empty(&lm->list) is true.
We already do a list_del_init when removing from grace_list, and it
looks like both users of this API initialize their lists properly.
Is that enough of a fix or am I missing the bigger picture?
--
Jeff Layton <[email protected]>