On Apr 18, 2024, at 8:36 AM, Theodore Ts'o <tytso@xxxxxxx> wrote:
>
> On Tue, Sep 26, 2023 at 11:40:16PM -0600, Andreas Dilger wrote:
>> The ext4 kernel code implemented support for s_mtime_hi,
>> s_wtime_hi, and related timestamp fields to avoid timestamp
>> overflow in 2038, but similar handling is not in e2fsprogs.
>> ...
>
> Hey Andreas,
>
> I had recently taken this patch, but I've since found that it was
> causing a number of problems. These have been fixed on the next
> branch, but if you have your own build of e2fsprogs, you might want to
> make sure you have these two fixups. The second is especially
> important if you plan to use debugfs's set_super_value command on
> customer file systems....
Ted, thanks for catching this. Indeed, I had not tested this on 32-bit
systems (I think even my watch is 64-bit?), but as other recent posts
attest there are still 32-bit systems in use somewhere in the world.
This y2038 patch hasn't been in use anywhere in production. I had just
noticed while looking at the code that it was inconsistent with the
ext4 code and thought I'd "do the right thing" and submit a patch to
fix it. It only had manual bench testing and "make check". Sorry to
have introduced a broken patch.
> In the future, I strongly suggest that large patches to e2fsprogs are
> run with make check run with trees built with "configure
> --enable-ubsan" and "configure -enable-asan". If you have a github
> account, pushing the changes so that the github actions will do a CI
> using github actions to make sure that there aren't build problems on
> i386, Windows, MacOS, and Android is also a good thing to do.
I've never used Github actions for this. If I fork tytso/e2fsprogs and
push to adilger/e2fsprogs, are those actions automated already with a
config file inside the repo, or do I need to set that up myself?
PS: the kernel.org repo looks like it has not been updated in 4 months,
despite emails that you have landed patches. I was pulling from there
and didn't notice until now that you have been pushing only to github.
Cheers, Andreas
> commit 5b599a325c1af94111940c14d888ade937f29d19
> Author: Theodore Ts'o <tytso@xxxxxxx>
> Date: Wed Apr 17 23:47:02 2024 -0400
>
> Fix 32-bit build and test failures
>
> Commit ca8bc9240a00 ("Add post-2038 timestamp support to e2fsprogs")
> was never built or tested on a 32-bit. It introduced some build
> problems when time_t is a 32-bit integer, and it exposed some test
> bugs. Fix them.
>
> Fixes: ca8bc9240a00 ("Add post-2038 timestamp support to e2fsprogs")
> Signed-off-by: Theodore Ts'o <tytso@xxxxxxx>
>
> commit 9103e1e792170a836884db4ee9f2762bf1684f09
> Author: Theodore Ts'o <tytso@xxxxxxx>
> Date: Thu Apr 18 10:04:27 2024 -0400
>
> debugfs: fix set_field's handling of timestamps
>
> How timestamps are encoded in inodes and superblocks are different.
> Unfortunately, commit ca8bc9240a00 which added post-2038 timestamps
> was (a) overwriting adjacent superblock fields and/or attempting
> unaligned writes to a 8-bit field from a 32-bit pointer, and (b) using
> the incorrect encoding for timestamps stored in inodes. Fix both of
> these issues, which were found thanks to UBSAN.
>
> Fixes: ca8bc9240a00 ("Add post-2038 timestamp support to e2fsprogs")
> Signed-off-by: Theodore Ts'o <tytso@xxxxxxx>
Cheers, Andreas
Attachment:
signature.asc
Description: Message signed with OpenPGP
