[e2fsprogs PATCH] e2fsck: don't allow journal inode to have encrypt flag

[e2fsprogs PATCH] e2fsck: don't allow journal inode to have encrypt flag

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

Since the kernel is being fixed to consider journal inodes with the
'encrypt' flag set to be invalid, also update e2fsck accordingly.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 e2fsck/journal.c                   |   3 ++-
 tests/f_badjour_encrypted/expect.1 |  30 +++++++++++++++++++++++++++++
 tests/f_badjour_encrypted/expect.2 |   7 +++++++
 tests/f_badjour_encrypted/image.gz | Bin 0 -> 2637 bytes
 tests/f_badjour_encrypted/name     |   1 +
 5 files changed, 40 insertions(+), 1 deletion(-)
 create mode 100644 tests/f_badjour_encrypted/expect.1
 create mode 100644 tests/f_badjour_encrypted/expect.2
 create mode 100644 tests/f_badjour_encrypted/image.gz
 create mode 100644 tests/f_badjour_encrypted/name

diff --git a/e2fsck/journal.c b/e2fsck/journal.c
index d802c5e9..343e48ba 100644
--- a/e2fsck/journal.c
+++ b/e2fsck/journal.c
@@ -1039,7 +1039,8 @@ static errcode_t e2fsck_get_journal(e2fsck_t ctx, journal_t **ret_journal)
 			tried_backup_jnl++;
 		}
 		if (!j_inode->i_ext2.i_links_count ||
-		    !LINUX_S_ISREG(j_inode->i_ext2.i_mode)) {
+		    !LINUX_S_ISREG(j_inode->i_ext2.i_mode) ||
+		    (j_inode->i_ext2.i_flags & EXT4_ENCRYPT_FL)) {
 			retval = EXT2_ET_NO_JOURNAL;
 			goto try_backup_journal;
 		}
