Message-Id: <E44A9FB1-280E-4EB7-9092-856E200EE500@dilger.ca>
Date: Thu, 18 Apr 2024 16:04:10 -0600
From: Andreas Dilger <adilger@...ger.ca>
To: Theodore Ts'o <tytso@....edu>
Cc: Ext4 Developers List <linux-ext4@...r.kernel.org>
Subject: Re: [PATCH] misc: add 2038 timestamp support

On Apr 18, 2024, at 8:36 AM, Theodore Ts'o <tytso@....edu> 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@....edu>
> 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@....edu>
> 
> commit 9103e1e792170a836884db4ee9f2762bf1684f09
> Author: Theodore Ts'o <tytso@....edu>
> 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@....edu>


Cheers, Andreas






Download attachment "signature.asc" of type "application/pgp-signature" (874 bytes)

Powered by blists - more mailing lists