[PATCH 0/2] ubifs: fixes for FS_IOC_GETFLAGS and FS_IOC_SETFLAGS

[PATCH 0/2] ubifs: fixes for FS_IOC_GETFLAGS and FS_IOC_SETFLAGS

[Eric Biggers]

On ubifs, fix FS_IOC_SETFLAGS to not clear the encrypt flag, and update
FS_IOC_GETFLAGS to return the encrypt flag like ext4 and f2fs do.

Eric Biggers (2):
  ubifs: fix FS_IOC_SETFLAGS unexpectedly clearing encrypt flag
  ubifs: add support for FS_ENCRYPT_FL

 fs/ubifs/ioctl.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

-- 
2.24.0.393.g34dc348eaf-goog

[PATCH 1/2] ubifs: fix FS_IOC_SETFLAGS unexpectedly clearing encrypt flag

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

UBIFS's implementation of FS_IOC_SETFLAGS fails to preserve existing
inode flags that aren't settable by FS_IOC_SETFLAGS, namely the encrypt
flag.  This causes the encrypt flag to be unexpectedly cleared.

Fix it by preserving existing unsettable flags, like ext4 and f2fs do.

Test case with kvm-xfstests shell:

    FSTYP=ubifs KEYCTL_PROG=keyctl
    . fs/ubifs/config
    . ~/xfstests/common/encrypt
    dev=$(__blkdev_to_ubi_volume /dev/vdc)
    ubiupdatevol -t $dev
    mount $dev /mnt -t ubifs
    k=$(_generate_session_encryption_key)
    mkdir /mnt/edir
    xfs_io -c "set_encpolicy $k" /mnt/edir
    echo contents > /mnt/edir/file
    chattr +i /mnt/edir/file
    chattr -i /mnt/edir/file

With the bug, the following errors occur on the last command:

    [   18.081559] fscrypt (ubifs, inode 67): Inconsistent encryption context (parent directory: 65)
    chattr: Operation not permitted while reading flags on /mnt/edir/file

Fixes: d475a507457b ("ubifs: Add skeleton for fscrypto")
Cc: <stable@vger.kernel.org> # v4.10+
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/ubifs/ioctl.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/ubifs/ioctl.c b/fs/ubifs/ioctl.c
index 5dc5abca11c70..eeb1be2598881 100644
--- a/fs/ubifs/ioctl.c
+++ b/fs/ubifs/ioctl.c
@@ -113,7 +113,8 @@ static int setflags(struct inode *inode, int flags)
 	if (err)
 		goto out_unlock;
 
-	ui->flags = ioctl2ubifs(flags);
+	ui->flags &= ~ioctl2ubifs(UBIFS_SUPPORTED_IOCTL_FLAGS);
+	ui->flags |= ioctl2ubifs(flags);
 	ubifs_set_inode_flags(inode);
 	inode->i_ctime = current_time(inode);
 	release = ui->dirty;
-- 
2.24.0.393.g34dc348eaf-goog

Re: [PATCH 1/2] ubifs: fix FS_IOC_SETFLAGS unexpectedly clearing encrypt flag

[Eric Biggers]

On Mon, Dec 16, 2019 at 03:06:35PM +0000, Sasha Levin wrote:
> Hi,
> 
> [This is an automated email]
> 
> This commit has been processed because it contains a "Fixes:" tag,
> fixing commit: d475a507457b ("ubifs: Add skeleton for fscrypto").
> 
> The bot has tested the following trees: v5.4.2, v5.3.15, v4.19.88, v4.14.158.
> 
> v5.4.2: Build OK!
> v5.3.15: Build OK!
> v4.19.88: Build failed! Errors:
>     fs/ubifs/ioctl.c:130:28: error: ‘UBIFS_SUPPORTED_IOCTL_FLAGS’ undeclared (first use in this function)
> 
> v4.14.158: Build failed! Errors:
>     fs/ubifs/ioctl.c:127:28: error: ‘UBIFS_SUPPORTED_IOCTL_FLAGS’ undeclared (first use in this function)
> 
> 
> NOTE: The patch will not be queued to stable trees until it is upstream.
> 
> How should we proceed with this patch?
> 

