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:	Mon, 2 May 2016 16:48:13 -0600
From:	Andreas Dilger <adilger@...ger.ca>
To:	David Howells <dhowells@...hat.com>
Cc:	linux-fsdevel@...r.kernel.org, linux-afs@...r.kernel.org,
	linux-nfs@...r.kernel.org, samba-technical@...ts.samba.org,
	linux-kernel@...r.kernel.org, linux-ext4@...r.kernel.org
Subject: Re: [PATCH 3/6] statx: Ext4: Return enhanced file attributes

On Apr 29, 2016, at 6:57 AM, David Howells <dhowells@...hat.com> wrote:
> 
> Return enhanced file attributes from the Ext4 filesystem.  This includes
> the following:
> 
> (1) The inode creation time (i_crtime) as i_btime, setting STATX_BTIME.
> 
> (2) The inode i_version as st_version if a file with I_VERSION set or a
>     directory, setting STATX_VERSION.
> 
> (3) FS_xxx_FL flags are returned as for ioctl(FS_IOC_GETFLAGS), setting
>     STATX_IOC_FLAGS.

I don't see where this is implemented in this patch, and it should be
removed from the commit message to avoid confusion.

> This requires that all ext4 inodes have a getattr call, not just some of
> them, so to this end, split the ext4_getattr() function and only call part
> of it where appropriate.
> 
> Example output:
> 
> 	[root@...romeda ~]# ./samples/statx/test-statx /usr
> 	statx(/usr) = 0
> 	results=37ef
> 	  Size: 4096            Blocks: 16         IO Block: 4096    directory
> 	Device: 08:02           Inode: 1572865     Links: 14
> 	Access: (0755/drwxr-xr-x)  Uid:     0   Gid:     0
> 	Access: 2015-11-03 16:12:30.000000000+0000
> 	Modify: 2013-10-18 15:29:18.000000000+0100
> 	Change: 2013-10-18 15:29:18.000000000+0100
> 	Data version: 2fh
> 	Inode flags: 00000000 (-------- -------- -------- --------)
> 	IO-blocksize: blksize=4096

One minor strangeness in this commit message is that the above usage example
doesn't include nanoseconds or Birth time for ext4, when this should be
supported for all ext4 filesystems?  Was this example run on an older ext3
filesystem (or some filesystem upgraded from ext3)?  Any ext4-formatted fs
should have the larger inode size to handle these extra fields.

Also a bit strange that "IO Block: 4096" and "IO-blocksize: blksize=4096"
is listed twice, but that seems to be an issue with the test-statx tool
and not necessarily the data being returned.

Cheers, Andreas

