Re: [PATCH 05/19] vfs: Introduce a non-repeating system-unique superblock ID [ver #16]

From: "Darrick J. Wong" <darrick.wong@oracle.com>
To: David Howells <dhowells@redhat.com>
Cc: viro@zeniv.linux.org.uk, raven@themaw.net, mszeredi@redhat.com,
	christian@brauner.io, linux-api@vger.kernel.org,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 05/19] vfs: Introduce a non-repeating system-unique superblock ID [ver #16]
Date: Wed, 19 Feb 2020 08:53:12 -0800	[thread overview]
Message-ID: <20200219165312.GD9504@magnolia> (raw)
In-Reply-To: <158204553565.3299825.3864357054582488949.stgit@warthog.procyon.org.uk>

On Tue, Feb 18, 2020 at 05:05:35PM +0000, David Howells wrote:
> Introduce an (effectively) non-repeating system-unique superblock ID that
> can be used to determine that two object are in the same superblock without
> risking reuse of the ID in the meantime (as is possible with device IDs).
> 
> The ID is time-based to make it harder to use it as a covert communications
> channel.
> 
> Also make it so that this ID can be fetched by the fsinfo() system call.
> The ID added so that superblock notification messages will also be able to
> be tagged with it.
> 
> Signed-off-by: David Howells <dhowells@redhat.com>
> ---
> 
>  fs/fsinfo.c               |    1 +
>  fs/super.c                |   24 ++++++++++++++++++++++++
>  include/linux/fs.h        |    3 +++
>  samples/vfs/test-fsinfo.c |    1 +
>  4 files changed, 29 insertions(+)
> 
> diff --git a/fs/fsinfo.c b/fs/fsinfo.c
> index 55710d6da327..f8e85762fc47 100644
> --- a/fs/fsinfo.c
> +++ b/fs/fsinfo.c
> @@ -92,6 +92,7 @@ static int fsinfo_generic_ids(struct path *path, struct fsinfo_context *ctx)
>  	p->f_fstype	= sb->s_magic;
>  	p->f_dev_major	= MAJOR(sb->s_dev);
>  	p->f_dev_minor	= MINOR(sb->s_dev);
> +	p->f_sb_id	= sb->s_unique_id;

Ahah, this is what the f_sb_id field is for.  I noticed a few patches
ago that it was in a header file but was never set.

I'm losing track of which IDs do what...

* f_fsid is that old int[2] thing that we used for statfs.  It sucks but
  we can't remove it because it's been in statfs since the beginning of
  time.

* f_fs_name is a string coming from s_type, which is the name of the fs
  (e.g. "XFS")?

* f_fstype comes from s_magic, which (for XFS) is 0x58465342.

* f_sb_id is basically an incore u64 cookie that one can use with the
  mount events thing that comes later in this patchset?

* FSINFO_ATTR_VOLUME_ID comes from s_id, which tends to be the block
  device name (at least for local filesystems)

* FSINFO_ATTR_VOLUME_UUID comes from s_uuid, which some filesystems fill
  in at mount time.

* FSINFO_ATTR_VOLUME_NAME is ... left to individual filesystems to
  implement, and (AFAICT) can be the label that one uses for things
  like: "mount LABEL=foo /home" ?

Assuming I got all of that right, can we please capture what all of
these "IDs" mean in the documentation?

(Assuming I got all that right, the code looks ok.)

--D

>  
>  	memcpy(&p->f_fsid, &buf.f_fsid, sizeof(p->f_fsid));
>  	strlcpy(p->f_fs_name, path->dentry->d_sb->s_type->name,
> diff --git a/fs/super.c b/fs/super.c
> index cd352530eca9..a63073e6127e 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -44,6 +44,8 @@ static int thaw_super_locked(struct super_block *sb);
>  
>  static LIST_HEAD(super_blocks);
>  static DEFINE_SPINLOCK(sb_lock);
> +static u64 sb_last_identifier;
> +static u64 sb_identifier_offset;
>  
>  static char *sb_writers_name[SB_FREEZE_LEVELS] = {
>  	"sb_writers",
> @@ -188,6 +190,27 @@ static void destroy_unused_super(struct super_block *s)
>  	destroy_super_work(&s->destroy_work);
>  }
>  
> +/*
> + * Generate a unique identifier for a superblock.
> + */
> +static void generate_super_id(struct super_block *s)
> +{
> +	u64 id = ktime_to_ns(ktime_get());
> +
> +	spin_lock(&sb_lock);
> +
> +	id += sb_identifier_offset;
> +	if (id <= sb_last_identifier) {
> +		id = sb_last_identifier + 1;
> +		sb_identifier_offset = sb_last_identifier - id;
> +	}
> +
> +	sb_last_identifier = id;
> +	spin_unlock(&sb_lock);
> +
> +	s->s_unique_id = id;
> +}
> +
>  /**
>   *	alloc_super	-	create new superblock
>   *	@type:	filesystem type superblock should belong to
> @@ -273,6 +296,7 @@ static struct super_block *alloc_super(struct file_system_type *type, int flags,
>  		goto fail;
>  	if (list_lru_init_memcg(&s->s_inode_lru, &s->s_shrink))
>  		goto fail;
> +	generate_super_id(s);
>  	return s;
>  
>  fail:
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index f74a4ee36eb3..e5db22d536a3 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1550,6 +1550,9 @@ struct super_block {
>  
>  	spinlock_t		s_inode_wblist_lock;
>  	struct list_head	s_inodes_wb;	/* writeback inodes */
> +
> +	/* Superblock event notifications */
> +	u64			s_unique_id;
>  } __randomize_layout;
>  
>  /* Helper functions so that in most cases filesystems will
> diff --git a/samples/vfs/test-fsinfo.c b/samples/vfs/test-fsinfo.c
> index 6fbf0ce099b2..d6ec5713364f 100644
> --- a/samples/vfs/test-fsinfo.c
> +++ b/samples/vfs/test-fsinfo.c
> @@ -140,6 +140,7 @@ static void dump_fsinfo_generic_ids(void *reply, unsigned int size)
>  	printf("\tdev          : %02x:%02x\n", f->f_dev_major, f->f_dev_minor);
>  	printf("\tfs           : type=%x name=%s\n", f->f_fstype, f->f_fs_name);
>  	printf("\tfsid         : %llx\n", (unsigned long long)f->f_fsid);
> +	printf("\tsbid         : %llx\n", (unsigned long long)f->f_sb_id);
>  }
>  
>  static void dump_fsinfo_generic_limits(void *reply, unsigned int size)
> 
> 