4.19 and 4.14 will build if you apply commit 2fe8b2d5578d
("ubifs: Reject unsupported ioctl flags explicitly") first.
That was a bug fix too, so I recommend applying it.

- Eric

[PATCH 2/2] ubifs: add support for FS_ENCRYPT_FL

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

Make the FS_IOC_GETFLAGS ioctl on ubifs return the FS_ENCRYPT_FL flag on
encrypted files, like ext4 and f2fs do.

Also make this flag be ignored by FS_IOC_SETFLAGS, like ext4 and f2fs
do, since it's a recognized flag but is not directly settable.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/ubifs/ioctl.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/fs/ubifs/ioctl.c b/fs/ubifs/ioctl.c
index eeb1be2598881..d49fc04f2d7d4 100644
--- a/fs/ubifs/ioctl.c
+++ b/fs/ubifs/ioctl.c
@@ -17,10 +17,14 @@
 #include "ubifs.h"
 
 /* Need to be kept consistent with checked flags in ioctl2ubifs() */
-#define UBIFS_SUPPORTED_IOCTL_FLAGS \
+#define UBIFS_SETTABLE_IOCTL_FLAGS \
 	(FS_COMPR_FL | FS_SYNC_FL | FS_APPEND_FL | \
 	 FS_IMMUTABLE_FL | FS_DIRSYNC_FL)
 
