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.

Reply via email to