diff mbox series

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

Message ID 20230415164821.166606-1-petrovicmarko2006@gmail.com
State Changes Requested
Headers show
Series None | expand

Commit Message

Marko Petrović April 15, 2023, 4:48 p.m. UTC
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.

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(-)

Comments

Marko Petrović April 16, 2023, 5:24 p.m. UTC | #1
Hello,

I have written the third version of the patch. Thank you for all of your
recommendations.

While writing the third patch version, I noticed that there was a
serious limitation of the code in second patch, namely the whole
xattrperm feature was available only and only as boot time flag so it
could not be used when hostfs was built as module since modules don't
have hostfs_args() function.

To overcome that issue, I have changed the content of
struct super_block -> s_fs_info to point to a struct hostfs_fs_info
containing the string that was previously there (to be used by old
functions) and the per-mountpoint use_xattr flag.
This allows easy extending of mount options in the future and thus
providing more flexibility to userspace to configure the filesystem.
For example, hostfs_attr, acl and noacl could be the mount options added
for POSIX ACLs and extended attributes in the future and if there is a
desire for that, append could become a mount option now too.

Regarding xattrperm as the kernel boot parameter, I left it available and
it defines the default behavior when mounting the filesystem (when
neither xattrperm nor noxattrperm is specified in mount options).

What do you think about this new change? Please, let me know if you have
any concerns so that I can address them.

Best regards,
Marko Petrović
Johannes Berg April 18, 2023, 8:31 a.m. UTC | #2
Hi,

On Sun, 2023-04-16 at 19:24 +0200, Marko Petrović wrote:
> 
> I have written the third version of the patch. Thank you for all of your
> recommendations.
> 
> While writing the third patch version, I noticed that there was a
> serious limitation of the code in second patch, namely the whole
> xattrperm feature was available only and only as boot time flag so it
> could not be used when hostfs was built as module since modules don't
> have hostfs_args() function.
> 
> To overcome that issue, I have changed the content of
> struct super_block -> s_fs_info to point to a struct hostfs_fs_info
> containing the string that was previously there (to be used by old
> functions) and the per-mountpoint use_xattr flag.
> This allows easy extending of mount options in the future and thus
> providing more flexibility to userspace to configure the filesystem.
> For example, hostfs_attr, acl and noacl could be the mount options added
> for POSIX ACLs and extended attributes in the future and if there is a
> desire for that, append could become a mount option now too.
> 
> Regarding xattrperm as the kernel boot parameter, I left it available and
> it defines the default behavior when mounting the filesystem (when
> neither xattrperm nor noxattrperm is specified in mount options).

It seems that _maybe_, similar to the 'hostfs' kernel argument, there
should be a way to contain the set of options?

What do I mean by that? I mean that today, the inside of the virtual
machine (for lack of a better word) can only mount outside folders that
are contained in the folder indicated by the 'hostfs' argument.
Similarly, perhaps the "outside administrator" should be able to
indicate that xattr permissions _must_ be used, or _must not_ be used?

Which would imply a new kernel argument that can be configured to "force
use", "force don't use" and "don't care", with perhaps even for backward
compatibility the default being "force don't use"?

Not sure. Anton? Richard? Any opinions?

johannes
Marko Petrović April 25, 2023, 4:35 p.m. UTC | #3
On Tue Apr 18, 2023 at 10:31 AM CEST, Johannes Berg wrote:
Hello,
Thanks for your reply!
> Hi,
>
> On Sun, 2023-04-16 at 19:24 +0200, Marko Petrović wrote:
> > 
> > I have written the third version of the patch. Thank you for all of your
> > recommendations.
> > 
> > While writing the third patch version, I noticed that there was a
> > serious limitation of the code in second patch, namely the whole
> > xattrperm feature was available only and only as boot time flag so it
> > could not be used when hostfs was built as module since modules don't
> > have hostfs_args() function.
> > 
> > To overcome that issue, I have changed the content of
> > struct super_block -> s_fs_info to point to a struct hostfs_fs_info
> > containing the string that was previously there (to be used by old
> > functions) and the per-mountpoint use_xattr flag.
> > This allows easy extending of mount options in the future and thus
> > providing more flexibility to userspace to configure the filesystem.
> > For example, hostfs_attr, acl and noacl could be the mount options added
> > for POSIX ACLs and extended attributes in the future and if there is a
> > desire for that, append could become a mount option now too.
> > 
> > Regarding xattrperm as the kernel boot parameter, I left it available and
> > it defines the default behavior when mounting the filesystem (when
> > neither xattrperm nor noxattrperm is specified in mount options).
>
> It seems that _maybe_, similar to the 'hostfs' kernel argument, there
> should be a way to contain the set of options?
>
> What do I mean by that? I mean that today, the inside of the virtual
> machine (for lack of a better word) can only mount outside folders that
> are contained in the folder indicated by the 'hostfs' argument.
> Similarly, perhaps the "outside administrator" should be able to
> indicate that xattr permissions _must_ be used, or _must not_ be used?
Nice observation. It shouldn't be hard to do this, I can just change
the interpreted meaning of mnt_use_xattr and hostfs_fs_info->use_xattr
to comply with this behavior. Thanks for bringing this to attention.
>
> Which would imply a new kernel argument that can be configured to "force
> use", "force don't use" and "don't care", with perhaps even for backward
> compatibility the default being "force don't use"?
>
> Not sure. Anton? Richard? Any opinions?
Maybe xattrperm and noxattrperm can be kernel command line arguments
used for "force use" and "force don't use" and when none is specified,
the behavior could be "don't care" which would thus be the default.

