Re: [PATCH RFC DRAFT v2 00/13] Move fscrypt and fsverity out of struct inode

Christian Brauner
2025-07-22T13:09:08Z

On Tue, Jul 22, 2025 at 02:57:06PM +0200, Christian Brauner wrote:
> Hey,
>
> This is a POC. We're still discussing alternatives and I want to provide
> some useful data on what I learned about using offsets to drop fscrypt
> and fsverity from struct inode.
>
> As discussed, this moves the fscrypt and fsverity pointers out of struct
> inode shrinking it by 16 bytes. The pointers move into the individual
> filesystems that actually do make use of them.
>
> In order to find the fscrypt and fsverity data pointers offsets from the
> embedded struct inode in the filesystem's private inode data are
> stored in struct inode_operations. This means we get fast access to the
> data pointers without having to rely on indirect calls.
>
> Bugs & Issues
> =============
>
> * For fscrypt specifically the biggest issue is
> fscrypt_prepare_new_inode() is called in filesystem's inode allocation
> functions before inode->i_op is set. That means the offset isn't
> available at the time when we would need it. To fix this we can set
> dummy encrypted inode operations for the respective filesystem with an
> initialized offset.
>
> * For both fscrypt & fsverity the biggest issue is that every codepath
> that currently calls make_bad_inode() after having initialized fscrypt
> or fsverity data will override inode->i_op with bad_inode_ops. At
> which point we're back to the previous problem: The offset isn't
> available anymore. So when inode->i_sb->s_op->evict_inode() is called
> fscrypt_put_encryption_info() doesn't have the offset available
> anymore and would corrupt the hell out of everything and also leak
> memory.
>
> Obviously we could use a flag to detect a bad inodes instead of i_op
> and let the filesystem assign it's own bad inode operations including
> the correct offset. Is it worth it?
>
> The other way I see we can fix this if we require fixed offsets in the
> filesystems inode so fscrypt and fsverity always now what offset to
> calculate. We could use two consecutive pointers at the beginning of
> the filesystem's inode. Does that always work and is it worth it?

Another, way less idiotic but way more obvious solution is to move the
offsets to struct super_operations. That will mean one additional
pointer deref but it will have the big advantage that the patch will
become really really simple. Thoughts? Otherwise I'd implement that.