on 2022/4/13 16:07, Christian Brauner wrote:
> On Tue, Apr 12, 2022 at 07:33:44PM +0800, Yang Xu wrote:
>> Since we can create temp file by using O_TMPFILE flag and filesystem driver also
>> has this api, we should also check this operation whether strip S_ISGID.
>>
>> Reviewed-by: Christian Brauner (Microsoft)<brauner@xxxxxxxxxx>
>> Signed-off-by: Yang Xu<xuyang2018.jy@xxxxxxxxxxx>
>> ---
>> src/idmapped-mounts/idmapped-mounts.c | 148 ++++++++++++++++++++++++++
>> 1 file changed, 148 insertions(+)
>>
>> diff --git a/src/idmapped-mounts/idmapped-mounts.c b/src/idmapped-mounts/idmapped-mounts.c
>> index 617f56e0..02f91558 100644
>> --- a/src/idmapped-mounts/idmapped-mounts.c
>> +++ b/src/idmapped-mounts/idmapped-mounts.c
>> @@ -51,6 +51,7 @@
>> #define FILE1_RENAME "file1_rename"
>> #define FILE2 "file2"
>> #define FILE2_RENAME "file2_rename"
>> +#define FILE3 "file3"
>> #define DIR1 "dir1"
>> #define DIR2 "dir2"
>> #define DIR3 "dir3"
>> @@ -337,6 +338,24 @@ out:
>> return fret;
>> }
>>
>> +static bool openat_tmpfile_supported(int dirfd)
>> +{
>> + int fd = -1;
>> +
>> + fd = openat(dirfd, ".", O_TMPFILE | O_RDWR, S_IXGRP | S_ISGID);
>> + if (fd == -1) {
>> + if (errno == ENOTSUP)
>> + return false;
>> + else
>> + return log_errno(false, "failure: create");
>> + }
>> +
>> + if (close(fd))
>> + log_stderr("failure: close");
>> +
>> + return true;
>> +}
>> +
>> /* __expected_uid_gid - check whether file is owned by the provided uid and gid */
>> static bool __expected_uid_gid(int dfd, const char *path, int flags,
>> uid_t expected_uid, gid_t expected_gid, bool log)
>> @@ -7841,7 +7860,10 @@ static int setgid_create(void)
>> {
>> int fret = -1;
>> int file1_fd = -EBADF;
>> + int tmpfile_fd = -EBADF;
>> pid_t pid;
>> + bool supported = false;
>> + char path[PATH_MAX];
>>
>> if (!caps_supported())
>> return 0;
>> @@ -7866,6 +7888,8 @@ static int setgid_create(void)
>> goto out;
>> }
>>
>> + supported = openat_tmpfile_supported(t_dir1_fd);
>> +
>> pid = fork();
>> if (pid< 0) {
>> log_stderr("failure: fork");
>> @@ -7929,6 +7953,25 @@ static int setgid_create(void)
>> if (unlinkat(t_dir1_fd, CHRDEV1, 0))
>> die("failure: delete");
>>
>> + /* create tmpfile via filesystem tmpfile api */
>> + if (supported) {
>> + tmpfile_fd = openat(t_dir1_fd, ".", O_TMPFILE | O_RDWR, S_IXGRP | S_ISGID);
>> + if (tmpfile_fd< 0)
>> + die("failure: create");
>> + /* link the temporary file into the filesystem, making it permanent */
>> + snprintf(path, PATH_MAX, "/proc/self/fd/%d", tmpfile_fd);
>> + if (linkat(AT_FDCWD, path, t_dir1_fd, FILE3, AT_SYMLINK_FOLLOW))
>> + die("failure: linkat");
>
> Fwiw, I don't think you need that snprintf() dance as you should be able
> to use AT_EMPTY_PATH:
>
> if (linkat(fd, "", t_dir1_fd, FILE3, AT_EMPTY_PATH))
>
> for this.
Oh, Yes, it works well. Thanks.
ps:I also use this way but failed before(I used wrong argument NULL
instead of "" when see open(2) man-pages ) .
Best Regards
Yang Xu
