Re: [Linux-ima-devel] [PATCH] fs: ubifs: Add i_version support

[Richard W. <ri...@no...>]

Sascha,

Am Dienstag, 12. September 2017, 12:39:00 CEST schrieb Sascha Hauer:
> This adds i_version support to UBIFS. The inodes i_version is used by
> IMA to detect changes to an inode and thus necessary to support IMA on
> UBIFS. The i_version is stored in the previously unused space in the
> UBIFS inode struct. Unlike in ext4 i_version support is unconditionally
> enabled in UBIFS as I saw no reason to make it optional.

But we need a new UBIFS feature flag to indicate that this filesystem has 
valid i_version fields.

> Signed-off-by: Sascha Hauer <s....@pe...>
> ---
>  fs/ubifs/dir.c         | 30 +++++++++++++++++++-----------
>  fs/ubifs/file.c        |  5 +++++
>  fs/ubifs/journal.c     |  3 ++-
>  fs/ubifs/super.c       |  2 ++
>  fs/ubifs/ubifs-media.h |  3 ++-
>  5 files changed, 30 insertions(+), 13 deletions(-)
> 
> I did this patch exclusively to support IMA on UBIFS. IMA uses the inode's
> i_version field to detect changes on inodes. A proper i_version support
> needs to make the i_version persistent on disk, although IMA itself doesn't
> need a persistent i_version. Last time an earlier version of this patch
> 
> was sent by Oleksij Rempel Richard said:
> > What about making i_version persistent?
> > We still have some empty fields in UBIFS' inode data structure.
> > But first we have to be very sure that we need it.
> 
> This patch exactly implements this suggestion, leaving the question if we
> really need it. I added the IMA maintainers to Cc in the hope that Mimi or
> Dmitry can give a good reason why there's no alternative to i_version for
> IMA.

Yes, it would be good to know more about the user, IMA. Does IMA store the 
version somewhere?
Are there requirements on ordering? i.e. What if UBIFS faces a power-cut
and the UBIFS i_version is behind IMA's version.
Maybe we have to teach UBIFS to update an inode less lazy that it currently 
does...

Thanks,
//richard