Use mount_setattr() in bind_mount()#756
Conversation
| .attr_set = MOUNT_ATTR_NOSUID | (devices ? 0 : MOUNT_ATTR_NODEV) | | ||
| (readonly ? MOUNT_ATTR_RDONLY : 0), | ||
| }; | ||
| if (mount_setattr(dest_fd, "", |
There was a problem hiding this comment.
musl does not have a library function for mount_setattr yet. Also older (before 2024?) glibc versions. We should ifdef-provide our own definition.
xxyzz
force-pushed
the
mount_setattr
branch
2 times, most recently
from
69fdb9e to
f65b3ec
Compare
Were they related somehow? |
No, these actions are just too outdated so I updated them. |
|
What was wrong with The code changes have some AI smell to me. |
|
I feel |
It needs a review from.someone who is actually responsible for the codebase. |
Please move unrelated changes to a separate commit, or even to a separate PR, to have a more focused review, and in general avoid unnecessary space changes. Also the commit message of the main change should mention the rationale from #754 and also have lines like: Ideally with your real name, but I don't think this is a strict requirement 😄 |
| const char *src); | ||
|
|
||
| #ifndef __NR_mount_setattr | ||
| #define __NR_mount_setattr 442 |
There was a problem hiding this comment.
Please note that syscall numbers in linux are platform specific, so this is not general enough. E.g. on alpha __NR_mount_setattr is 552, see https://github.com/torvalds/linux/blob/6f3ed7fec72fc8979b2a8c7219c0a9fcfc8d07b5/arch/alpha/kernel/syscalls/syscall.tbl#L484
What about having a simpler approach like done for pivot_root? So that we require that __NR_mount_setattr is defined but we support the cases where the system function mount_setattr is not.
There was a problem hiding this comment.
Code updated. I think I've read that function before but somehow forgot it...
Fixes: containers#754 Signed-off-by: xxyzz <gitpull@protonmail.com>
4 participants
Fix #754, all tests passed. I also update some GitHub Actions.