That may also be reasonable for the purpose of backward compatibility
since the usage of extended attributes would then be specified as an mount
option and applications not aware of it would just use the old behavior
(since the extended attributes would be used only when specified in
mount options).

On the other hand, that would require a little different mounting of
root filesystem. Maybe adding rootxattrperm as a new kernel command line
argument for mounting root with "rootfstype=hostfs hostfs=rootxattrperm"
could be the solution (for when root should use extended attributes, but
the general behavior should still be "don't care")?
What are your opinions?

Best regards,
Marko Petrović
Johannes Berg April 25, 2023, 5:11 p.m. UTC | #4
On Tue, 2023-04-25 at 18:35 +0200, Marko Petrović wrote:
> > It seems that _maybe_, similar to the 'hostfs' kernel argument, there
> > should be a way to contain the set of options?
> > 
> > What do I mean by that? I mean that today, the inside of the virtual
> > machine (for lack of a better word) can only mount outside folders that
> > are contained in the folder indicated by the 'hostfs' argument.
> > Similarly, perhaps the "outside administrator" should be able to
> > indicate that xattr permissions _must_ be used, or _must not_ be used?
> Nice observation. It shouldn't be hard to do this, I can just change
> the interpreted meaning of mnt_use_xattr and hostfs_fs_info->use_xattr
> to comply with this behavior. Thanks for bringing this to attention.
> > 
> > Which would imply a new kernel argument that can be configured to "force
> > use", "force don't use" and "don't care", with perhaps even for backward
> > compatibility the default being "force don't use"?
> > 
> > Not sure. Anton? Richard? Any opinions?

> Maybe xattrperm and noxattrperm can be kernel command line arguments
> used for "force use" and "force don't use" and when none is specified,
> the behavior could be "don't care" which would thus be the default.

Right. Actually now looking at this again, they should probably be flags
inside the hostfs= argument? Like the "append" flag now.

Not really sure what the default should be, perhaps it makes sense to
not allow it by default so it's the same as now? But I don't know how
strict we need to be about this.

> That may also be reasonable for the purpose of backward compatibility
> since the usage of extended attributes would then be specified as an mount
> option and applications not aware of it would just use the old behavior
> (since the extended attributes would be used only when specified in
> mount options).

Right. I was more thinking of the isolation aspects of this.

> On the other hand, that would require a little different mounting of
> root filesystem. Maybe adding rootxattrperm as a new kernel command line
> argument for mounting root with "rootfstype=hostfs hostfs=rootxattrperm"
> could be the solution (for when root should use extended attributes, but
> the general behavior should still be "don't care")?
> What are your opinions?

Oh, that's a good point too. I don't think I have much of an opinion on
it though. But yeah, why not have another flag "rootxattrperm" for the
hostfs= option, along with xattrperm and noxattrperm (or allowxattrperm
and forcexattrperm if we need noxattrperm to be the default per above.)

johannes

PS: Note that in uml/next my patches with the split are merged, so when
you rebase please rebase on that and adjust accordingly with the
exported symbol we discussed.
Richard Weinberger Aug. 28, 2023, 7:48 p.m. UTC | #5
----- 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
diff mbox series

Patch

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];
+	int use_xattr;
+};
+
 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"
 );
 #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);
+	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);
+	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);
 	__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);
+	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);
+
+	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);
 	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
 	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;
+	/* 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));
+
+		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));
+		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));
+		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--;
+		}
+		if (setxattr(pathname, "user.umlmode", mode_string,
+						       sizeof(mode_string), 0))
+			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;
 }