Re: [PATCH v3 2/2] hostfs: store permissions in extended attributes

[Richard Weinberger <richard@nod.at>]

----- Ursprüngliche Mail -----
> Von: "Marko Petrović" <petrovicmarko2006@gmail.com>
> An: "linux-um" <linux-um@lists.infradead.org>
> CC: "richard" <richard@nod.at>, "anton ivanov" <anton.ivanov@cambridgegreys.com>, "Johannes Berg"
> <johannes@sipsolutions.net>, "Marko Petrović" <petrovicmarko2006@gmail.com>
> Gesendet: Samstag, 15. April 2023 18:48:21
> Betreff: [PATCH v3 2/2] hostfs: store permissions in extended attributes

> This patch adds support for xattrperm hostfs kernel command line option
> which enables the use of extended attributes for storing permissions by
> default on all mounted hostfs filesystems.

Sorry for the delay. Just unearthed this patch from patchwork.
 
> The support for per-mountpoint xattrperm/noxattrperm is added.
> 
> 'struct super_block -> s_fs_info' now points to 'struct hostfs_fs_info'.
> All code that relied on s_fs_info pointing to a string is changed to use
> the same string stored inside 'struct hostfs_fs_info'. This allows easy
> extensions of the super_block data in the future for storing mount
> options.
> 
> Function hostfs_parse_options() is added for parsing the string of
> mount options and storing them in 'struct hostfs_fs_info'.
> 
> When xattrperm is enabled, filesystem stores permissions in a
> human-readable string in attributes user.umlmode (for mode) and
> user.umlcred (for uid and gid).
> 
> Signed-off-by: Marko Petrović <petrovicmarko2006@gmail.com>
> ---
> fs/hostfs/hostfs.h      |  13 ++-
> fs/hostfs/hostfs_kern.c | 129 +++++++++++++++++----
> fs/hostfs/hostfs_user.c | 242 ++++++++++++++++++++++++++++++++++++----
> 3 files changed, 340 insertions(+), 44 deletions(-)
> 
> diff --git a/fs/hostfs/hostfs.h b/fs/hostfs/hostfs.h
> index 69cb796f6270..03f773b7c423 100644
> --- a/fs/hostfs/hostfs.h
> +++ b/fs/hostfs/hostfs.h
> @@ -37,6 +37,7 @@
>  * is on, and remove the appropriate bits from attr->ia_mode (attr is a
>  * "struct iattr *"). -BlaisorBlade
>  */
> +extern int use_xattr;
> struct hostfs_timespec {
> 	long long tv_sec;
> 	long long tv_nsec;
> @@ -67,7 +68,8 @@ struct hostfs_stat {
> 	unsigned int min;
> };
> 
> -extern int stat_file(const char *path, struct hostfs_stat *p, int fd);
> +extern int stat_file(const char *path, struct hostfs_stat *p, int fd,
> +							int mnt_use_xattr);
> extern int access_file(char *path, int r, int w, int x);
> extern int open_file(char *path, int r, int w, int append);
> extern void *open_dir(char *path, int *err_out);
> @@ -83,11 +85,14 @@ extern int write_file(int fd, unsigned long long *offset,
> const char *buf,
> 		      int len);
> extern int lseek_file(int fd, long long offset, int whence);
> extern int fsync_file(int fd, int datasync);
> -extern int file_create(char *name, int mode);
> -extern int set_attr(const char *file, struct hostfs_iattr *attrs, int fd);
> +extern int file_create(char *name, int mode, uid_t uid, gid_t gid,
> +							int mnt_use_xattr);
> +extern int set_attr(const char *file, struct hostfs_iattr *attrs, int fd,
> +							    int mnt_use_xattr);
> extern int make_symlink(const char *from, const char *to);
> extern int unlink_file(const char *file);
> -extern int do_mkdir(const char *file, int mode);
> +extern int do_mkdir(const char *file, int mode, uid_t uid, gid_t gid,
> +							int mnt_use_xattr);
> extern int hostfs_do_rmdir(const char *file);
> extern int do_mknod(const char *file, int mode, unsigned int major,
> 		    unsigned int minor);
> diff --git a/fs/hostfs/hostfs_kern.c b/fs/hostfs/hostfs_kern.c
> index 28b4f15c19eb..a2afe70abf14 100644
> --- a/fs/hostfs/hostfs_kern.c
> +++ b/fs/hostfs/hostfs_kern.c
> @@ -17,6 +17,7 @@
> #include <linux/writeback.h>
> #include <linux/mount.h>
> #include <linux/namei.h>
> +#include <linux/uidgid.h>
> #include "hostfs.h"
> #include <init.h>
> #include <kern.h>
> @@ -28,6 +29,11 @@ struct hostfs_inode_info {
> 	struct mutex open_mutex;
> };
> 
> +struct hostfs_fs_info {
> +	char host_root_path[PATH_MAX+1];

Why +1? AFAIK PATH_MAX includes space for the NUL byte.

> +	int use_xattr;

This is a very generic name.
Feel also free to use bool instead of int.

> +};
> +
> static inline struct hostfs_inode_info *HOSTFS_I(struct inode *inode)
> {
> 	return list_entry(inode, struct hostfs_inode_info, vfs_inode);
> @@ -64,6 +70,8 @@ static int __init hostfs_args(char *options, int *add)
> 		if (*options != '\0') {
> 			if (!strcmp(options, "append"))
> 				append = 1;
> +			else if (!strcmp(options, "xattrperm"))
> +				use_xattr = 1;
> 			else printf("hostfs_args - unsupported option - %s\n",
> 				    options);
> 		}
> @@ -79,18 +87,22 @@ __uml_setup("hostfs=", hostfs_args,
> "    tree on the host.  If this isn't specified, then a user inside UML can\n"
> "    mount anything on the host that's accessible to the user that's running\n"
> "    it.\n"
> -"    The only flag currently supported is 'append', which specifies that all\n"
> -"    files opened by hostfs will be opened in append mode.\n\n"
> +"    The only flags currently supported are 'append', which specifies that\n"
> +"    all files opened by hostfs will be opened in append mode and
> 'xattrperm'\n"
> +"    which specifies that permissions of files will be stored in extended\n"
> +"    attributes.\n\n"

Please mention that they are stored on xattrs on the host side.

> );
> #endif
> 
> static char *__dentry_name(struct dentry *dentry, char *name)
> {
> 	char *p = dentry_path_raw(dentry, name, PATH_MAX);
> +	struct hostfs_fs_info *sb_data;
> 	char *root;
> 	size_t len;
> 
> -	root = dentry->d_sb->s_fs_info;
> +	sb_data = dentry->d_sb->s_fs_info;
> +	root = sb_data->host_root_path;
> 	len = strlen(root);
> 	if (IS_ERR(p)) {
> 		__putname(name);
> @@ -203,8 +215,10 @@ static int hostfs_statfs(struct dentry *dentry, struct
> kstatfs *sf)
> 	long long f_bavail;
> 	long long f_files;
> 	long long f_ffree;
> +	struct hostfs_fs_info *sb_data;
> 
> -	err = do_statfs(dentry->d_sb->s_fs_info,
> +	sb_data = dentry->d_sb->s_fs_info;
> +	err = do_statfs(sb_data->host_root_path,
> 			&sf->f_bsize, &f_blocks, &f_bfree, &f_bavail, &f_files,
> 			&f_ffree, &sf->f_fsid, sizeof(sf->f_fsid),
> 			&sf->f_namelen);
> @@ -250,15 +264,21 @@ static void hostfs_free_inode(struct inode *inode)
> 
> static int hostfs_show_options(struct seq_file *seq, struct dentry *root)
> {
> -	const char *root_path = root->d_sb->s_fs_info;
> +	struct hostfs_fs_info *sb_data = root->d_sb->s_fs_info;
> 	size_t offset = strlen(root_ino) + 1;
> 
> -	if (strlen(root_path) > offset)
> -		seq_show_option(seq, root_path + offset, NULL);
> +	if (strlen(sb_data->host_root_path) > offset)
> +		seq_show_option(seq, sb_data->host_root_path + offset, NULL);
> 
> 	if (append)
> 		seq_puts(seq, ",append");
> 
> +	if (sb_data->use_xattr == 0)
> +		seq_puts(seq, ",noxattrperm");
> +
> +	if (sb_data->use_xattr == 1)
> +		seq_puts(seq, ",xattrperm");
> +
> 	return 0;
> }
> 
> @@ -516,7 +536,8 @@ static int read_name(struct inode *ino, char *name)
> {
> 	dev_t rdev;
> 	struct hostfs_stat st;
> -	int err = stat_file(name, &st, -1);
> +	struct hostfs_fs_info *sb_data = ino->i_sb->s_fs_info;
> +	int err = stat_file(name, &st, -1, sb_data->use_xattr);
> 	if (err)
> 		return err;
> 
> @@ -564,9 +585,13 @@ static int hostfs_create(struct mnt_idmap *idmap, struct
> inode *dir,
> 			 struct dentry *dentry, umode_t mode, bool excl)
> {
> 	struct inode *inode;
> +	struct hostfs_fs_info *sb_data;
> 	char *name;
> 	int error, fd;
> +	unsigned int currentuid;
> +	unsigned int currentgid;
> 
> +	sb_data = dentry->d_sb->s_fs_info;
> 	inode = hostfs_iget(dir->i_sb);
> 	if (IS_ERR(inode)) {
> 		error = PTR_ERR(inode);
> @@ -578,7 +603,10 @@ static int hostfs_create(struct mnt_idmap *idmap, struct
> inode *dir,
> 	if (name == NULL)
> 		goto out_put;
> 
> -	fd = file_create(name, mode & 0777);
> +	currentuid = from_kuid(current->cred->user_ns, current->cred->euid);
> +	currentgid = from_kgid(current->cred->user_ns, current->cred->egid);

I think we should better use filesystem instead of the effective id.
These days they're in most cases the same, but applications could still use setfsuid() and friends.

> +	fd = file_create(name, mode & 0777, currentuid, currentgid,
> +							   sb_data->use_xattr);
> 	if (fd < 0)
> 		error = fd;
> 	else
> @@ -675,12 +703,18 @@ static int hostfs_symlink(struct mnt_idmap *idmap, struct
> inode *ino,
> static int hostfs_mkdir(struct mnt_idmap *idmap, struct inode *ino,
> 			struct dentry *dentry, umode_t mode)
> {
> +	struct hostfs_fs_info *sb_data;
> 	char *file;
> 	int err;
> +	unsigned int currentuid;
> +	unsigned int currentgid;
> 
> +	sb_data = dentry->d_sb->s_fs_info;
> 	if ((file = dentry_name(dentry)) == NULL)
> 		return -ENOMEM;
> -	err = do_mkdir(file, mode);
> +	currentuid = from_kuid(current->cred->user_ns, current->cred->euid);
> +	currentgid = from_kgid(current->cred->user_ns, current->cred->egid);

Same.
Also make this a helper function, please.

> +	err = do_mkdir(file, mode, currentuid, currentgid, sb_data->use_xattr);
> 	__putname(file);
> 	return err;
> }
> @@ -796,10 +830,12 @@ static int hostfs_setattr(struct mnt_idmap *idmap,
> {
> 	struct inode *inode = d_inode(dentry);
> 	struct hostfs_iattr attrs;
> +	struct hostfs_fs_info *sb_data;
> 	char *name;
> 	int err;
> 
> 	int fd = HOSTFS_I(inode)->fd;
> +	sb_data = dentry->d_sb->s_fs_info;
> 
> 	err = setattr_prepare(&nop_mnt_idmap, dentry, attr);
> 	if (err)
> @@ -849,7 +885,7 @@ static int hostfs_setattr(struct mnt_idmap *idmap,
> 	name = dentry_name(dentry);
> 	if (name == NULL)
> 		return -ENOMEM;
> -	err = set_attr(name, &attrs, fd);
> +	err = set_attr(name, &attrs, fd, sb_data->use_xattr);

Instead of passing use_xattr to very function just pass sb_data itself
as first parameter. Maybe it is useful later for passing other filesystem
context stuff.


> 	__putname(name);
> 	if (err)
> 		return err;
> @@ -915,10 +951,58 @@ static const struct inode_operations hostfs_link_iops = {
> 	.get_link	= hostfs_get_link,
> };
> 
> +static int hostfs_parse_options(struct hostfs_fs_info *sb_data, const char *d)
> +{
> +	int err = 0, unknown = 0;
> +	char *ptr, *options, *string;
> +
> +	snprintf(sb_data->host_root_path, PATH_MAX, "%s/", root_ino);

Why is host_root_path a static array? Just use kasprintf() or such.

> +	sb_data->use_xattr = -1;
> +	if (d == NULL)
> +		return 0;
> +
> +	string = kmalloc(strlen(d) + 1, GFP_KERNEL);
> +	if (string == NULL) {
> +		err = -ENOMEM;
> +		goto out;
> +	}
> +	options = string;
> +	strcpy(options, d);

Why not strdup()?

> +
> +	while (options) {
> +		ptr = strchr(options, ',');
> +		if (ptr != NULL)
> +			*ptr++ = '\0';
> +		if (*options != '\0') {
> +			if (!strcmp(options, "noxattrperm")) {
> +				sb_data->use_xattr = 0;
> +				goto _break;
> +			}
> +			if (!strcmp(options, "xattrperm")) {
> +				sb_data->use_xattr = 1;
> +				goto _break;
> +			}
> +			if (!unknown) {
> +				unknown = 1;
> +				strcat(sb_data->host_root_path, options);
> +				goto _break;
> +			}
> +			pr_warn("hostfs: unsupported mount option\n");
> +			err = -EINVAL;
> +			goto out;
> +		}
> +_break:
> +		options = ptr;
> +	}
> +
> +out:
> +	kfree(string);
> +	return err;
> +}
> static int hostfs_fill_sb_common(struct super_block *sb, void *d, int silent)
> {
> 	struct inode *root_inode;
> -	char *host_root_path, *req_root = d;
> +	struct hostfs_fs_info *sb_data;
> 	int err;
> 
> 	sb->s_blocksize = 1024;
> @@ -931,26 +1015,26 @@ static int hostfs_fill_sb_common(struct super_block *sb,
> void *d, int silent)
> 	if (err)
> 		goto out;
> 
> -	/* NULL is printed as '(null)' by printf(): avoid that. */
> -	if (req_root == NULL)
> -		req_root = "";
> -
> 	err = -ENOMEM;
> -	sb->s_fs_info = host_root_path =
> -		kasprintf(GFP_KERNEL, "%s/%s", root_ino, req_root);
> -	if (host_root_path == NULL)
> +	sb_data = kmalloc(sizeof(struct hostfs_fs_info), GFP_KERNEL);
> +	if (sb_data == NULL)
> +		goto out;
> +
> +	err = hostfs_parse_options(sb_data, d);
> +	if (err != 0)
> 		goto out;
> +	sb->s_fs_info = sb_data;
> 
> 	root_inode = new_inode(sb);
> 	if (!root_inode)
> 		goto out;
> 
> -	err = read_name(root_inode, host_root_path);
> +	err = read_name(root_inode, sb_data->host_root_path);

Also for such functions, just pass sb_data.

> 	if (err)
> 		goto out_put;
> 
> 	if (S_ISLNK(root_inode->i_mode)) {
> -		char *name = follow_link(host_root_path);
> +		char *name = follow_link(sb_data->host_root_path);
> 		if (IS_ERR(name)) {
> 			err = PTR_ERR(name);
> 			goto out_put;
> @@ -1001,6 +1085,9 @@ static int __init init_hostfs(void)
> 	hostfs_inode_cache = KMEM_CACHE(hostfs_inode_info, 0);
> 	if (!hostfs_inode_cache)
> 		return -ENOMEM;
> +	#ifdef MODULE
> +	use_xattr = 0;
> +	#endif

Why is this needed?

> 	return register_filesystem(&hostfs_type);
> }
> 
> diff --git a/fs/hostfs/hostfs_user.c b/fs/hostfs/hostfs_user.c
> index 5ecc4706172b..f5dd524cd153 100644
> --- a/fs/hostfs/hostfs_user.c
> +++ b/fs/hostfs/hostfs_user.c
> @@ -10,14 +10,19 @@
> #include <errno.h>
> #include <fcntl.h>
> #include <string.h>
> +#include <stdlib.h>
> #include <sys/stat.h>
> #include <sys/time.h>
> #include <sys/types.h>
> #include <sys/vfs.h>
> #include <sys/syscall.h>
> +#include <sys/xattr.h>
> +#include <um_malloc.h>
> #include "hostfs.h"
> #include <utime.h>
> 
> +int use_xattr;
> +
> static void stat64_to_hostfs(const struct stat64 *buf, struct hostfs_stat *p)
> {
> 	p->ino = buf->st_ino;
> @@ -38,7 +43,184 @@ static void stat64_to_hostfs(const struct stat64 *buf,
> struct hostfs_stat *p)
> 	p->min = os_minor(buf->st_rdev);
> }
> 
> -int stat_file(const char *path, struct hostfs_stat *p, int fd)
> +static int uml_chown(const char *pathname, uid_t owner, gid_t group,
> +							int mnt_use_xattr)
> +{
> +	int xattr = (mnt_use_xattr == -1) ? use_xattr : mnt_use_xattr;

At this level you should not distinguish between a flag set at mount time or globally.
Use one variable and set it at mount time.
e.g. the global module flag is the default and a mount option can override it.

> +	/* 10 digits uid, 10 digits gid, null byte and ',' */
> +	char umlcred[22];
> +	/* max_gid=4294967295 - 10 digits + null byte */
> +	char gid[11];
> +	int i = 0;
> +
> +	if (xattr) {
> +		memset(umlcred, 0, sizeof(umlcred));
> +		getxattr(pathname, "user.umlcred", umlcred, sizeof(umlcred));

You need to check the return code. What if the xattr is not there?

> +		while (umlcred[i] != ',' && umlcred[i] != '\0')
> +			i++;

What if the xattr contains garbage and no NULL terminator?
...then you loop here forever.

> +		umlcred[i] = '\0';
> +
> +		if (group == -1)
> +			strncpy(gid, &umlcred[i+1], sizeof(gid));
> +		else
> +			snprintf(gid, sizeof(gid), "%u", group);
> +		if (owner != -1)
> +			snprintf(umlcred, sizeof(gid), "%u", owner);
> +
> +		strcat(umlcred, ",");
> +		strncat(umlcred, gid, sizeof(gid-1));

I'm really not a fan of such old school C string hackery.
Can't you read using ssscanf() and produce the new string with snprintf() or such?

> +		if (setxattr(pathname, "user.umlcred", umlcred,
> +						strlen(umlcred)+1, 0))
> +			return -errno;
> +
> +		return 0;
> +	}
> +	if (chown(pathname, owner, group))
> +		return -errno;
> +	return 0;
> +}
> +
> +static int uml_fchown(int fd, uid_t owner, gid_t group, int mnt_use_xattr)
> +{
> +	int xattr = (mnt_use_xattr == -1) ? use_xattr : mnt_use_xattr;
> +	/* 10 digits uid, 10 digits gid, null byte and ',' */
> +	char umlcred[22];
> +	/* max_gid=4294967295 - 10 digits + null byte */
> +	char gid[11];
> +	int i = 0;
> +
> +	if (xattr) {
> +		memset(umlcred, 0, sizeof(umlcred));
> +		fgetxattr(fd, "user.umlcred", umlcred, sizeof(umlcred));
> +
> +		while (umlcred[i] != ',' && umlcred[i] != '\0')
> +			i++;
> +		umlcred[i] = '\0';
> +
> +		if (group == -1)
> +			strncpy(gid, &umlcred[i+1], sizeof(gid));
> +		else
> +			snprintf(gid, sizeof(gid), "%u", group);
> +		if (owner != -1)
> +			snprintf(umlcred, sizeof(gid), "%u", owner);
> +
> +		strcat(umlcred, ",");
> +		strncat(umlcred, gid, sizeof(gid-1));

Please add a helper function for common code.

> +		if (fsetxattr(fd, "user.umlcred", umlcred, strlen(umlcred)+1, 0))
> +			return -errno;
> +
> +		return 0;
> +	}
> +	if (fchown(fd, owner, group))
> +		return -errno;
> +	return 0;
> +}
> +
> +static int uml_chmod(const char *pathname, unsigned int mode, int
> mnt_use_xattr)
> +{
> +	int xattr = (mnt_use_xattr == -1) ? use_xattr : mnt_use_xattr;
> +	int i;
> +	char mode_string[7];	/* mode is 16-bit -> max 6 digits in octal */
> +
> +	if (xattr) {
> +		strcpy(mode_string, "000000");
> +		i = sizeof(mode_string) - 1;
> +		while (mode > 0) {
> +			mode_string[i] = (mode % 8) + '0';
> +			mode = mode / 8;
> +			i--;
> +		}

kstrtoint()?

> +		if (setxattr(pathname, "user.umlmode", mode_string,
> +						       sizeof(mode_string), 0))

So we have user.umlcred and user.umlmode xattrs on the host side.
Why not just one xattr?

> +			return -errno;
> +		return 0;
> +	}
> +	if (chmod(pathname, mode))
> +		return -errno;
> +	return 0;
> +}
> +
> +static int uml_fchmod(int fd, unsigned int mode, int mnt_use_xattr)
> +{
> +	int xattr = (mnt_use_xattr == -1) ? use_xattr : mnt_use_xattr;
> +	int i;
> +	char mode_string[7];	/* mode is 16-bit -> max 6 digits in octal */
> +
> +	if (xattr) {
> +		strcpy(mode_string, "000000");
> +		i = sizeof(mode_string) - 1;
> +		while (mode > 0) {
> +			mode_string[i] = (mode % 8) + '0';
> +			mode = mode / 8;
> +			i--;
> +		}
> +		if (fsetxattr(fd, "user.umlmode", mode_string,
> +						  sizeof(mode_string), 0))
> +			return -errno;
> +		return 0;
> +	}
> +	if (fchmod(fd, mode))
> +		return -errno;
> +	return 0;
> +}
> +
> +static void read_permissions(const char *path, struct stat64 *p,
> +							int mnt_use_xattr)
> +{
> +	unsigned int mode = 0, i;
> +	char mode_string[7], umlcred[22];
> +	int xattr = (mnt_use_xattr == -1) ? use_xattr : mnt_use_xattr;
> +
> +	if (!xattr)
> +		return;
> +
> +	if (getxattr(path, "user.umlmode", mode_string, sizeof(mode_string)) != -1) {
> +		i = 0;
> +		while (mode_string[i] != '\0') {
> +			mode *= 8;
> +			mode += mode_string[i] - '0';
> +			i++;
> +		}
> +		p->st_mode = mode;
> +	}
> +
> +	if (getxattr(path, "user.umlcred", umlcred, sizeof(umlcred)) != -1) {
> +		i = 0;
> +		while (umlcred[i] != ',')
> +			i++;
> +		umlcred[i] = '\0';
> +
> +		if (strlen(umlcred) > 0)
> +			p->st_uid = atoi(umlcred);
> +		if (strlen(&umlcred[i+1]) > 0)
> +			p->st_gid = atoi(&umlcred[i+1]);
> +	}
> +}
> +
> +static long is_set_gid(const char *path, int mnt_use_xattr)
> +{
> +	int i = strlen(path) + 1;
> +	char *parent = uml_kmalloc(i, UM_GFP_KERNEL);
> +	struct stat64 buf = { 0 };
> +
> +	if (parent == NULL)
> +		return -1;
> +	strcpy(parent, path);
> +	i = i - 3;
> +	while (parent[i] != '/')
> +		i--;
> +	parent[i] = '\0';
> +
> +	stat64(parent, &buf);
> +	read_permissions(parent, &buf, mnt_use_xattr);
> +	kfree(parent);
> +	if (buf.st_mode & S_ISGID)
> +		return buf.st_gid;
> +	return -1;
> +}
> +
> +int stat_file(const char *path, struct hostfs_stat *p, int fd, int
> mnt_use_xattr)
> {
> 	struct stat64 buf;
> 
> @@ -48,6 +230,7 @@ int stat_file(const char *path, struct hostfs_stat *p, int
> fd)
> 	} else if (lstat64(path, &buf) < 0) {
> 		return -errno;
> 	}
> +	read_permissions(path, &buf, mnt_use_xattr);
> 	stat64_to_hostfs(&buf, p);
> 	return 0;
> }
> @@ -181,44 +364,60 @@ void close_dir(void *stream)
> 	closedir(stream);
> }
> 
> -int file_create(char *name, int mode)
> +int file_create(char *name, int mode, uid_t uid, gid_t gid, int mnt_use_xattr)
> {
> 	int fd;
> +	long ret;
> 
> 	fd = open64(name, O_CREAT | O_RDWR, mode);
> 	if (fd < 0)
> 		return -errno;
> +
> +	ret = is_set_gid(name, mnt_use_xattr);
> +	if (ret != -1)
> +		gid = ret;
> +	uml_chown(name, uid, gid, mnt_use_xattr);
> 	return fd;
> }
> 
> -int set_attr(const char *file, struct hostfs_iattr *attrs, int fd)
> +int set_attr(const char *file, struct hostfs_iattr *attrs, int fd,
> +							     int mnt_use_xattr)
> {
> 	struct hostfs_stat st;
> 	struct timeval times[2];
> -	int err, ma;
> +	int err, ma, ret;
> 
> 	if (attrs->ia_valid & HOSTFS_ATTR_MODE) {
> 		if (fd >= 0) {
> -			if (fchmod(fd, attrs->ia_mode) != 0)
> -				return -errno;
> -		} else if (chmod(file, attrs->ia_mode) != 0) {
> -			return -errno;
> +			ret = uml_fchmod(fd, attrs->ia_mode, mnt_use_xattr);
> +			if (ret != 0)
> +				return ret;
> +		} else {
> +			ret = uml_chmod(file, attrs->ia_mode, mnt_use_xattr);
> +			if (ret != 0)
> +				return ret;
> 		}
> 	}
> 	if (attrs->ia_valid & HOSTFS_ATTR_UID) {
> 		if (fd >= 0) {
> -			if (fchown(fd, attrs->ia_uid, -1))
> -				return -errno;
> -		} else if (chown(file, attrs->ia_uid, -1)) {
> -			return -errno;
> +			ret = uml_fchown(fd, attrs->ia_uid, -1, mnt_use_xattr);
> +			if (ret != 0)
> +				return ret;
> +		} else {
> +			ret = uml_chown(file, attrs->ia_uid, -1, mnt_use_xattr);
> +			if (ret != 0)
> +				return ret;
> 		}
> 	}
> 	if (attrs->ia_valid & HOSTFS_ATTR_GID) {
> 		if (fd >= 0) {
> -			if (fchown(fd, -1, attrs->ia_gid))
> -				return -errno;
> -		} else if (chown(file, -1, attrs->ia_gid)) {
> -			return -errno;
> +			ret = uml_fchown(fd, -1, attrs->ia_gid, mnt_use_xattr);
> +			if (ret != 0)
> +				return ret;
> +		} else {
> +			ret = uml_chown(file, -1, attrs->ia_gid, mnt_use_xattr);
> +			if (ret != 0)
> +				return ret;
> 		}
> 	}
> 	if (attrs->ia_valid & HOSTFS_ATTR_SIZE) {
> @@ -237,7 +436,7 @@ int set_attr(const char *file, struct hostfs_iattr *attrs,
> int fd)
> 	 */
> 	ma = (HOSTFS_ATTR_ATIME_SET | HOSTFS_ATTR_MTIME_SET);
> 	if (attrs->ia_valid & ma) {
> -		err = stat_file(file, &st, fd);
> +		err = stat_file(file, &st, fd, mnt_use_xattr);
> 		if (err != 0)
> 			return err;
> 
> @@ -265,7 +464,7 @@ int set_attr(const char *file, struct hostfs_iattr *attrs,
> int fd)
> 
> 	/* Note: ctime is not handled */
> 	if (attrs->ia_valid & (HOSTFS_ATTR_ATIME | HOSTFS_ATTR_MTIME)) {
> -		err = stat_file(file, &st, fd);
> +		err = stat_file(file, &st, fd, mnt_use_xattr);
> 		attrs->ia_atime = st.atime;
> 		attrs->ia_mtime = st.mtime;
> 		if (err != 0)
> @@ -294,13 +493,18 @@ int unlink_file(const char *file)
> 	return 0;
> }
> 
> -int do_mkdir(const char *file, int mode)
> +int do_mkdir(const char *file, int mode, uid_t uid, gid_t gid, int
> mnt_use_xattr)
> {
> 	int err;
> +	long ret;
> 
> 	err = mkdir(file, mode);
> 	if (err)
> 		return -errno;
> +	ret = is_set_gid(file, mnt_use_xattr);
> +	if (ret != -1)
> +		gid = ret;
> +	uml_chown(file, uid, gid, mnt_use_xattr);
> 	return 0;
> }
> 
> --
> 2.39.2

_______________________________________________
linux-um mailing list
linux-um@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-um