[PATCH] ext4: fix decoding of raw_inode timestamps

[PATCH] ext4: fix decoding of raw_inode timestamps

[Jeff Layton]

When we covert a timestamp from raw disk format, we need to consider it
to be signed, as the value may represent a date earlier than 1970. This
fixes generic/258 on ext4.

Cc: Jan Kara <jack@suse.cz>
Fixes: f2ddb05870fb ("ext4: convert to ctime accessor functions")
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/ext4/ext4.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

It might be best to just squash this fix in with the ext4 conversion in
the vfs tree.

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index d63543187359..2af347669db7 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -877,7 +877,7 @@ static inline __le32 ext4_encode_extra_time(struct timespec64 ts)
 static inline struct timespec64 ext4_decode_extra_time(__le32 base,
 						       __le32 extra)
 {
-	struct timespec64 ts = { .tv_sec = le32_to_cpu(base) };
+	struct timespec64 ts = { .tv_sec = (signed)le32_to_cpu(base) };
 
 	if (unlikely(extra & cpu_to_le32(EXT4_EPOCH_MASK)))
 		ts.tv_sec += (u64)(le32_to_cpu(extra) & EXT4_EPOCH_MASK) << 32;
-- 
2.41.0

Re: [PATCH] ext4: fix decoding of raw_inode timestamps

[Christian Brauner]

On Wed, 12 Jul 2023 11:02:49 -0400, Jeff Layton wrote:
> When we covert a timestamp from raw disk format, we need to consider it
> to be signed, as the value may represent a date earlier than 1970. This
> fixes generic/258 on ext4.
> 
> 

Applied to the vfs.ctime branch of the vfs/vfs.git tree.
Patches in the vfs.ctime branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs.ctime

[1/1 FOLDED] ext4: convert to ctime accessor functions
      https://git.kernel.org/vfs/vfs/c/f65cb009d449

Re: [PATCH] ext4: fix decoding of raw_inode timestamps

[Theodore Ts'o]

On Wed, Jul 12, 2023 at 11:02:49AM -0400, Jeff Layton wrote:
> When we covert a timestamp from raw disk format, we need to consider it
> to be signed, as the value may represent a date earlier than 1970. This
> fixes generic/258 on ext4.
> 
> Cc: Jan Kara <jack@suse.cz>
> Fixes: f2ddb05870fb ("ext4: convert to ctime accessor functions")
> Signed-off-by: Jeff Layton <jlayton@kernel.org>

Acked-by: Theodore Ts'o <tytso@mit.edu>

Thanks for the fix!

It had been on my list to checking to see if the ext4 kunit tests
would pass, since Jan had mentioned that he had done the work to make
sure the ext4 kunit test would compile, but he hadn't gotten around to
try run the kunit test.  Unfortunately, I hadn't gotten to it.

I *think* the ext4 kunit tests should have caught this as well; out of
curiosity, have you tried running the ext4 kunit tests either before
or after this patch?  If so, what were your findings?

Cheers,

					- Ted

Re: [PATCH] ext4: fix decoding of raw_inode timestamps

[Jeff Layton]

On Wed, 2023-07-12 at 13:52 -0400, Theodore Ts'o wrote:
> On Wed, Jul 12, 2023 at 11:02:49AM -0400, Jeff Layton wrote:
> > When we covert a timestamp from raw disk format, we need to consider it
> > to be signed, as the value may represent a date earlier than 1970. This
> > fixes generic/258 on ext4.
> > 
> > Cc: Jan Kara <jack@suse.cz>
> > Fixes: f2ddb05870fb ("ext4: convert to ctime accessor functions")
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> 
> Acked-by: Theodore Ts'o <tytso@mit.edu>
> 
> Thanks for the fix!
> 
> It had been on my list to checking to see if the ext4 kunit tests
> would pass, since Jan had mentioned that he had done the work to make
> sure the ext4 kunit test would compile, but he hadn't gotten around to
> try run the kunit test.  Unfortunately, I hadn't gotten to it.
> 
> I *think* the ext4 kunit tests should have caught this as well; out of
> curiosity, have you tried running the ext4 kunit tests either before
> or after this patch?  If so, what were your findings?
> 
> Cheers,
> 
> 					- Ted

No, I haven't. I'm running fstests on it now. Is there a quickstart for
running those tests?
-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH] ext4: fix decoding of raw_inode timestamps

[Theodore Ts'o]

On Wed, Jul 12, 2023 at 02:09:59PM -0400, Jeff Layton wrote:
> 
> No, I haven't. I'm running fstests on it now. Is there a quickstart for
> running those tests?

At the top level kernel sources:

./tools/testing/kunit/kunit.py  run --kunitconfig ./fs/ext4/.kunitconfig

You should get:

[17:23:09] Starting KUnit Kernel (1/1)...
[17:23:09] ============================================================
[17:23:09] =============== ext4_inode_test (1 subtest) ================
[17:23:09] ============= inode_test_xtimestamp_decoding  ==============
[17:23:09] [PASSED] 1901-12-13 Lower bound of 32bit < 0 timestamp, no extra bits
[17:23:09] [PASSED] 1969-12-31 Upper bound of 32bit < 0 timestamp, no extra bits
[17:23:09] [PASSED] 1970-01-01 Lower bound of 32bit >=0 timestamp, no extra bits
[17:23:09] [PASSED] 2038-01-19 Upper bound of 32bit >=0 timestamp, no extra bits
[17:23:09] [PASSED] 2038-01-19 Lower bound of 32bit <0 timestamp, lo extra sec bit on
[17:23:09] [PASSED] 2106-02-07 Upper bound of 32bit <0 timestamp, lo extra sec bit on
[17:23:09] [PASSED] 2106-02-07 Lower bound of 32bit >=0 timestamp, lo extra sec bit on
[17:23:09] [PASSED] 2174-02-25 Upper bound of 32bit >=0 timestamp, lo extra sec bit on
[17:23:09] [PASSED] 2174-02-25 Lower bound of 32bit <0 timestamp, hi extra sec bit on
[17:23:09] [PASSED] 2242-03-16 Upper bound of 32bit <0 timestamp, hi extra sec bit on
[17:23:09] [PASSED] 2242-03-16 Lower bound of 32bit >=0 timestamp, hi extra sec bit on
[17:23:09] [PASSED] 2310-04-04 Upper bound of 32bit >=0 timestamp, hi extra sec bit on
[17:23:09] [PASSED] 2310-04-04 Upper bound of 32bit>=0 timestamp, hi extra sec bit 1. 1 ns
[17:23:09] [PASSED] 2378-04-22 Lower bound of 32bit>= timestamp. Extra sec bits 1. Max ns
[17:23:09] [PASSED] 2378-04-22 Lower bound of 32bit >=0 timestamp. All extra sec bits on
[17:23:09] [PASSED] 2446-05-10 Upper bound of 32bit >=0 timestamp. All extra sec bits on
[17:23:09] ========= [PASSED] inode_test_xtimestamp_decoding ==========
[17:23:09] ================= [PASSED] ext4_inode_test =================
[17:23:09] ============================================================
[17:23:09] Testing complete. Ran 16 tests: passed: 16
[17:23:09] Elapsed time: 1.943s total, 0.001s configuring, 1.777s building, 0.123s running

	   	   	 	       	      		   - Ted

Re: [PATCH] ext4: fix decoding of raw_inode timestamps

[Jeff Layton]

On Wed, 2023-07-12 at 17:25 -0400, Theodore Ts'o wrote:
> On Wed, Jul 12, 2023 at 02:09:59PM -0400, Jeff Layton wrote:
> > 
> > No, I haven't. I'm running fstests on it now. Is there a quickstart for
> > running those tests?
> 
> At the top level kernel sources:
> 
> ./tools/testing/kunit/kunit.py  run --kunitconfig ./fs/ext4/.kunitconfig
> 
> You should get:
> 
> [17:23:09] Starting KUnit Kernel (1/1)...
> [17:23:09] ============================================================
> [17:23:09] =============== ext4_inode_test (1 subtest) ================
> [17:23:09] ============= inode_test_xtimestamp_decoding  ==============
> [17:23:09] [PASSED] 1901-12-13 Lower bound of 32bit < 0 timestamp, no extra bits
> [17:23:09] [PASSED] 1969-12-31 Upper bound of 32bit < 0 timestamp, no extra bits
> [17:23:09] [PASSED] 1970-01-01 Lower bound of 32bit >=0 timestamp, no extra bits
> [17:23:09] [PASSED] 2038-01-19 Upper bound of 32bit >=0 timestamp, no extra bits
> [17:23:09] [PASSED] 2038-01-19 Lower bound of 32bit <0 timestamp, lo extra sec bit on
> [17:23:09] [PASSED] 2106-02-07 Upper bound of 32bit <0 timestamp, lo extra sec bit on
> [17:23:09] [PASSED] 2106-02-07 Lower bound of 32bit >=0 timestamp, lo extra sec bit on
> [17:23:09] [PASSED] 2174-02-25 Upper bound of 32bit >=0 timestamp, lo extra sec bit on
> [17:23:09] [PASSED] 2174-02-25 Lower bound of 32bit <0 timestamp, hi extra sec bit on
> [17:23:09] [PASSED] 2242-03-16 Upper bound of 32bit <0 timestamp, hi extra sec bit on
> [17:23:09] [PASSED] 2242-03-16 Lower bound of 32bit >=0 timestamp, hi extra sec bit on
> [17:23:09] [PASSED] 2310-04-04 Upper bound of 32bit >=0 timestamp, hi extra sec bit on
> [17:23:09] [PASSED] 2310-04-04 Upper bound of 32bit>=0 timestamp, hi extra sec bit 1. 1 ns
> [17:23:09] [PASSED] 2378-04-22 Lower bound of 32bit>= timestamp. Extra sec bits 1. Max ns
> [17:23:09] [PASSED] 2378-04-22 Lower bound of 32bit >=0 timestamp. All extra sec bits on
> [17:23:09] [PASSED] 2446-05-10 Upper bound of 32bit >=0 timestamp. All extra sec bits on
> [17:23:09] ========= [PASSED] inode_test_xtimestamp_decoding ==========
> [17:23:09] ================= [PASSED] ext4_inode_test =================
> [17:23:09] ============================================================
> [17:23:09] Testing complete. Ran 16 tests: passed: 16
> [17:23:09] Elapsed time: 1.943s total, 0.001s configuring, 1.777s building, 0.123s running
> 
> 	   	   	 	       	      		   - Ted

Thanks Ted,

The above output is what I get with the fix in place. Without this
patch, I get:

$ make ARCH=um O=.kunit --jobs=16
[06:46:35] Starting KUnit Kernel (1/1)...
[06:46:35] ============================================================
[06:46:35] =============== ext4_inode_test (1 subtest) ================
[06:46:35] ============= inode_test_xtimestamp_decoding  ==============
[06:46:35]     # inode_test_xtimestamp_decoding: EXPECTATION FAILED at fs/ext4/inode-test.c:252
[06:46:35]     Expected test_param->expected.tv_sec == timestamp.tv_sec, but
[06:46:35]         test_param->expected.tv_sec == -2147483648 (0xffffffff80000000)
[06:46:35]         timestamp.tv_sec == 2147483648 (0x80000000)
[06:46:35] 1901-12-13 Lower bound of 32bit < 0 timestamp, no extra bits: msb:1 lower_bound:1 extra_bits: 0
[06:46:35] [FAILED] 1901-12-13 Lower bound of 32bit < 0 timestamp, no extra bits
[06:46:35]     # inode_test_xtimestamp_decoding: EXPECTATION FAILED at fs/ext4/inode-test.c:252
[06:46:35]     Expected test_param->expected.tv_sec == timestamp.tv_sec, but
[06:46:35]         test_param->expected.tv_sec == -1 (0xffffffffffffffff)
[06:46:35]         timestamp.tv_sec == 4294967295 (0xffffffff)
[06:46:35] 1969-12-31 Upper bound of 32bit < 0 timestamp, no extra bits: msb:1 lower_bound:0 extra_bits: 0
[06:46:35] [FAILED] 1969-12-31 Upper bound of 32bit < 0 timestamp, no extra bits
[06:46:35] [FAILED] 1970-01-01 Lower bound of 32bit >=0 timestamp, no extra bits
[06:46:35] [FAILED] 2038-01-19 Upper bound of 32bit >=0 timestamp, no extra bits
[06:46:35]     # inode_test_xtimestamp_decoding: EXPECTATION FAILED at fs/ext4/inode-test.c:252
[06:46:35]     Expected test_param->expected.tv_sec == timestamp.tv_sec, but
[06:46:35]         test_param->expected.tv_sec == 2147483648 (0x80000000)
[06:46:35]         timestamp.tv_sec == 6442450944 (0x180000000)
[06:46:35] 2038-01-19 Lower bound of 32bit <0 timestamp, lo extra sec bit on: msb:1 lower_bound:1 extra_bits: 1
[06:46:35] [FAILED] 2038-01-19 Lower bound of 32bit <0 timestamp, lo extra sec bit on
[06:46:35]     # inode_test_xtimestamp_decoding: EXPECTATION FAILED at fs/ext4/inode-test.c:252
[06:46:35]     Expected test_param->expected.tv_sec == timestamp.tv_sec, but
[06:46:35]         test_param->expected.tv_sec == 4294967295 (0xffffffff)
[06:46:35]         timestamp.tv_sec == 8589934591 (0x1ffffffff)
[06:46:35] 2106-02-07 Upper bound of 32bit <0 timestamp, lo extra sec bit on: msb:1 lower_bound:0 extra_bits: 1
[06:46:35] [FAILED] 2106-02-07 Upper bound of 32bit <0 timestamp, lo extra sec bit on
[06:46:35] [FAILED] 2106-02-07 Lower bound of 32bit >=0 timestamp, lo extra sec bit on
[06:46:35] [FAILED] 2174-02-25 Upper bound of 32bit >=0 timestamp, lo extra sec bit on
[06:46:35]     # inode_test_xtimestamp_decoding: EXPECTATION FAILED at fs/ext4/inode-test.c:252
[06:46:35]     Expected test_param->expected.tv_sec == timestamp.tv_sec, but
[06:46:35]         test_param->expected.tv_sec == 6442450944 (0x180000000)
[06:46:35]         timestamp.tv_sec == 10737418240 (0x280000000)
[06:46:35] 2174-02-25 Lower bound of 32bit <0 timestamp, hi extra sec bit on: msb:1 lower_bound:1 extra_bits: 2
[06:46:35] [FAILED] 2174-02-25 Lower bound of 32bit <0 timestamp, hi extra sec bit on
[06:46:35]     # inode_test_xtimestamp_decoding: EXPECTATION FAILED at fs/ext4/inode-test.c:252
[06:46:35]     Expected test_param->expected.tv_sec == timestamp.tv_sec, but
[06:46:35]         test_param->expected.tv_sec == 8589934591 (0x1ffffffff)
[06:46:35]         timestamp.tv_sec == 12884901887 (0x2ffffffff)
[06:46:35] 2242-03-16 Upper bound of 32bit <0 timestamp, hi extra sec bit on: msb:1 lower_bound:0 extra_bits: 2
[06:46:35] [FAILED] 2242-03-16 Upper bound of 32bit <0 timestamp, hi extra sec bit on
[06:46:35] [FAILED] 2242-03-16 Lower bound of 32bit >=0 timestamp, hi extra sec bit on
[06:46:35] [FAILED] 2310-04-04 Upper bound of 32bit >=0 timestamp, hi extra sec bit on
[06:46:35] [FAILED] 2310-04-04 Upper bound of 32bit>=0 timestamp, hi extra sec bit 1. 1 ns
[06:46:35] [FAILED] 2378-04-22 Lower bound of 32bit>= timestamp. Extra sec bits 1. Max ns
[06:46:35] [FAILED] 2378-04-22 Lower bound of 32bit >=0 timestamp. All extra sec bits on
[06:46:35] [FAILED] 2446-05-10 Upper bound of 32bit >=0 timestamp. All extra sec bits on
[06:46:35] # inode_test_xtimestamp_decoding: pass:0 fail:16 skip:0 total:16
[06:46:35] ========= [FAILED] inode_test_xtimestamp_decoding ==========
[06:46:35] # Totals: pass:0 fail:16 skip:0 total:16
[06:46:35] ================= [FAILED] ext4_inode_test =================
[06:46:35] ============================================================
[06:46:35] Testing complete. Ran 16 tests: failed: 16
[06:46:35] Elapsed time: 14.549s total, 0.002s configuring, 14.229s building, 0.275s running


Cheers,
-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH] ext4: fix decoding of raw_inode timestamps

[Theodore Ts'o]

On Thu, Jul 13, 2023 at 06:48:04AM -0400, Jeff Layton wrote:
> 
> The above output is what I get with the fix in place. Without this
> patch, I get: ...

Thanks!!  It's good to know the _one_ kunit test we have is capable of
detecting this.  We have a patch series lined up to add our *second*
unit test (for the block allocator) for the next merge window, and
while our unit test coverage is still quite small, it's nice to know
that it can detect problems --- and much faster than running xfstests.  :-}

     	    	   	    	    	 	- Ted

Re: [PATCH] ext4: fix decoding of raw_inode timestamps

[Jeff Layton]

On Thu, 2023-07-13 at 09:04 -0400, Theodore Ts'o wrote:
> On Thu, Jul 13, 2023 at 06:48:04AM -0400, Jeff Layton wrote:
> > 
> > The above output is what I get with the fix in place. Without this
> > patch, I get: ...
> 
> Thanks!!  It's good to know the _one_ kunit test we have is capable of
> detecting this.  We have a patch series lined up to add our *second*
> unit test (for the block allocator) for the next merge window, and
> while our unit test coverage is still quite small, it's nice to know
> that it can detect problems --- and much faster than running xfstests.  :-}
> 

Yeah, it's pretty quick! I need to consider adding some tests for some
other areas that are difficult to view outside the kernel (the errseq_t
infrastructure comes to mind).
-- 
Jeff Layton <jlayton@kernel.org>