core/logging: Fix race in writing to vrb_ep_ops

[dsciebu]

'vrb_ep_ops' is a static object which field 'ops_open' gets overriden in 'vrb_open_ep'. If the operation is performed by multiple threads at once, which is totally possible, we end up with a data race. Switching it to thread local fixes it.

Signed-off-by: Dariusz Sciebura [email protected]

[dsciebu]

There is a bunch of static variables spread all over the place. IMHO they all should be marked as _Thread_local, as, even if they are not overridden somewhere now, they can be in future leading to another race. If you agree, just let know - I can modify my PR. Otherwise, let's stick to this ver.

[piotrchmiel]

Isn't it duplicate of #10571 and #10569 ?

[dsciebu]

Isn't it duplicate of #10571 and #10569 ?

It is in a sense that it generally solves the same issue. However what I am promoting here is a general rule of marking static globals with _Thread_local as probably there are still used concurrently in several places and will probably be used that way in the future. IMO the best solution would be to rethink the architecture and refactor the existing code, but as a quick fix adding thread locality to variables like the one mentioned may help fix a lot of UBs for pretty much 'free'.

[dsciebu]

I accidentally found another spot with an identical bug, this time in xnet_ep.c. The line:
(*ep_fid)->fid.ops->ops_open = xnet_ep_ops_open;
overwrites the global assigned to (*ep_fid)->fid.ops assigned in the following line:
(*ep_fid)->fid.ops = &xnet_ep_fi_ops;
I will push a fix for it in a separate PR, but please treat it as an argument for my idea of marking the globals with thread_local.
Edit: The fix mentioned is #10590.

[dsciebu]

I accidentally found another spot with an identical bug, this time in xnet_ep.c. The line: (*ep_fid)->fid.ops->ops_open = xnet_ep_ops_open; overwrites the global assigned to (*ep_fid)->fid.ops assigned in the following line: (*ep_fid)->fid.ops = &xnet_ep_fi_ops; I will push a fix for it in a separate PR, but please treat it as an argument for my idea of marking the globals with thread_local. Edit: The fix mentioned is #10590.

@shefty - counting on your voice!