Received-SPF: pass (google.com: domain of linux-ext4+bounces-2147-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
From: Andreas Dilger <adilger@dilger.ca>
Message-Id: <E44A9FB1-280E-4EB7-9092-856E200EE500@dilger.ca>
Content-Type: multipart/signed;
 boundary="Apple-Mail=_68ED42E6-2BA3-4024-850D-77F8474F0A11";
 protocol="application/pgp-signature"; micalg=pgp-sha256
Precedence: bulk
Mime-Version: 1.0 (Mac OS X Mail 10.3 \(3273\))
Subject: Re: [PATCH] misc: add 2038 timestamp support
Date: Thu, 18 Apr 2024 16:04:10 -0600
In-Reply-To: <20240418143611.GA3373668@mit.edu>
Cc: Ext4 Developers List <linux-ext4@vger.kernel.org>
To: Theodore Ts'o <tytso@mit.edu>
References: <20230927054016.16645-1-adilger@dilger.ca>
 <20240418143611.GA3373668@mit.edu>


--Apple-Mail=_68ED42E6-2BA3-4024-850D-77F8474F0A11
Content-Transfer-Encoding: 7bit
Content-Type: text/plain;
	charset=us-ascii

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


Cheers, Andreas


--Apple-Mail=_68ED42E6-2BA3-4024-850D-77F8474F0A11
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
	filename=signature.asc
Content-Type: application/pgp-signature;
	name=signature.asc
Content-Description: Message signed with OpenPGP

-----BEGIN PGP SIGNATURE-----
Comment: GPGTools - http://gpgtools.org

iQIzBAEBCAAdFiEEDb73u6ZejP5ZMprvcqXauRfMH+AFAmYhmNoACgkQcqXauRfM
H+CkCg/+Ot8YN6S95JeGw2SEMbS18mNRFT92ilF4P1AfaWyJKquEDzzkB//J3rM0
7lB+qvEZ6kyrZTHfBoNZuFz+ogjBc9C2a4scX7jOyilOb4xgVM7DXBlxTeUSbx3p
I9EZCFjWOaI1k7yBVti1+pD0Hw8/rkU59ADTlzYH9qJlQvEWoMmXr8UJgEQYVA3Z
TAQ2sAVgAqmsKuH11fYoQMTghKg45LZpD7hMbjEmqCtc5ackcxx4buEFNbRlgdv2
iH9Si/fmWGBnSIeEBgS0+T5Gfz3+z886lSvldSpyBbsWtTRRVMGK742shUdlZQFA
wcUyP3il7pHGjBn4NGA3MlyiLVHT5lgOv9BeUrbGSLbUKPW0MiPwb59AP+XaQzgG
fg8NUdUhfrKX6dCmiN8por6sNNCkh4kFK7UU7BCdmZToZllsGLq6JTz1p0H6gAKS
bFRsGAGbvMq2+fEHLiwPp/rY3SSzUaRTM0+lbrwRVVsJNq0SVmkY5m7/GOWmlPPO
F3OCUtF5qCXI8b346jGKlRUYZZ01n8BekK0mXmIAYvW/hMEaPpBypE5VHJwW8TN1
ArGUbzrmv7N9Bkm58rL2P5oJOHfFnoiKbUeM02hND68kewIf1BoBmsT2I/NMkI51
T9OxpVynvR2RfbnUEKWOLkJpU4B+hyy+Nxn+08K9vfA+bPBbhVQ=
=zOzO
-----END PGP SIGNATURE-----

--Apple-Mail=_68ED42E6-2BA3-4024-850D-77F8474F0A11--