> 
> Signed-off-by: David Howells <dhowells@...hat.com>
> ---
> 
> fs/ext4/ext4.h    |    2 ++
> fs/ext4/file.c    |    2 +-
> fs/ext4/inode.c   |   30 +++++++++++++++++++++++++++---
> fs/ext4/namei.c   |    2 ++
> fs/ext4/symlink.c |    2 ++
> 5 files changed, 34 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 349afebe21ee..2f25eaa63f39 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -2549,6 +2549,8 @@ extern int  ext4_getattr(struct vfsmount *mnt, struct dentry *dentry,
> 				struct kstat *stat);
> extern void ext4_evict_inode(struct inode *);
> extern void ext4_clear_inode(struct inode *);
> +extern int  ext4_file_getattr(struct vfsmount *mnt, struct dentry *dentry,
> +			      struct kstat *stat);
> extern int  ext4_sync_inode(handle_t *, struct inode *);
> extern void ext4_dirty_inode(struct inode *, int);
> extern int ext4_change_inode_journal_flag(struct inode *, int);
> diff --git a/fs/ext4/file.c b/fs/ext4/file.c
> index fa2208bae2e1..45c7b8644d0e 100644
> --- a/fs/ext4/file.c
> +++ b/fs/ext4/file.c
> @@ -715,7 +715,7 @@ const struct file_operations ext4_file_operations = {
> 
> const struct inode_operations ext4_file_inode_operations = {
> 	.setattr	= ext4_setattr,
> -	.getattr	= ext4_getattr,
> +	.getattr	= ext4_file_getattr,
> 	.setxattr	= generic_setxattr,
> 	.getxattr	= generic_getxattr,
> 	.listxattr	= ext4_listxattr,
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 981a1fc30eaa..309b6cff8afc 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -5100,11 +5100,35 @@ err_out:
> int ext4_getattr(struct vfsmount *mnt, struct dentry *dentry,
> 		 struct kstat *stat)
> {
> -	struct inode *inode;
> -	unsigned long long delalloc_blocks;
> +	struct inode *inode = d_inode(dentry);
> +	struct ext4_inode *raw_inode;
> +	struct ext4_inode_info *ei = EXT4_I(inode);
> +
> +	stat->result_mask |= STATX_GEN;
> +	stat->gen = inode->i_generation;
> +
> +	if (EXT4_FITS_IN_INODE(raw_inode, ei, i_crtime)) {
> +		stat->result_mask |= STATX_BTIME;
> +		stat->btime.tv_sec = ei->i_crtime.tv_sec;
> +		stat->btime.tv_nsec = ei->i_crtime.tv_nsec;
> +	}
> +
> +	if (S_ISDIR(inode->i_mode) || IS_I_VERSION(inode)) {
> +		stat->result_mask |= STATX_VERSION;
> +		stat->version = inode->i_version;
> +	}
> 
> -	inode = d_inode(dentry);
> 	generic_fillattr(inode, stat);
> +	return 0;
> +}
> +
> +int ext4_file_getattr(struct vfsmount *mnt, struct dentry *dentry,
> +		      struct kstat *stat)
> +{
> +	struct inode *inode = dentry->d_inode;
> +	u64 delalloc_blocks;
> +
> +	ext4_getattr(mnt, dentry, stat);
> 
> 	/*
> 	 * If there is inline data in the inode, the inode will normally not
> diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
> index 48e4b8907826..9b5fdc971e22 100644
> --- a/fs/ext4/namei.c
> +++ b/fs/ext4/namei.c
> @@ -3882,6 +3882,7 @@ const struct inode_operations ext4_dir_inode_operations = {
> 	.tmpfile	= ext4_tmpfile,
> 	.rename2	= ext4_rename2,
> 	.setattr	= ext4_setattr,
> +	.getattr	= ext4_getattr,
> 	.setxattr	= generic_setxattr,
> 	.getxattr	= generic_getxattr,
> 	.listxattr	= ext4_listxattr,
> @@ -3893,6 +3894,7 @@ const struct inode_operations ext4_dir_inode_operations = {
> 
> const struct inode_operations ext4_special_inode_operations = {
> 	.setattr	= ext4_setattr,
> +	.getattr	= ext4_getattr,
> 	.setxattr	= generic_setxattr,
> 	.getxattr	= generic_getxattr,
> 	.listxattr	= ext4_listxattr,
> diff --git a/fs/ext4/symlink.c b/fs/ext4/symlink.c
> index 75ed5c2f0c16..54015f3d7516 100644
> --- a/fs/ext4/symlink.c
> +++ b/fs/ext4/symlink.c
> @@ -105,6 +105,7 @@ const struct inode_operations ext4_symlink_inode_operations = {
> 	.readlink	= generic_readlink,
> 	.get_link	= page_get_link,
> 	.setattr	= ext4_setattr,
> +	.getattr	= ext4_getattr,
> 	.setxattr	= generic_setxattr,
> 	.getxattr	= generic_getxattr,
> 	.listxattr	= ext4_listxattr,
> @@ -115,6 +116,7 @@ const struct inode_operations ext4_fast_symlink_inode_operations = {
> 	.readlink	= generic_readlink,
> 	.get_link	= simple_get_link,
> 	.setattr	= ext4_setattr,
> +	.getattr	= ext4_getattr,
> 	.setxattr	= generic_setxattr,
> 	.getxattr	= generic_getxattr,
> 	.listxattr	= ext4_listxattr,
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


Cheers, Andreas






Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