linux-ext4 - Re: [v5 5/5] Adds FS_IOC_FSSETXATTR/FS_IOC_FSGETXATTR interface support for ext4

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 28 Oct 2014 09:40:58 +1100
From:	Dave Chinner <david@...morbit.com>
To:	Li Xi <pkuelelixi@...il.com>
Cc:	"linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
	Ext4 Developers List <linux-ext4@...r.kernel.org>,
	"linux-api@...r.kernel.org" <linux-api@...r.kernel.org>,
	Theodore Ts'o <tytso@....edu>,
	Andreas Dilger <adilger@...ger.ca>, Jan Kara <jack@...e.cz>,
	"viro@...iv.linux.org.uk" <viro@...iv.linux.org.uk>,
	"hch@...radead.org" <hch@...radead.org>,
	Dmitry Monakhov <dmonakhov@...nvz.org>
Subject: Re: [v5 5/5] Adds FS_IOC_FSSETXATTR/FS_IOC_FSGETXATTR interface
 support for ext4

On Mon, Oct 27, 2014 at 09:09:19AM +0800, Li Xi wrote:
> On Mon, Oct 27, 2014 at 5:56 AM, Dave Chinner <david@...morbit.com> wrote:
> > On Sun, Oct 26, 2014 at 01:22:53PM +0800, Li Xi wrote:
> >> This patch adds FS_IOC_FSSETXATTR/FS_IOC_FSGETXATTR ioctl interface
> >> support for ext4. The interface is kept consistent with
> >> XFS_IOC_FSGETXATTR/XFS_IOC_FSGETXATTR.
> >
> > What you haven't mentioned is that you also changed the fsxattr
> > interface structure to add functionality and new behaviours that
> > isn't supported by XFS or existing applications that use the
> > interface.
> >
> > There is no need to modify the interface *at all* for ext4 to use
> > it. Fields that ext4 does not use can be zeroed on getxattr, and
> > ignored on setxattr - you do not need to add new fields to say what
> > fields are valid.
> Sorry, I don't want to change the interfaces either. But, the problem
> is that zero might be valid value for some fields. How can we
> distinguish an unsupported attribute and an attribute whose value
> is zero?

You don't. Userspace has no concept of what parts of the struct
fsxattr are valid or not, nor what are valid values the filesystem
will accept or reject.

> It is common case the only part of the fields are supported.
> So, for example, if we don't have valid flags, how can use space
> application tell kernel which attributes should be skipped when it
> tries to set only a part of atrributes?

It *doesn't*. Userspace requires the kernel to initialise the struct
fsxattr before it tries to modify anything, just like
fcntl(F_[GS]ETFL) and other similar "file flag change" syscall APIs.

IOWs, you have to initialise the struct fsxattr by calling
FS_IOC_FSGETXATTR before you call FS_IOC_FSSETXATTR. The
intialisation sets all the fields to the current (correct) values,
and hence when the set call is made all the fields then have the
same/correct values in them except for what the application changed.

E.g. this code from xfs_quota to clear the project ID on a given
file or directory:

	if ((fd = open(path, O_RDONLY|O_NOCTTY)) == -1) {
		// error handling ....
        } else if (xfsctl(path, fd, XFS_IOC_FSGETXATTR, &fsx) < 0) {
		// error handling ....
        }

        fsx.fsx_projid = 0;
        fsx.fsx_xflags &= ~XFS_XFLAG_PROJINHERIT;
        if (xfsctl(path, fd, XFS_IOC_FSSETXATTR, &fsx) < 0) {
		// error handling ....
        }
        close(fd);

This ensures that *only* the project ID and the specific project ID
inheritance flag is cleared, and none of the other inode flags or
state are modified....

Cheers,

Dave.
-- 
Dave Chinner
david@...morbit.com
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html