Possible race condition in umask?

[chengguizi]

Problem Description

We are currently experiencing seeminly race condition issues, where sometime exactly 1 of all the /dev/shm/ecal_ files has the permission set to 644 instead of 666.

It typically happens when we run with sudo which default to have umask 0002.

We are using 6.0.1 eCAL. I have inspect the source code, for both 6.0.1 and latest main, I notice:

ecal/ecal/core/src/io/shm/linux/ecal_memfile_os.cpp

Line 46 in 966435b

int previous_umask = umask(000); // set umask to nothing, so we can create files with all possible permission bits

umask is being called in different parts of eCAL code. and there is actually blocking calls between

const int previous_umask = ::umask(000);

and

::umask(previous_umask);

Wouldn't this create race condition where, for example we have two thread, thread1 and thread2:

thread1: ::umask(000);
thread2: ::umask(000);
thread1: ::umask(previous_umask);

thread2 ::shm_open(name.c_str(), O_RDWR | O_CREAT

thread2: ::umask(previous_umask);

the thread2 shm_open will have the wrong umask, not 000, but actually the process umask.

How to reproduce

Can't be sure.

But running pubisher in sudo privilege helps reproduction. And must be multiple topics streaming together.

How did you get eCAL?

I don't know

Environment

eCAL: 6.0.1

eCAL System Information

Ubuntu 24.04

[KerstinKeller]

Hi @chengguizi,
thank you for reporting.
First of all, if we provided you a fix based on 6.0.1 (or better 6.0.2 / 6.1.1), would you be able to test that out?

And it's seems that you're right, in case of creating SHM concurrently, we might run into the problem which you described.
There are a few ideas how we can go about it in general:

Serializing SHM file creation for different threads (bugfix approach)

Bugfix approach for current support branches. We have one global mutex for all SHM memfile creations, to ensure umasking is done and finished safely
pro: quick fix
con: doesn't handle user side umasking activities, only serializes eCAL activities

User responsibility of handling umasking

eCAL doesn't handle setting masks, but the user is.
pro: low implementation effort
con: it's on the user to make sure ecal processes can communicate effectively

Setting file permissions after create

We create without umask, and change file permissions later
pro: low implementation effort
con: error handling necessary, in case other file is trying to be opened, while file masks are still at creation level.

Introduce ecal group

We create a folder with ACL permission set on an ecal user group which users need to be added to. All shm objects can be created within this folder
pro: clean solution
con: breaks binary ecal compatibility with older eCAL versions, since files will live in another location

I think the serialization of the file creation can be implemented easily and straightforward as a bug fix, for other solutions we might want to consider carefully the desired behavior

[chengguizi]

Hi @KerstinKeller , we are reproducting in deployed environment and it is using eCAL 6.0.1 for now. It would be great we first verify the fix there.

I am keen to have have the fix either:

1. global mutex
2. change permission after creation (first strict 600, then relaxed 666)

in either case, i think ecal should give explicit warnings that if publisher detects any of its managed files is having non-desired permission. So at least user can choose what to do.

[chengguizi]

@KerstinKeller Keen to know if a fix is to be expected soon? Keen to test out and bounce ideas around

[KerstinKeller]

I have created #2613. However, I am not sure if it can be directly cherry picked to 6.0.1.
Anyways, also for your production environment, we always suggest that you update to the latest version within the same minor version, because we do apply bugfixes only.

[KerstinKeller]

For now, we'll leave it with #2613. This will cover umask creation from eCAL side.
For the other possibilities (e.g. umasking is the users responsibility, I will keep this issue open).

[chengguizi]

Sounds good! It is a pity that it seems to be a design limitation of umask in Linux/POSIX.

We currently bypass the issue by avoiding running the process with non-666 permissions, just to make sure permission not blocking any access.

A suggestion. I think there should be explicit warning from subscriber eCAL, if the shm open return failed due to permission, it should give proper eCAL warning. So user can understand what happens.

[KerstinKeller]

Yes, unfortunately our error handling in this regard is not very good 👎 I created also PR for 6.0 and 6.1 releases, so you can build from support branches or wait for a new patch releases (although that might take a little time).
In general, there are many places where we would need better error handling in eCAL.....