OS_chmod is requiring read access

[zanzaben]

Describe the bug
If you don't have read access to a file then OS_chmod will not be able to change it. This is most likely caused by chmod opening the file as a way to avoid filename race potential, see the code snip.

To Reproduce

1. Set the file access to write only.
2. Try to change the access using OS_chmod
3. will get an OS_ERROR

Expected behavior
You should be able to change the permissions of a file without read access.

Code snips

osal/src/os/portable/os-impl-posix-files.c

Lines 201 to 206 in 1a82657

	/* Open file to avoid filename race potential */
	fd = open(local_path, O_RDONLY);
	if (fd < 0)
	{
	return OS_ERROR;
	}

System observed on:
Ubuntu 20.04

Reporter Info
Alex Campbell GSFC

[jphickey]

I think this came up in a previous CCB at one point, when it was changed to use fstat / fchmod to do this on a file descriptor rather than file name....

Linux offers a solution via the O_PATH flag - which opens the fd in a special mode where normal reading/writing is not possible but stat/chmod are possible. But AFAIK this flag is a non-posix extension. Perhaps it warrants some sort of #ifdef logic to determine of the OS provides this extension and use it if it does.

[astrogeco]

CCB 2020-11-12

We discussed this a while ago when we talked about
Depends on the file system implementation
No easy way to fix this in POSIX
DOS-FS does not use this so we don't know what happens
Add an ifdef and use O_PATH if it exists

[jphickey]

Regarding the actual test cases -- my suggestion is to still run through the test, but make the one where it tries to re-enable read access to be an "MIR" type if it fails.

That is -- if it succeeds and returns OS_SUCCESS then the test passes no problem - but if it fails with non-OS_SUCCESS, then report it as MIR - so it will not stop the test.

This way if the system does support O_PATH we can actually exercise its intended purpose and confirm its working as expected, but on a system that doesn't have O_PATH the test case is just skipped. (MIR doesn't fail the test).

[zanzaben]

If it is not able to be opened in read only can we then try to open it in write only? All that matters is that the file is open to prevent race potential.

[skliper]

If it is not able to be opened in read only can we then try to open it in write only? All that matters is that the file is open to prevent race potential.

After a chance to think about this, seems preferable to the system-dependent O_PATH approach. Could you submit a PR? Since the only options are OS_READ_ONLY, OS_WRITE_ONLY, and OS_READ_WRITE trying both one should succeed.

[jphickey]

If it is not able to be opened in read only can we then try to open it in write only? All that matters is that the file is open to prevent race potential.

After a chance to think about this, seems preferable to the system-dependent O_PATH approach. Could you submit a PR? Since the only options are OS_READ_ONLY, OS_WRITE_ONLY, and OS_READ_WRITE trying both one should succeed.

Note that it is possible to have neither read nor write permission on a file, even for the file's owner (i.e. mode 0000). The O_PATH still works even for this, but opening for write will not.

[skliper]

...possible to have neither

Understood (which is why I was hesitant at first w/ the suggestion), but it seems like the OSAL only supports setting it as the listed three? If OSAL doesn't support setting it as neither, does OS_chmod really need to support it conditionally based on if O_PATH is available? Seems like added complexity that doesn't universally solve the problem, so I'm leaning towards consistency...