diff --git a/tests/f_badjour_encrypted/expect.1 b/tests/f_badjour_encrypted/expect.1
new file mode 100644
index 00000000..e88e3770
--- /dev/null
+++ b/tests/f_badjour_encrypted/expect.1
@@ -0,0 +1,30 @@
+Superblock has an invalid journal (inode 8).
+Clear? yes
+
+*** journal has been deleted ***
+
+Pass 1: Checking inodes, blocks, and sizes
+Journal inode is not in use, but contains data.  Clear? yes
+
+Pass 2: Checking directory structure
+Pass 3: Checking directory connectivity
+Pass 4: Checking reference counts
+Pass 5: Checking group summary information
+Block bitmap differences:  -(32--33) -(35--49) -(115--1121)
+Fix? yes
+
+Free blocks count wrong for group #0 (926, counted=1950).
+Fix? yes
+
+Free blocks count wrong (926, counted=1950).
+Fix? yes
+
+Recreate journal? yes
+
+Creating journal (1024 blocks):  Done.
+
+*** journal has been regenerated ***
+
+test_filesys: ***** FILE SYSTEM WAS MODIFIED *****
+test_filesys: 11/256 files (0.0% non-contiguous), 1122/2048 blocks
+Exit status is 1
diff --git a/tests/f_badjour_encrypted/expect.2 b/tests/f_badjour_encrypted/expect.2
new file mode 100644
index 00000000..a3744874
--- /dev/null
+++ b/tests/f_badjour_encrypted/expect.2
@@ -0,0 +1,7 @@
+Pass 1: Checking inodes, blocks, and sizes
+Pass 2: Checking directory structure
+Pass 3: Checking directory connectivity
+Pass 4: Checking reference counts
+Pass 5: Checking group summary information
+test_filesys: 11/256 files (9.1% non-contiguous), 1122/2048 blocks
+Exit status is 0
diff --git a/tests/f_badjour_encrypted/image.gz b/tests/f_badjour_encrypted/image.gz
new file mode 100644
index 0000000000000000000000000000000000000000..660496ea5bba9b5589e6ce522feb998a56ab946a
GIT binary patch
literal 2637
zcmb2|=3oE;CgwMHnR%A}ly$iH+hCjen-$w~Z}VkIC+6NxyCc!NYR5sNh}+xBbE4nR
zN&o!s6aW5(lcp<{UH`qxzUS4ptP5pfA==wwf8L3{|MOkc?r&esd8Bgo#^iqA5gFQC
z<@R-dVr@lCdHkR5{rAnDhwlG>jraZU^QHBbXJlmsKi|6fW!ukpZ~6bfSpUB2;pTmx
zj{M*I=+61|wWkw4%t_n+w)pIx)GIr3+pcz<UH*C1)vHaf+5&5L-JN;=TiBLGj=QD!
zvwYR3oA+2BEcm`&zU2O)tkADJdjH<PwxLjZ&!1oZHNUt0v)ga`;{kvAnl~H3enld#
z$+@%1mc~a}A54fin`clHvi0Skf*&i~udaD@=i-@%?sorPp11i^EC2t^@q3p&nP$X3
zzi$!s_i3c{*ZQX!$^Q>{-utf_{{O+ORnhK@!q@)4KVSKq`3wIq_AmS|<S*DSsCW3^
z@Q?8?^DlP4J^%iH{<8evDyi)kD?Tqg`0VhxvhA|(9o0n`65ek4mi5+?8xj$4$636y
z`F^c@U-jvQi=WK?|9-~qcRR0El+UlfvVnKm_Je1l*?MoM1?axIu;=xksEaRLt^f1+
zcI>}%!KM17_pFRy`>QV2pCx6B<9`)>o$;~a`cLx=*Y;<@Z|{cwUm;riQQi9d{>A@(
z&c6T7uKx1sm(kDuuPD6Dvs!ev|MSq*t68t|F8)>W`}uD3t6l#pUM)HIKYZ_&&TqRj
zWsCn;=6#)UcWr%}?)}rh(#y8)a4!C~Kik#*_w$dx|Fgp#PX@#7>W*1KzH66-US2JE
z^-NZn>F!y<XV*Rp{k)o$kqqNTwTy<qXb9jBf%d~z-yS%C)HqDo|5YJ89>4NY+0hUf
k4S}H$0uFBvZUnjWfI|K2%r`6YXzOOCSzdj93=9ek0BUk;ZvX%Q

literal 0
HcmV?d00001

diff --git a/tests/f_badjour_encrypted/name b/tests/f_badjour_encrypted/name
new file mode 100644
index 00000000..e8f4c04f
--- /dev/null
+++ b/tests/f_badjour_encrypted/name
@@ -0,0 +1 @@
+journal inode has encrypt flag

base-commit: aad34909b6648579f42dade5af5b46821aa4d845
-- 
2.38.1

Re: [e2fsprogs PATCH] e2fsck: don't allow journal inode to have encrypt flag

[Andreas Dilger]

[-- Attachment #1: Type: text/plain, Size: 2828 bytes --]

On Nov 1, 2022, at 11:35 PM, Eric Biggers <ebiggers@kernel.org> wrote:
> 
> From: Eric Biggers <ebiggers@google.com>
> 
> Since the kernel is being fixed to consider journal inodes with the
> 'encrypt' flag set to be invalid, also update e2fsck accordingly.
> 
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
> e2fsck/journal.c                   |   3 ++-
> tests/f_badjour_encrypted/expect.1 |  30 +++++++++++++++++++++++++++++
> tests/f_badjour_encrypted/expect.2 |   7 +++++++
> tests/f_badjour_encrypted/image.gz | Bin 0 -> 2637 bytes

Good to have a test case for this.

In the past Ted has asked that new test cases are generated via mke2fs
and debugfs in "f_XXX/script" file rather than a binary image, if possible.
That avoids saving a binary blob in Git, and also makes it much more
clear in the future what is done to the filesystem to corrupt it, rather
than having to reverse engineer this from the bits on disk.

Something like tests/f_dup4/script is a good example of this.  You may
be able to use something as simple as the following:

    touch $TMPFILE
    $MKE2FS -t ext4 -b 1024 -J size=4 ... $TMPFILE 16384
    $DEBUGFS -w -R 'set_inode_field <8> flags 0x800' $TMPFILE
    . $cmd_dir/run_e2fsck

It might be more involved, depending on how the journal was corrupted.
For complex debugfs changes, it is better to use a "here" document to
perform multiple commands than running debugfs multiple times.

The script is also much more compact than the binary image, and tolerates
changes a lot better as well.

Cheers, Andreas

> diff --git a/tests/f_badjour_encrypted/image.gz b/tests/f_badjour_encrypted/image.gz
> new file mode 100644
> index 0000000000000000000000000000000000000000..660496ea5bba9b5589e6ce522feb998a56ab946a
> GIT binary patch
> literal 2637
> zcmb2|=3oE;CgwMHnR%A}ly$iH+hCjen-$w~Z}VkIC+6NxyCc!NYR5sNh}+xBbE4nR
> zN&o!s6aW5(lcp<{UH`qxzUS4ptP5pfA==wwf8L3{|MOkc?r&esd8Bgo#^iqA5gFQC
> z<@R-dVr@lCdHkR5{rAnDhwlG>jraZU^QHBbXJlmsKi|6fW!ukpZ~6bfSpUB2;pTmx
> zj{M*I=+61|wWkw4%t_n+w)pIx)GIr3+pcz<UH*C1)vHaf+5&5L-JN;=TiBLGj=QD!
> zvwYR3oA+2BEcm`&zU2O)tkADJdjH<PwxLjZ&!1oZHNUt0v)ga`;{kvAnl~H3enld#
> z$+@%1mc~a}A54fin`clHvi0Skf*&i~udaD@=i-@%?sorPp11i^EC2t^@q3p&nP$X3
> zzi$!s_i3c{*ZQX!$^Q>{-utf_{{O+ORnhK@!q@)4KVSKq`3wIq_AmS|<S*DSsCW3^
> z@Q?8?^DlP4J^%iH{<8evDyi)kD?Tqg`0VhxvhA|(9o0n`65ek4mi5+?8xj$4$636y
> z`F^c@U-jvQi=WK?|9-~qcRR0El+UlfvVnKm_Je1l*?MoM1?axIu;=xksEaRLt^f1+
> zcI>}%!KM17_pFRy`>QV2pCx6B<9`)>o$;~a`cLx=*Y;<@Z|{cwUm;riQQi9d{>A@(
> z&c6T7uKx1sm(kDuuPD6Dvs!ev|MSq*t68t|F8)>W`}uD3t6l#pUM)HIKYZ_&&TqRj
> zWsCn;=6#)UcWr%}?)}rh(#y8)a4!C~Kik#*_w$dx|Fgp#PX@#7>W*1KzH66-US2JE
> z^-NZn>F!y<XV*Rp{k)o$kqqNTwTy<qXb9jBf%d~z-yS%C)HqDo|5YJ89>4NY+0hUf
> k4S}H$0uFBvZUnjWfI|K2%r`6YXzOOCSzdj93=9ek0BUk;ZvX%Q


Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]

Re: [e2fsprogs PATCH] e2fsck: don't allow journal inode to have encrypt flag

[Eric Biggers]

On Wed, Nov 02, 2022 at 02:55:05PM -0600, Andreas Dilger wrote:
> On Nov 1, 2022, at 11:35 PM, Eric Biggers <ebiggers@kernel.org> wrote:
> > 
> > From: Eric Biggers <ebiggers@google.com>
> > 
> > Since the kernel is being fixed to consider journal inodes with the
> > 'encrypt' flag set to be invalid, also update e2fsck accordingly.
> > 
> > Signed-off-by: Eric Biggers <ebiggers@google.com>
> > ---
> > e2fsck/journal.c                   |   3 ++-
> > tests/f_badjour_encrypted/expect.1 |  30 +++++++++++++++++++++++++++++
> > tests/f_badjour_encrypted/expect.2 |   7 +++++++
> > tests/f_badjour_encrypted/image.gz | Bin 0 -> 2637 bytes
> 
> Good to have a test case for this.
> 
> In the past Ted has asked that new test cases are generated via mke2fs
> and debugfs in "f_XXX/script" file rather than a binary image, if possible.

I didn't realize the test suite supported this.  Done in v2, thanks!

- Eric