[RFC PATCH] fs: obtain the inode generation number from vfs directly

[RFC PATCH] fs: obtain the inode generation number from vfs directly

[Hongbo Li]

Many mainstream file systems already support the GETVERSION ioctl,
and their implementations are completely the same, essentially
just obtain the value of i_generation. We think this ioctl can be
implemented at the VFS layer, so the file systems do not need to
implement it individually.

Signed-off-by: Hongbo Li <lihongbo22@huawei.com>
---
 fs/ioctl.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/fs/ioctl.c b/fs/ioctl.c
index 64776891120c..dff887ec52c4 100644
--- a/fs/ioctl.c
+++ b/fs/ioctl.c
@@ -878,6 +878,9 @@ static int do_vfs_ioctl(struct file *filp, unsigned int fd,
 	case FS_IOC_GETFSUUID:
 		return ioctl_getfsuuid(filp, argp);
 
+	case FS_IOC_GETVERSION:
+		return put_user(inode->i_generation, (int __user *)argp);
+
 	case FS_IOC_GETFSSYSFSPATH:
 		return ioctl_get_fs_sysfs_path(filp, argp);
 
@@ -992,6 +995,9 @@ COMPAT_SYSCALL_DEFINE3(ioctl, unsigned int, fd, unsigned int, cmd,
 		cmd = (cmd == FS_IOC32_GETFLAGS) ?
 			FS_IOC_GETFLAGS : FS_IOC_SETFLAGS;
 		fallthrough;
+	case FS_IOC32_GETVERSION:
+		cmd = FS_IOC_GETVERSION;
+		fallthrough;
 	/*
 	 * everything else in do_vfs_ioctl() takes either a compatible
 	 * pointer argument or no argument -- call it with a modified
-- 
2.34.1

Re: [RFC PATCH] fs: obtain the inode generation number from vfs directly

[Darrick J. Wong]

On Tue, Aug 27, 2024 at 01:41:08AM +0000, Hongbo Li wrote:
> Many mainstream file systems already support the GETVERSION ioctl,
> and their implementations are completely the same, essentially
> just obtain the value of i_generation. We think this ioctl can be
> implemented at the VFS layer, so the file systems do not need to
> implement it individually.

What if a filesystem never touches i_generation?  Is it ok to advertise
a generation number of zero when that's really meaningless?  Or should
we gate the generic ioctl on (say) whether or not the fs implements file
handles and/or supports nfs?

--D

> Signed-off-by: Hongbo Li <lihongbo22@huawei.com>
> ---
>  fs/ioctl.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/fs/ioctl.c b/fs/ioctl.c
> index 64776891120c..dff887ec52c4 100644
> --- a/fs/ioctl.c
> +++ b/fs/ioctl.c
> @@ -878,6 +878,9 @@ static int do_vfs_ioctl(struct file *filp, unsigned int fd,
>  	case FS_IOC_GETFSUUID:
>  		return ioctl_getfsuuid(filp, argp);
>  
> +	case FS_IOC_GETVERSION:
> +		return put_user(inode->i_generation, (int __user *)argp);
> +
>  	case FS_IOC_GETFSSYSFSPATH:
>  		return ioctl_get_fs_sysfs_path(filp, argp);
>  
> @@ -992,6 +995,9 @@ COMPAT_SYSCALL_DEFINE3(ioctl, unsigned int, fd, unsigned int, cmd,
>  		cmd = (cmd == FS_IOC32_GETFLAGS) ?
>  			FS_IOC_GETFLAGS : FS_IOC_SETFLAGS;
>  		fallthrough;
> +	case FS_IOC32_GETVERSION:
> +		cmd = FS_IOC_GETVERSION;
> +		fallthrough;
>  	/*
>  	 * everything else in do_vfs_ioctl() takes either a compatible
>  	 * pointer argument or no argument -- call it with a modified
> -- 
> 2.34.1
> 
> 

Re: [RFC PATCH] fs: obtain the inode generation number from vfs directly

[Hongbo Li]

On 2024/8/27 10:13, Darrick J. Wong wrote:
> On Tue, Aug 27, 2024 at 01:41:08AM +0000, Hongbo Li wrote:
>> Many mainstream file systems already support the GETVERSION ioctl,
>> and their implementations are completely the same, essentially
>> just obtain the value of i_generation. We think this ioctl can be
>> implemented at the VFS layer, so the file systems do not need to
>> implement it individually.
> 
> What if a filesystem never touches i_generation?  Is it ok to advertise
> a generation number of zero when that's really meaningless?  Or should
> we gate the generic ioctl on (say) whether or not the fs implements file
> handles and/or supports nfs?

This ioctl mainly returns the i_generation, and whether it has meaning 
is up to the specific file system. Some tools will invoke 
IOC_GETVERSION, such as `lsattr -v`(but if it's lattr, it won't), but 
users may not necessarily actually use this value.

Thanks,
Hongbo

> 
> --D
> 
>> Signed-off-by: Hongbo Li <lihongbo22@huawei.com>
>> ---
>>   fs/ioctl.c | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/fs/ioctl.c b/fs/ioctl.c
>> index 64776891120c..dff887ec52c4 100644
>> --- a/fs/ioctl.c
>> +++ b/fs/ioctl.c
>> @@ -878,6 +878,9 @@ static int do_vfs_ioctl(struct file *filp, unsigned int fd,
>>   	case FS_IOC_GETFSUUID:
>>   		return ioctl_getfsuuid(filp, argp);
>>   
>> +	case FS_IOC_GETVERSION:
>> +		return put_user(inode->i_generation, (int __user *)argp);
>> +
>>   	case FS_IOC_GETFSSYSFSPATH:
>>   		return ioctl_get_fs_sysfs_path(filp, argp);
>>   
>> @@ -992,6 +995,9 @@ COMPAT_SYSCALL_DEFINE3(ioctl, unsigned int, fd, unsigned int, cmd,
>>   		cmd = (cmd == FS_IOC32_GETFLAGS) ?
>>   			FS_IOC_GETFLAGS : FS_IOC_SETFLAGS;
>>   		fallthrough;
>> +	case FS_IOC32_GETVERSION:
>> +		cmd = FS_IOC_GETVERSION;
>> +		fallthrough;
>>   	/*
>>   	 * everything else in do_vfs_ioctl() takes either a compatible
>>   	 * pointer argument or no argument -- call it with a modified
>> -- 
>> 2.34.1
>>
>>
> 

Re: [RFC PATCH] fs: obtain the inode generation number from vfs directly

[Darrick J. Wong]

On Tue, Aug 27, 2024 at 10:32:38AM +0800, Hongbo Li wrote:
> 
> 
> On 2024/8/27 10:13, Darrick J. Wong wrote:
> > On Tue, Aug 27, 2024 at 01:41:08AM +0000, Hongbo Li wrote:
> > > Many mainstream file systems already support the GETVERSION ioctl,
> > > and their implementations are completely the same, essentially
> > > just obtain the value of i_generation. We think this ioctl can be
> > > implemented at the VFS layer, so the file systems do not need to
> > > implement it individually.
> > 
> > What if a filesystem never touches i_generation?  Is it ok to advertise
> > a generation number of zero when that's really meaningless?  Or should
> > we gate the generic ioctl on (say) whether or not the fs implements file
> > handles and/or supports nfs?
> 
> This ioctl mainly returns the i_generation, and whether it has meaning is up
> to the specific file system. Some tools will invoke IOC_GETVERSION, such as
> `lsattr -v`(but if it's lattr, it won't), but users may not necessarily
> actually use this value.

That's not how that works.  If the kernel starts exporting a datum,
people will start using it, and then the expectation that it will
*continue* to work becomes ingrained in the userspace ABI forever.
Be careful about establishing new behaviors for vfat.

--D

> Thanks,
> Hongbo
> 
> > 
> > --D
> > 
> > > Signed-off-by: Hongbo Li <lihongbo22@huawei.com>
> > > ---
> > >   fs/ioctl.c | 6 ++++++
> > >   1 file changed, 6 insertions(+)
> > > 
> > > diff --git a/fs/ioctl.c b/fs/ioctl.c
> > > index 64776891120c..dff887ec52c4 100644
> > > --- a/fs/ioctl.c
> > > +++ b/fs/ioctl.c
> > > @@ -878,6 +878,9 @@ static int do_vfs_ioctl(struct file *filp, unsigned int fd,
> > >   	case FS_IOC_GETFSUUID:
> > >   		return ioctl_getfsuuid(filp, argp);
> > > +	case FS_IOC_GETVERSION:
> > > +		return put_user(inode->i_generation, (int __user *)argp);
> > > +
> > >   	case FS_IOC_GETFSSYSFSPATH:
> > >   		return ioctl_get_fs_sysfs_path(filp, argp);
> > > @@ -992,6 +995,9 @@ COMPAT_SYSCALL_DEFINE3(ioctl, unsigned int, fd, unsigned int, cmd,
> > >   		cmd = (cmd == FS_IOC32_GETFLAGS) ?
> > >   			FS_IOC_GETFLAGS : FS_IOC_SETFLAGS;
> > >   		fallthrough;
> > > +	case FS_IOC32_GETVERSION:
> > > +		cmd = FS_IOC_GETVERSION;
> > > +		fallthrough;
> > >   	/*
> > >   	 * everything else in do_vfs_ioctl() takes either a compatible
> > >   	 * pointer argument or no argument -- call it with a modified
> > > -- 
> > > 2.34.1
> > > 
> > > 
> > 
> 

Re: [RFC PATCH] fs: obtain the inode generation number from vfs directly

[Christian Brauner]

On Mon, Aug 26, 2024 at 10:37:12PM GMT, Darrick J. Wong wrote:
> On Tue, Aug 27, 2024 at 10:32:38AM +0800, Hongbo Li wrote:
> > 
> > 
> > On 2024/8/27 10:13, Darrick J. Wong wrote:
> > > On Tue, Aug 27, 2024 at 01:41:08AM +0000, Hongbo Li wrote:
> > > > Many mainstream file systems already support the GETVERSION ioctl,
> > > > and their implementations are completely the same, essentially
> > > > just obtain the value of i_generation. We think this ioctl can be
> > > > implemented at the VFS layer, so the file systems do not need to
> > > > implement it individually.
> > > 
> > > What if a filesystem never touches i_generation?  Is it ok to advertise
> > > a generation number of zero when that's really meaningless?  Or should
> > > we gate the generic ioctl on (say) whether or not the fs implements file
> > > handles and/or supports nfs?
> > 
> > This ioctl mainly returns the i_generation, and whether it has meaning is up
> > to the specific file system. Some tools will invoke IOC_GETVERSION, such as
> > `lsattr -v`(but if it's lattr, it won't), but users may not necessarily
> > actually use this value.
> 
> That's not how that works.  If the kernel starts exporting a datum,
> people will start using it, and then the expectation that it will
> *continue* to work becomes ingrained in the userspace ABI forever.
> Be careful about establishing new behaviors for vfat.

Is the meaning even the same across all filesystems? And what is the
meaning of this anyway? Is this described/defined for userspace
anywhere?

Re: [RFC PATCH] fs: obtain the inode generation number from vfs directly

[Darrick J. Wong]

On Tue, Aug 27, 2024 at 11:22:17AM +0200, Christian Brauner wrote:
> On Mon, Aug 26, 2024 at 10:37:12PM GMT, Darrick J. Wong wrote:
> > On Tue, Aug 27, 2024 at 10:32:38AM +0800, Hongbo Li wrote:
> > > 
> > > 
> > > On 2024/8/27 10:13, Darrick J. Wong wrote:
> > > > On Tue, Aug 27, 2024 at 01:41:08AM +0000, Hongbo Li wrote:
> > > > > Many mainstream file systems already support the GETVERSION ioctl,
> > > > > and their implementations are completely the same, essentially
> > > > > just obtain the value of i_generation. We think this ioctl can be
> > > > > implemented at the VFS layer, so the file systems do not need to
> > > > > implement it individually.
> > > > 
> > > > What if a filesystem never touches i_generation?  Is it ok to advertise
> > > > a generation number of zero when that's really meaningless?  Or should
> > > > we gate the generic ioctl on (say) whether or not the fs implements file
> > > > handles and/or supports nfs?
> > > 
> > > This ioctl mainly returns the i_generation, and whether it has meaning is up
> > > to the specific file system. Some tools will invoke IOC_GETVERSION, such as
> > > `lsattr -v`(but if it's lattr, it won't), but users may not necessarily
> > > actually use this value.
> > 
> > That's not how that works.  If the kernel starts exporting a datum,
> > people will start using it, and then the expectation that it will
> > *continue* to work becomes ingrained in the userspace ABI forever.
> > Be careful about establishing new behaviors for vfat.
> 
> Is the meaning even the same across all filesystems? And what is the
> meaning of this anyway? Is this described/defined for userspace
> anywhere?

AFAICT there's no manpage so I guess we could return getrandom32() if we
wanted to. ;)

But in seriousness, the usual four filesystems return i_generation.
That is changed every time an inumber gets reused so that anyone with an
old file handle cannot accidentally open the wrong file.  In theory one
could use GETVERSION to construct file handles (if you do, UHLHAND!)
instead of using name_to_handle_at, which is why it's dangerous to
implement GETVERSION for everyone without checking if i_generation makes
sense.

--D

Re: [RFC PATCH] fs: obtain the inode generation number from vfs directly

[Hongbo Li]

On 2024/8/28 1:11, Darrick J. Wong wrote:
> On Tue, Aug 27, 2024 at 11:22:17AM +0200, Christian Brauner wrote:
>> On Mon, Aug 26, 2024 at 10:37:12PM GMT, Darrick J. Wong wrote:
>>> On Tue, Aug 27, 2024 at 10:32:38AM +0800, Hongbo Li wrote:
>>>>
>>>>
>>>> On 2024/8/27 10:13, Darrick J. Wong wrote:
>>>>> On Tue, Aug 27, 2024 at 01:41:08AM +0000, Hongbo Li wrote:
>>>>>> Many mainstream file systems already support the GETVERSION ioctl,
>>>>>> and their implementations are completely the same, essentially
>>>>>> just obtain the value of i_generation. We think this ioctl can be
>>>>>> implemented at the VFS layer, so the file systems do not need to
>>>>>> implement it individually.
>>>>>
>>>>> What if a filesystem never touches i_generation?  Is it ok to advertise
>>>>> a generation number of zero when that's really meaningless?  Or should
>>>>> we gate the generic ioctl on (say) whether or not the fs implements file
>>>>> handles and/or supports nfs?
>>>>
>>>> This ioctl mainly returns the i_generation, and whether it has meaning is up
>>>> to the specific file system. Some tools will invoke IOC_GETVERSION, such as
>>>> `lsattr -v`(but if it's lattr, it won't), but users may not necessarily
>>>> actually use this value.
>>>
>>> That's not how that works.  If the kernel starts exporting a datum,
>>> people will start using it, and then the expectation that it will
>>> *continue* to work becomes ingrained in the userspace ABI forever.
>>> Be careful about establishing new behaviors for vfat.
>>
>> Is the meaning even the same across all filesystems? And what is the
>> meaning of this anyway? Is this described/defined for userspace
>> anywhere?
> 
> AFAICT there's no manpage so I guess we could return getrandom32() if we
> wanted to. ;)
> 
> But in seriousness, the usual four filesystems return i_generation.
> That is changed every time an inumber gets reused so that anyone with an
> old file handle cannot accidentally open the wrong file.  In theory one
> could use GETVERSION to construct file handles (if you do, UHLHAND!)
> instead of using name_to_handle_at, which is why it's dangerous to
> implement GETVERSION for everyone without checking if i_generation makes
> sense.

I'm not sure if my understanding of you is correct. As my humble 
opinions, the ioctl is for returning information to the user, and it 
cannot rely on this information returned to the user to ensure the 
security. If the file system wants to be secure internally, it should 
decouple from the VFS layer interface, rather than just not implementing it.
For NFS, constructing a file handle is easy, you can successfully 
construct a file handle by capturing the nfs protocol packet even 
thought the i_generation is not exposed.

Thanks,
Hongbo
> 
> --D

Re: [RFC PATCH] fs: obtain the inode generation number from vfs directly

[Theodore Ts'o]

On Tue, Aug 27, 2024 at 10:11:48AM -0700, Darrick J. Wong wrote:
> 
> But in seriousness, the usual four filesystems return i_generation.
> That is changed every time an inumber gets reused so that anyone with an
> old file handle cannot accidentally open the wrong file.  In theory one
> could use GETVERSION to construct file handles (if you do, UHLHAND!)
> instead of using name_to_handle_at, which is why it's dangerous to
> implement GETVERSION for everyone without checking if i_generation makes
> sense.

I believe the primary use case for {FS,EXT4}_IOC_GETVERSION was for
userspace NFS servers to construt file handles.

For file systems that don't store persistent i_generation numbers, I
think it would be absolutely wrong that FS_IOC_GETVERSION to return
zero, or some nonsense random number.  The right thing to do would be
to have it return an ENOTTY error if somene tries to call
FS_IOC_GETVERSION on a vfat file system.  Otherwise this could lead to
potential user data loss/corruption for users of userspace nfs servers.

	       	    		    	      	 - Ted

Re: [RFC PATCH] fs: obtain the inode generation number from vfs directly

[Dave Chinner]

On Tue, Aug 27, 2024 at 10:11:48AM -0700, Darrick J. Wong wrote:
> On Tue, Aug 27, 2024 at 11:22:17AM +0200, Christian Brauner wrote:
> > On Mon, Aug 26, 2024 at 10:37:12PM GMT, Darrick J. Wong wrote:
> > > On Tue, Aug 27, 2024 at 10:32:38AM +0800, Hongbo Li wrote:
> > > > 
> > > > 
> > > > On 2024/8/27 10:13, Darrick J. Wong wrote:
> > > > > On Tue, Aug 27, 2024 at 01:41:08AM +0000, Hongbo Li wrote:
> > > > > > Many mainstream file systems already support the GETVERSION ioctl,
> > > > > > and their implementations are completely the same, essentially
> > > > > > just obtain the value of i_generation. We think this ioctl can be
> > > > > > implemented at the VFS layer, so the file systems do not need to
> > > > > > implement it individually.
> > > > > 
> > > > > What if a filesystem never touches i_generation?  Is it ok to advertise
> > > > > a generation number of zero when that's really meaningless?  Or should
> > > > > we gate the generic ioctl on (say) whether or not the fs implements file
> > > > > handles and/or supports nfs?
> > > > 
> > > > This ioctl mainly returns the i_generation, and whether it has meaning is up
> > > > to the specific file system. Some tools will invoke IOC_GETVERSION, such as
> > > > `lsattr -v`(but if it's lattr, it won't), but users may not necessarily
> > > > actually use this value.
> > > 
> > > That's not how that works.  If the kernel starts exporting a datum,
> > > people will start using it, and then the expectation that it will
> > > *continue* to work becomes ingrained in the userspace ABI forever.
> > > Be careful about establishing new behaviors for vfat.
> > 
> > Is the meaning even the same across all filesystems? And what is the
> > meaning of this anyway? Is this described/defined for userspace
> > anywhere?
> 
> AFAICT there's no manpage so I guess we could return getrandom32() if we
> wanted to. ;)
> 
> But in seriousness, the usual four filesystems return i_generation.

We do? 

I thought we didn't expose it except via bulkstat (which requires
CAP_SYS_ADMIN in the initns).

/me goes looking

Ugh. Well, there you go. I've been living a lie for 20 years.

> That is changed every time an inumber gets reused so that anyone with an
> old file handle cannot accidentally open the wrong file.  In theory one
> could use GETVERSION to construct file handles

Not theory. We've been constructing XFS filehandles in -privileged-
userspace applications since the late 90s. Both DMAPI applications
(HSMs) and xfsdump do this in combination with bulkstat to retreive
the generation to enable full filesystem access without directory
traversal being necessary.

I was completely unaware that FS_IOC_GETVERSION was implemented by
XFS and so this information is available to unprivileged users...

> (if you do, UHLHAND!)

Not familiar with that acronym.

> instead of using name_to_handle_at, which is why it's dangerous to
> implement GETVERSION for everyone without checking if i_generation makes
> sense.

Yup. If you have predictable generation numbers then it's trivial to
guess filehandles once you know the inode number. Exposing
generation numbers to unprivileged users allows them to determine if
the generation numbers are predictable. Determining patterns is
often as simple as a loop doing open(create); get inode number +
generation; unlink().

That's one of the reasons we moved to randomised base generation
numbers in XFS back in 2008 - making NFS filehandles on XFS
filesystems less predictable and less prone to collisions.  Given
that we are actually exposing them to unprivileged users, we
probably also need to get rid of the monotonic increment when the
inode is freed and randomise that as well.

And now I see EXT4_IOC_SETVERSION, and I want to run screaming.....

-Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [RFC PATCH] fs: obtain the inode generation number from vfs directly

[Jan Kara]

On Wed 28-08-24 15:38:49, Dave Chinner wrote:
> On Tue, Aug 27, 2024 at 10:11:48AM -0700, Darrick J. Wong wrote:
> > On Tue, Aug 27, 2024 at 11:22:17AM +0200, Christian Brauner wrote:
> > > On Mon, Aug 26, 2024 at 10:37:12PM GMT, Darrick J. Wong wrote:
> > > > On Tue, Aug 27, 2024 at 10:32:38AM +0800, Hongbo Li wrote:
> > > > > 
> > > > > 
> > > > > On 2024/8/27 10:13, Darrick J. Wong wrote:
> > > > > > On Tue, Aug 27, 2024 at 01:41:08AM +0000, Hongbo Li wrote:
> > > > > > > Many mainstream file systems already support the GETVERSION ioctl,
> > > > > > > and their implementations are completely the same, essentially
> > > > > > > just obtain the value of i_generation. We think this ioctl can be
> > > > > > > implemented at the VFS layer, so the file systems do not need to
> > > > > > > implement it individually.
> > > > > > 
> > > > > > What if a filesystem never touches i_generation?  Is it ok to advertise
> > > > > > a generation number of zero when that's really meaningless?  Or should
> > > > > > we gate the generic ioctl on (say) whether or not the fs implements file
> > > > > > handles and/or supports nfs?
> > > > > 
> > > > > This ioctl mainly returns the i_generation, and whether it has meaning is up
> > > > > to the specific file system. Some tools will invoke IOC_GETVERSION, such as
> > > > > `lsattr -v`(but if it's lattr, it won't), but users may not necessarily
> > > > > actually use this value.
> > > > 
> > > > That's not how that works.  If the kernel starts exporting a datum,
> > > > people will start using it, and then the expectation that it will
> > > > *continue* to work becomes ingrained in the userspace ABI forever.
> > > > Be careful about establishing new behaviors for vfat.
> > > 
> > > Is the meaning even the same across all filesystems? And what is the
> > > meaning of this anyway? Is this described/defined for userspace
> > > anywhere?
> > 
> > AFAICT there's no manpage so I guess we could return getrandom32() if we
> > wanted to. ;)
> > 
> > But in seriousness, the usual four filesystems return i_generation.
> 
> We do? 
> 
> I thought we didn't expose it except via bulkstat (which requires
> CAP_SYS_ADMIN in the initns).
> 
> /me goes looking
> 
> Ugh. Well, there you go. I've been living a lie for 20 years.
> 
> > That is changed every time an inumber gets reused so that anyone with an
> > old file handle cannot accidentally open the wrong file.  In theory one
> > could use GETVERSION to construct file handles
> 
> Not theory. We've been constructing XFS filehandles in -privileged-
> userspace applications since the late 90s. Both DMAPI applications
> (HSMs) and xfsdump do this in combination with bulkstat to retreive
> the generation to enable full filesystem access without directory
> traversal being necessary.
> 
> I was completely unaware that FS_IOC_GETVERSION was implemented by
> XFS and so this information is available to unprivileged users...
> 
> > (if you do, UHLHAND!)
> 
> Not familiar with that acronym.
> 
> > instead of using name_to_handle_at, which is why it's dangerous to
> > implement GETVERSION for everyone without checking if i_generation makes
> > sense.
> 
> Yup. If you have predictable generation numbers then it's trivial to
> guess filehandles once you know the inode number. Exposing
> generation numbers to unprivileged users allows them to determine if
> the generation numbers are predictable. Determining patterns is
> often as simple as a loop doing open(create); get inode number +
> generation; unlink().

As far as VFS goes, we have always assumed that a valid file handles can be
easily forged by unpriviledged userspace and hence all syscalls taking file
handle are gated by CAP_DAC_READ_SEARCH capability check. That means
userspace can indeed create a valid file handle but unless the process has
sufficient priviledges to crawl the whole filesystem, VFS will not allow it
to do anything special with it.

I don't know what XFS interfaces use file handles and what are the
permission requirements there but effectively relying on a 32-bit cookie
value for security seems like a rather weak security these days to me...

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [RFC PATCH] fs: obtain the inode generation number from vfs directly

[Darrick J. Wong]

On Wed, Aug 28, 2024 at 05:55:28PM +0200, Jan Kara wrote:
> On Wed 28-08-24 15:38:49, Dave Chinner wrote:
> > On Tue, Aug 27, 2024 at 10:11:48AM -0700, Darrick J. Wong wrote:
> > > On Tue, Aug 27, 2024 at 11:22:17AM +0200, Christian Brauner wrote:
> > > > On Mon, Aug 26, 2024 at 10:37:12PM GMT, Darrick J. Wong wrote:
> > > > > On Tue, Aug 27, 2024 at 10:32:38AM +0800, Hongbo Li wrote:
> > > > > > 
> > > > > > 
> > > > > > On 2024/8/27 10:13, Darrick J. Wong wrote:
> > > > > > > On Tue, Aug 27, 2024 at 01:41:08AM +0000, Hongbo Li wrote:
> > > > > > > > Many mainstream file systems already support the GETVERSION ioctl,
> > > > > > > > and their implementations are completely the same, essentially
> > > > > > > > just obtain the value of i_generation. We think this ioctl can be
> > > > > > > > implemented at the VFS layer, so the file systems do not need to
> > > > > > > > implement it individually.
> > > > > > > 
> > > > > > > What if a filesystem never touches i_generation?  Is it ok to advertise
> > > > > > > a generation number of zero when that's really meaningless?  Or should
> > > > > > > we gate the generic ioctl on (say) whether or not the fs implements file
> > > > > > > handles and/or supports nfs?
> > > > > > 
> > > > > > This ioctl mainly returns the i_generation, and whether it has meaning is up
> > > > > > to the specific file system. Some tools will invoke IOC_GETVERSION, such as
> > > > > > `lsattr -v`(but if it's lattr, it won't), but users may not necessarily
> > > > > > actually use this value.
> > > > > 
> > > > > That's not how that works.  If the kernel starts exporting a datum,
> > > > > people will start using it, and then the expectation that it will
> > > > > *continue* to work becomes ingrained in the userspace ABI forever.
> > > > > Be careful about establishing new behaviors for vfat.
> > > > 
> > > > Is the meaning even the same across all filesystems? And what is the
> > > > meaning of this anyway? Is this described/defined for userspace
> > > > anywhere?
> > > 
> > > AFAICT there's no manpage so I guess we could return getrandom32() if we
> > > wanted to. ;)
> > > 
> > > But in seriousness, the usual four filesystems return i_generation.
> > 
> > We do? 
> > 
> > I thought we didn't expose it except via bulkstat (which requires
> > CAP_SYS_ADMIN in the initns).
> > 
> > /me goes looking
> > 
> > Ugh. Well, there you go. I've been living a lie for 20 years.
> > 
> > > That is changed every time an inumber gets reused so that anyone with an
> > > old file handle cannot accidentally open the wrong file.  In theory one
> > > could use GETVERSION to construct file handles
> > 
> > Not theory. We've been constructing XFS filehandles in -privileged-
> > userspace applications since the late 90s. Both DMAPI applications
> > (HSMs) and xfsdump do this in combination with bulkstat to retreive
> > the generation to enable full filesystem access without directory
> > traversal being necessary.
> > 
> > I was completely unaware that FS_IOC_GETVERSION was implemented by
> > XFS and so this information is available to unprivileged users...
> > 
> > > (if you do, UHLHAND!)
> > Not familiar with that acronym.

U Have Lost, Have A Nice Day!

> > 
> > > instead of using name_to_handle_at, which is why it's dangerous to
> > > implement GETVERSION for everyone without checking if i_generation makes
> > > sense.
> > 
> > Yup. If you have predictable generation numbers then it's trivial to
> > guess filehandles once you know the inode number. Exposing
> > generation numbers to unprivileged users allows them to determine if
> > the generation numbers are predictable. Determining patterns is
> > often as simple as a loop doing open(create); get inode number +
> > generation; unlink().
> 
> As far as VFS goes, we have always assumed that a valid file handles can be
> easily forged by unpriviledged userspace and hence all syscalls taking file
> handle are gated by CAP_DAC_READ_SEARCH capability check. That means
> userspace can indeed create a valid file handle but unless the process has
> sufficient priviledges to crawl the whole filesystem, VFS will not allow it
> to do anything special with it.
> 
> I don't know what XFS interfaces use file handles and what are the
> permission requirements there but effectively relying on a 32-bit cookie
> value for security seems like a rather weak security these days to me...

CAP_SYS_ADMIN.

--D

> 								Honza
> -- 
> Jan Kara <jack@suse.com>
> SUSE Labs, CR
> 

Re: [RFC PATCH] fs: obtain the inode generation number from vfs directly

[Dave Chinner]

On Wed, Aug 28, 2024 at 05:55:28PM +0200, Jan Kara wrote:
> On Wed 28-08-24 15:38:49, Dave Chinner wrote:
> > > instead of using name_to_handle_at, which is why it's dangerous to
> > > implement GETVERSION for everyone without checking if i_generation makes
> > > sense.
> > 
> > Yup. If you have predictable generation numbers then it's trivial to
> > guess filehandles once you know the inode number. Exposing
> > generation numbers to unprivileged users allows them to determine if
> > the generation numbers are predictable. Determining patterns is
> > often as simple as a loop doing open(create); get inode number +
> > generation; unlink().
> 
> As far as VFS goes, we have always assumed that a valid file handles can be
> easily forged by unpriviledged userspace and hence all syscalls taking file
> handle are gated by CAP_DAC_READ_SEARCH capability check. That means
> userspace can indeed create a valid file handle but unless the process has
> sufficient priviledges to crawl the whole filesystem, VFS will not allow it
> to do anything special with it.

Yup.

The issue that was raised back in ~2008 was to do with rogue
machines on the network being able to trivially spoof NFS
filehandles to bypass directory access permission checks on the
server side. Once the generation numbers are randomised, this sort
of attack is easily detected as the ESTALE counter on the server
side goes through the roof...

> I don't know what XFS interfaces use file handles and what are the
> permission requirements there but effectively relying on a 32-bit cookie
> value for security seems like a rather weak security these days to me...

It's always been CAP_SYS_ADMIN for local filehandle interfaces.

-Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [RFC PATCH] fs: obtain the inode generation number from vfs directly

[Matthew Wilcox]

On Tue, Aug 27, 2024 at 01:41:08AM +0000, Hongbo Li wrote:
> Many mainstream file systems already support the GETVERSION ioctl,
> and their implementations are completely the same, essentially
> just obtain the value of i_generation. We think this ioctl can be
> implemented at the VFS layer, so the file systems do not need to
> implement it individually.

... then you should also remove the implementation from every
filesystem, not just add it to the VFS.

Re: [RFC PATCH] fs: obtain the inode generation number from vfs directly

[Hongbo Li]

On 2024/8/27 10:53, Matthew Wilcox wrote:
> On Tue, Aug 27, 2024 at 01:41:08AM +0000, Hongbo Li wrote:
>> Many mainstream file systems already support the GETVERSION ioctl,
>> and their implementations are completely the same, essentially
>> just obtain the value of i_generation. We think this ioctl can be
>> implemented at the VFS layer, so the file systems do not need to
>> implement it individually.
> 
> ... then you should also remove the implementation from every
> filesystem, not just add it to the VFS.
> 

Yeah, this is just an RFC submission, mainly to see what everyone's 
opinions are. If this is ok, I will send the v2 that includes the 
removal of all file systems' implementations on IOC_GETVERSION.

Thanks,
Hongbo

> 

Re: [RFC PATCH] fs: obtain the inode generation number from vfs directly

[Dave Chinner]

On Tue, Aug 27, 2024 at 01:41:08AM +0000, Hongbo Li wrote:
> Many mainstream file systems already support the GETVERSION ioctl,
> and their implementations are completely the same, essentially
> just obtain the value of i_generation. We think this ioctl can be
> implemented at the VFS layer, so the file systems do not need to
> implement it individually.
> 
> Signed-off-by: Hongbo Li <lihongbo22@huawei.com>

Nope. No. Definitely not.

I definitely do not want unprivileged users to be able to construct
arbitrary file handles in userspace.....

-Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [RFC PATCH] fs: obtain the inode generation number from vfs directly

[Tavian Barnes]

On Tue, 27 Aug 2024 01:41:08 +0000, Hongbo Li wrote:
> Many mainstream file systems already support the GETVERSION ioctl,
> and their implementations are completely the same, essentially
> just obtain the value of i_generation. We think this ioctl can be
> implemented at the VFS layer, so the file systems do not need to
> implement it individually.
>
> Signed-off-by: Hongbo Li <lihongbo22@huawei.com>
> ---
>  fs/ioctl.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/fs/ioctl.c b/fs/ioctl.c
> index 64776891120c..dff887ec52c4 100644
> --- a/fs/ioctl.c
> +++ b/fs/ioctl.c
> @@ -878,6 +878,9 @@ static int do_vfs_ioctl(struct file *filp, unsigned int fd,
>  	case FS_IOC_GETFSUUID:
>  		return ioctl_getfsuuid(filp, argp);
>  
> +	case FS_IOC_GETVERSION:
> +		return put_user(inode->i_generation, (int __user *)argp);
> +
>  	case FS_IOC_GETFSSYSFSPATH:
>  		return ioctl_get_fs_sysfs_path(filp, argp);
>  
> @@ -992,6 +995,9 @@ COMPAT_SYSCALL_DEFINE3(ioctl, unsigned int, fd, unsigned int, cmd,
>  		cmd = (cmd == FS_IOC32_GETFLAGS) ?
>  			FS_IOC_GETFLAGS : FS_IOC_SETFLAGS;
>  		fallthrough;

The above case falls through...

> +	case FS_IOC32_GETVERSION:
> +		cmd = FS_IOC_GETVERSION;

... to this new case, which clobbers cmd, breaking FS_IOC32_[GS]ETFLAGS.

> +		fallthrough;
>  	/*
>  	 * everything else in do_vfs_ioctl() takes either a compatible
>  	 * pointer argument or no argument -- call it with a modified