Linuxulator: fix symbol link file permissions returned by stat

[ankohuu_outlook.com]

Summary

According to the POSIX standard，symbolic links can always be read, except that the value of the file mode bits returned in the st_mode field of the stat structure is unspecified.

Linux implemented like this: stat() always returns st_mode 0777 for almost symbolic link files, but on FreeBSD returns its own real permission.
Some linux programs do obtain the symbol link file permissions in proc or sys folders and check whether it is equal to 777, so I think maybe linux_stat return 0777 better at this time.

BTW, I found two exceptions on Linux, I don’t know if there are any other exceptions.

1. lr-x------ /proc/xxx/fd/yyy

If file is open for reading, mode is S_IRUSR | S_IXUSR.
If file is open for writing, mode is S_IWUSR | S_IXUSR.
linux code 5.7-rc1:

static void tid_fd_update_inode(struct task_struct *task, struct inode *inode, fmode_t f_mode)
{
    task_dump_owner(task, 0, &inode->i_uid, &inode->i_gid);

    if (S_ISLNK(inode->i_mode)) {
        unsigned i_mode = S_IFLNK;
        if (f_mode & FMODE_READ)
            i_mode |= S_IRUSR | S_IXUSR;
        if (f_mode & FMODE_WRITE)
            i_mode |= S_IWUSR | S_IXUSR;
        inode->i_mode = i_mode;
    }
    security_task_to_inode(task, inode);
}

In this case, we let fdescfs handle it.

2. lr-------- /proc/aaa/map_files/bbb-ccc

If file is open for reading, mode is S_IRUSR.
If file is open for writing, mode is S_IWUSR.
linux code 5.7-rc1:

static struct dentry *
proc_map_files_instantiate(struct dentry *dentry, struct task_struct *task, const void *ptr)
{
    fmode_t mode = (fmode_t)(unsigned long)ptr;
    struct proc_inode *ei;
    struct inode *inode;

    inode = proc_pid_make_inode(dentry->d_sb, task, S_IFLNK |
                    ((mode & FMODE_READ ) ? S_IRUSR : 0) |
                    ((mode & FMODE_WRITE) ? S_IWUSR : 0));
    if (!inode)
        return ERR_PTR(-ENOENT);

    ei = PROC_I(inode);
    ei->op.proc_get_link = map_files_get_link;

    inode->i_op = &proc_map_files_link_inode_operations;
    inode->i_size = 64;

    d_set_d_op(dentry, &tid_map_files_dentry_operations);
    return d_splice_alias(inode, dentry);
}

In this case, we don't have such files and just skip.

[kib]

sys/compat/linux/linux_stats.c

93

Why do you need to check for VV_READLINK? Isn't it already handled in the changed S_IF<MODE>, and S_ISLNK() would be false?

I suggest to move this if() to 'else if' of vn_isdisk() check.

[ankohuu_outlook.com]

sys/compat/linux/linux_stats.c

93

Why do you need to check for VV_READLINK? Isn't it already handled in the changed S_IF<MODE>, and S_ISLNK() would be false?

I suggest to move this if() to 'else if' of vn_isdisk() check.

93

Why do you need to check for VV_READLINK? Isn't it already handled in the changed S_IF<MODE>, and S_ISLNK() would be false?

I suggest to move this if() to 'else if' of vn_isdisk() check.

93

Why do you need to check for VV_READLINK?

I did this to find out the symlink files under the path /proc/self/fd, on Linux not all symlink permissions are 777 as I mentioned in the summary, files in /proc/self/fd follow rules below,
if file is open for reading, mode is S_IRUSR | S_IXUSR，if file is open for writing, mode is S_IWUSR | S_IXUSR. In rc.d/linux we mount devfs to /compat/linux/dev with linrdlnk option,
files under it have VV_READLINK flag, and /compat/linux/proc/self/fd is a link to /compat/linux/dev.

Isn't it already handled in the changed S_IF<MODE>, and S_ISLNK() would be false?

do you mean this code?

if (vn_isdisk(vp)) {
		sb->st_mode &= ~S_IFMT;
		sb->st_mode |= S_IFBLK;
}

I think only some VCHRs can change their mode to S_IFBLK, my logic only deals with VLNKs, they are mutually exclusive and will not affect each other

I suggest to move this if() to 'else if' of vn_isdisk() check.

Yes it will improve efficiency, I made some changes, please check

[ankohuu_outlook.com]

1. call perm modify function only when file is symbol link
2. only call perm function in lstat hook
3. if fdescfs mounted in /compat/linux/dev, file perm generated by open flag

[kib]

sys/compat/linux/linux_stats.c

93

No, I mean that VV_READLINK is irrelevant there. Look at the fdescfs code where it is set and used.

[kib]

sys/fs/fdescfs/fdesc_vnops.c

465

This is severe layering violation, fget() is before vnode lock in the global order. I think you do not get a warning from any debugging facility in kernel because fget() is mostly avoids locks ATM. Sill, this must not be done.

I do not have only two possible suggestions: either do not do anything, or possibly cache file flags in fdescfs vnode v_data at the time of fdesc_allocvp(). There is fget-ed file around that time.