+/* Need to be kept consistent with checked flags in ubifs2ioctl() */
+#define UBIFS_GETTABLE_IOCTL_FLAGS \
+	(UBIFS_SETTABLE_IOCTL_FLAGS | FS_ENCRYPT_FL)
+
 /**
  * ubifs_set_inode_flags - set VFS inode flags.
  * @inode: VFS inode to set flags for
@@ -91,6 +95,8 @@ static int ubifs2ioctl(int ubifs_flags)
 		ioctl_flags |= FS_IMMUTABLE_FL;
 	if (ubifs_flags & UBIFS_DIRSYNC_FL)
 		ioctl_flags |= FS_DIRSYNC_FL;
+	if (ubifs_flags & UBIFS_CRYPT_FL)
+		ioctl_flags |= FS_ENCRYPT_FL;
 
 	return ioctl_flags;
 }
@@ -113,7 +119,7 @@ static int setflags(struct inode *inode, int flags)
 	if (err)
 		goto out_unlock;
 
-	ui->flags &= ~ioctl2ubifs(UBIFS_SUPPORTED_IOCTL_FLAGS);
+	ui->flags &= ~ioctl2ubifs(UBIFS_SETTABLE_IOCTL_FLAGS);
 	ui->flags |= ioctl2ubifs(flags);
 	ubifs_set_inode_flags(inode);
 	inode->i_ctime = current_time(inode);
@@ -156,8 +162,9 @@ long ubifs_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 		if (get_user(flags, (int __user *) arg))
 			return -EFAULT;
 
-		if (flags & ~UBIFS_SUPPORTED_IOCTL_FLAGS)
+		if (flags & ~UBIFS_GETTABLE_IOCTL_FLAGS)
 			return -EOPNOTSUPP;
+		flags &= UBIFS_SETTABLE_IOCTL_FLAGS;
 
 		if (!S_ISDIR(inode->i_mode))
 			flags &= ~FS_DIRSYNC_FL;
-- 
2.24.0.393.g34dc348eaf-goog

Re: [PATCH 0/2] ubifs: fixes for FS_IOC_GETFLAGS and FS_IOC_SETFLAGS

[Eric Biggers]

On Mon, Dec 09, 2019 at 02:23:23PM -0800, Eric Biggers wrote:
> On ubifs, fix FS_IOC_SETFLAGS to not clear the encrypt flag, and update
> FS_IOC_GETFLAGS to return the encrypt flag like ext4 and f2fs do.
> 
> Eric Biggers (2):
>   ubifs: fix FS_IOC_SETFLAGS unexpectedly clearing encrypt flag
>   ubifs: add support for FS_ENCRYPT_FL
> 
>  fs/ubifs/ioctl.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 

Richard, can you consider applying this series to the UBIFS tree for 5.6?

- Eric

Re: [PATCH 0/2] ubifs: fixes for FS_IOC_GETFLAGS and FS_IOC_SETFLAGS

[Eric Biggers]

On Mon, Dec 09, 2019 at 02:23:23PM -0800, Eric Biggers wrote:
> On ubifs, fix FS_IOC_SETFLAGS to not clear the encrypt flag, and update
> FS_IOC_GETFLAGS to return the encrypt flag like ext4 and f2fs do.
> 
> Eric Biggers (2):
>   ubifs: fix FS_IOC_SETFLAGS unexpectedly clearing encrypt flag
>   ubifs: add support for FS_ENCRYPT_FL
> 
>  fs/ubifs/ioctl.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)

Richard, have you had a chance to review these?  I'm intending that these be
taken through the UBIFS tree too.

- Eric

Re: [PATCH 0/2] ubifs: fixes for FS_IOC_GETFLAGS and FS_IOC_SETFLAGS

[Richard Weinberger]

----- Ursprüngliche Mail -----
> Von: "Eric Biggers" <ebiggers@kernel.org>
> An: "richard" <richard@nod.at>
> CC: "linux-mtd" <linux-mtd@lists.infradead.org>, "linux-fscrypt" <linux-fscrypt@vger.kernel.org>, "linux-fsdevel"
> <linux-fsdevel@vger.kernel.org>
> Gesendet: Dienstag, 14. Januar 2020 23:00:17
> Betreff: Re: [PATCH 0/2] ubifs: fixes for FS_IOC_GETFLAGS and FS_IOC_SETFLAGS

> On Mon, Dec 09, 2019 at 02:23:23PM -0800, Eric Biggers wrote:
>> On ubifs, fix FS_IOC_SETFLAGS to not clear the encrypt flag, and update
>> FS_IOC_GETFLAGS to return the encrypt flag like ext4 and f2fs do.
>> 
>> Eric Biggers (2):
>>   ubifs: fix FS_IOC_SETFLAGS unexpectedly clearing encrypt flag
>>   ubifs: add support for FS_ENCRYPT_FL
>> 
>>  fs/ubifs/ioctl.c | 14 +++++++++++---
>>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> Richard, have you had a chance to review these?  I'm intending that these be
> taken through the UBIFS tree too.

It is in my review queue. Didn't I update the patch state in patchwork (me double checks)?

Thanks,
//richard

Re: [PATCH 0/2] ubifs: fixes for FS_IOC_GETFLAGS and FS_IOC_SETFLAGS

[Richard Weinberger]

On Tue, Jan 14, 2020 at 11:04 PM Richard Weinberger <richard@nod.at> wrote:
>
> ----- Ursprüngliche Mail -----
> > Von: "Eric Biggers" <ebiggers@kernel.org>
> > An: "richard" <richard@nod.at>
> > CC: "linux-mtd" <linux-mtd@lists.infradead.org>, "linux-fscrypt" <linux-fscrypt@vger.kernel.org>, "linux-fsdevel"
> > <linux-fsdevel@vger.kernel.org>
> > Gesendet: Dienstag, 14. Januar 2020 23:00:17
> > Betreff: Re: [PATCH 0/2] ubifs: fixes for FS_IOC_GETFLAGS and FS_IOC_SETFLAGS
>
> > On Mon, Dec 09, 2019 at 02:23:23PM -0800, Eric Biggers wrote:
> >> On ubifs, fix FS_IOC_SETFLAGS to not clear the encrypt flag, and update
> >> FS_IOC_GETFLAGS to return the encrypt flag like ext4 and f2fs do.
> >>
> >> Eric Biggers (2):
> >>   ubifs: fix FS_IOC_SETFLAGS unexpectedly clearing encrypt flag
> >>   ubifs: add support for FS_ENCRYPT_FL
> >>
> >>  fs/ubifs/ioctl.c | 14 +++++++++++---
> >>  1 file changed, 11 insertions(+), 3 deletions(-)
> >
> > Richard, have you had a chance to review these?  I'm intending that these be
> > taken through the UBIFS tree too.

Both applied. Thanks a lot for addressing this, Eric.
-- 
Thanks,
//richard