Re: [PATCH] hfs/hfsplus: fix timestamp wrapped issue

[Charalampos Mitrodimas <charmitro@posteo.net>]

Viacheslav Dubeyko <Slava.Dubeyko@ibm.com> writes:

> On Wed, 2026-02-18 at 02:00 +0000, Charalampos Mitrodimas wrote:
>> Viacheslav Dubeyko <Slava.Dubeyko@ibm.com> writes:
>> X-TUID: LHfQOjL/T+3i
>> 
>> > On Tue, 2026-02-17 at 02:39 +0000, Charalampos Mitrodimas wrote:
>> > > Viacheslav Dubeyko <slava@dubeyko.com> writes:
>> > > 
>> > > > The xfstests' test-case generic/258 fails to execute
>> > > > correctly:
>> > > > 
>> > > > FSTYP -- hfsplus
>> > > > PLATFORM -- Linux/x86_64 hfsplus-testing-0001 6.15.0-rc4+ #8 SMP PREEMPT_DYNAMIC Thu May 1 16:43:22 PDT 2025
>> > > > MKFS_OPTIONS -- /dev/loop51
>> > > > MOUNT_OPTIONS -- /dev/loop51 /mnt/scratch
>> > > > 
>> > > > generic/258 [failed, exit status 1]- output mismatch (see xfstests-dev/results//generic/258.out.bad)
>> > > > 
>> > > > The main reason of the issue is the logic:
>> > > > 
>> > > > cpu_to_be32(lower_32_bits(ut) + HFSPLUS_UTC_OFFSET)
>> > > > 
>> > > > At first, we take the lower 32 bits of the value and, then
>> > > > we add the time offset. However, if we have negative value
>> > > > then we make completely wrong calculation.
>> > > > 
>> > > > This patch corrects the logic of __hfsp_mt2ut() and
>> > > > __hfsp_ut2mt (HFS+ case), __hfs_m_to_utime() and
>> > > > __hfs_u_to_mtime (HFS case). The HFS_MIN_TIMESTAMP_SECS and
>> > > > HFS_MAX_TIMESTAMP_SECS have been introduced in
>> > > > include/linux/hfs_common.h. Also, HFS_UTC_OFFSET constant
>> > > > has been moved to include/linux/hfs_common.h. The hfs_fill_super()
>> > > > and hfsplus_fill_super() logic defines sb->s_time_min,
>> > > > sb->s_time_max, and sb->s_time_gran.
>> > > > 
>> > > > sudo ./check generic/258
>> > > > FSTYP         -- hfsplus
>> > > > PLATFORM      -- Linux/x86_64 hfsplus-testing-0001 6.19.0-rc1+ #87 SMP PREEMPT_DYNAMIC Mon Feb 16 14:48:57 PST 2026
>> > > > MKFS_OPTIONS  -- /dev/loop51
>> > > > MOUNT_OPTIONS -- /dev/loop51 /mnt/scratch
>> > > > 
>> > > > generic/258 29s ...  39s
>> > > > Ran: generic/258
>> > > > Passed all 1 tests
>> > > > 
>> > > > [1] https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_hfs-2Dlinux-2Dkernel_hfs-2Dlinux-2Dkernel_issues_133&d=DwIBAg&c=BSDicqBQBDjDI9RkVyTcHQ&r=q5bIm4AXMzc8NJu1_RGmnQ2fMWKq4Y4RAkElvUgSs00&m=0fT-uL56OPndiS3viO0tbIofDhce7l_DvqX2Ig5e11E9sRGSZHesLvgpGvaEGpvj&s=52rC3TXLKWz8arNKZMySDx-vwms5z-Md0bnvP6tGkEM&e= 
>> > > > 
>> > > > Signed-off-by: Viacheslav Dubeyko <slava@dubeyko.com>
>> > > > cc: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
>> > > > cc: Yangtao Li <frank.li@vivo.com>
>> > > > cc: linux-fsdevel@vger.kernel.org
>> > > > ---
>> > > >  fs/hfs/hfs_fs.h            | 17 ++++-------------
>> > > >  fs/hfs/super.c             |  4 ++++
>> > > >  fs/hfsplus/hfsplus_fs.h    | 13 ++++---------
>> > > >  fs/hfsplus/super.c         |  4 ++++
>> > > >  include/linux/hfs_common.h | 18 ++++++++++++++++++
>> > > >  5 files changed, 34 insertions(+), 22 deletions(-)
>> > > > 
>> > > > diff --git a/fs/hfs/hfs_fs.h b/fs/hfs/hfs_fs.h
>> > > > index ac0e83f77a0f..7d529e6789b8 100644
>> > > > --- a/fs/hfs/hfs_fs.h
>> > > > +++ b/fs/hfs/hfs_fs.h
>> > > > @@ -229,21 +229,11 @@ extern int hfs_mac2asc(struct super_block *sb,
>> > > >  extern void hfs_mark_mdb_dirty(struct super_block *sb);
>> > > >  
>> > > >  /*
>> > > > - * There are two time systems.  Both are based on seconds since
>> > > > - * a particular time/date.
>> > > > - *	Unix:	signed little-endian since 00:00 GMT, Jan. 1, 1970
>> > > > - *	mac:	unsigned big-endian since 00:00 GMT, Jan. 1, 1904
>> > > > - *
>> > > > - * HFS implementations are highly inconsistent, this one matches the
>> > > > - * traditional behavior of 64-bit Linux, giving the most useful
>> > > > - * time range between 1970 and 2106, by treating any on-disk timestamp
>> > > > - * under HFS_UTC_OFFSET (Jan 1 1970) as a time between 2040 and 2106.
>> > > > + * time helpers: convert between 1904-base and 1970-base timestamps
>> > > >   */
>> > > > -#define HFS_UTC_OFFSET 2082844800U
>> > > > -
>> > > >  static inline time64_t __hfs_m_to_utime(__be32 mt)
>> > > >  {
>> > > > -	time64_t ut = (u32)(be32_to_cpu(mt) - HFS_UTC_OFFSET);
>> > > > +	time64_t ut = (time64_t)be32_to_cpu(mt) - HFS_UTC_OFFSET;
>> > > >  
>> > > >  	return ut + sys_tz.tz_minuteswest * 60;
>> > > >  }
>> > > > @@ -251,8 +241,9 @@ static inline time64_t __hfs_m_to_utime(__be32 mt)
>> > > >  static inline __be32 __hfs_u_to_mtime(time64_t ut)
>> > > >  {
>> > > >  	ut -= sys_tz.tz_minuteswest * 60;
>> > > > +	ut += HFS_UTC_OFFSET;
>> > > >  
>> > > > -	return cpu_to_be32(lower_32_bits(ut) + HFS_UTC_OFFSET);
>> > > > +	return cpu_to_be32(lower_32_bits(ut));
>> > > >  }
>> > > >  #define HFS_I(inode)	(container_of(inode, struct hfs_inode_info, vfs_inode))
>> > > >  #define HFS_SB(sb)	((struct hfs_sb_info *)(sb)->s_fs_info)
>> > > > diff --git a/fs/hfs/super.c b/fs/hfs/super.c
>> > > > index 97546d6b41f4..6b6c138812b7 100644
>> > > > --- a/fs/hfs/super.c
>> > > > +++ b/fs/hfs/super.c
>> > > > @@ -341,6 +341,10 @@ static int hfs_fill_super(struct super_block *sb, struct fs_context *fc)
>> > > >  	sb->s_flags |= SB_NODIRATIME;
>> > > >  	mutex_init(&sbi->bitmap_lock);
>> > > >  
>> > > > +	sb->s_time_gran = NSEC_PER_SEC;
>> > > > +	sb->s_time_min = HFS_MIN_TIMESTAMP_SECS;
>> > > > +	sb->s_time_max = HFS_MAX_TIMESTAMP_SECS;
>> > > > +
>> > > >  	res = hfs_mdb_get(sb);
>> > > >  	if (res) {
>> > > >  		if (!silent)
>> > > > diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h
>> > > > index 5f891b73a646..3554faf84c15 100644
>> > > > --- a/fs/hfsplus/hfsplus_fs.h
>> > > > +++ b/fs/hfsplus/hfsplus_fs.h
>> > > > @@ -511,24 +511,19 @@ int hfsplus_read_wrapper(struct super_block *sb);
>> > > >  
>> > > >  /*
>> > > >   * time helpers: convert between 1904-base and 1970-base timestamps
>> > > > - *
>> > > > - * HFS+ implementations are highly inconsistent, this one matches the
>> > > > - * traditional behavior of 64-bit Linux, giving the most useful
>> > > > - * time range between 1970 and 2106, by treating any on-disk timestamp
>> > > > - * under HFSPLUS_UTC_OFFSET (Jan 1 1970) as a time between 2040 and 2106.
>> > > >   */
>> > > > -#define HFSPLUS_UTC_OFFSET 2082844800U
>> > > > -
>> > > >  static inline time64_t __hfsp_mt2ut(__be32 mt)
>> > > >  {
>> > > > -	time64_t ut = (u32)(be32_to_cpu(mt) - HFSPLUS_UTC_OFFSET);
>> > > > +	time64_t ut = (time64_t)be32_to_cpu(mt) - HFS_UTC_OFFSET;
>> > > >  
>> > > >  	return ut;
>> > > >  }
>> > > >  
>> > > >  static inline __be32 __hfsp_ut2mt(time64_t ut)
>> > > >  {
>> > > > -	return cpu_to_be32(lower_32_bits(ut) + HFSPLUS_UTC_OFFSET);
>> > > > +	ut += HFS_UTC_OFFSET;
>> > > > +
>> > > > +	return cpu_to_be32(lower_32_bits(ut));
>> > > >  }
>> > > >  
>> > > >  static inline enum hfsplus_btree_mutex_classes
>> > > > diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c
>> > > > index 592d8fbb748c..dcd61868d199 100644
>> > > > --- a/fs/hfsplus/super.c
>> > > > +++ b/fs/hfsplus/super.c
>> > > > @@ -487,6 +487,10 @@ static int hfsplus_fill_super(struct super_block *sb, struct fs_context *fc)
>> > > >  	if (!sbi->rsrc_clump_blocks)
>> > > >  		sbi->rsrc_clump_blocks = 1;
>> > > >  
>> > > > +	sb->s_time_gran = NSEC_PER_SEC;
>> > > > +	sb->s_time_min = HFS_MIN_TIMESTAMP_SECS;
>> > > > +	sb->s_time_max = HFS_MAX_TIMESTAMP_SECS;
>> > > > +
>> > > >  	err = -EFBIG;
>> > > >  	last_fs_block = sbi->total_blocks - 1;
>> > > >  	last_fs_page = (last_fs_block << sbi->alloc_blksz_shift) >>
>> > > > diff --git a/include/linux/hfs_common.h b/include/linux/hfs_common.h
>> > > > index dadb5e0aa8a3..816ac2f0996d 100644
>> > > > --- a/include/linux/hfs_common.h
>> > > > +++ b/include/linux/hfs_common.h
>> > > > @@ -650,4 +650,22 @@ typedef union {
>> > > >  	struct hfsplus_attr_key attr;
>> > > >  } __packed hfsplus_btree_key;
>> > > >  
>> > > > +/*
>> > > > + * There are two time systems.  Both are based on seconds since
>> > > > + * a particular time/date.
>> > > > + *	Unix:	signed little-endian since 00:00 GMT, Jan. 1, 1970
>> > > > + *	mac:	unsigned big-endian since 00:00 GMT, Jan. 1, 1904
>> > > > + *
>> > > > + * HFS/HFS+ implementations are highly inconsistent, this one matches the
>> > > > + * traditional behavior of 64-bit Linux, giving the most useful
>> > > > + * time range between 1970 and 2106, by treating any on-disk timestamp
>> > > > + * under HFS_UTC_OFFSET (Jan 1 1970) as a time between 2040 and 2106.
>> > > > + */
>> > > 
>> > > Since this is replacing the wrapping behavior with a linear 1904-2040
>> > > mapping, should we update this comment to match? It still describes the
>> > > old "2040 to 2106" wrapping semantics.
>> > > 
>> > 
>> > Frankly speaking, I don't quite follow what do you mean here. This patch doesn't
>> > change the approach. It simply fixes the incorrect calculation logic. Do you
>> > mean that this wrapping issue was the main approach? Currently, I don't see what
>> > needs to be updated in the comment.
>> 
>> Hi,
>> 
>> The comment says "time range between 1970 and 2106, by treating any
>> on-disk timestamp under HFS_UTC_OFFSET (Jan 1 1970) as a time between
>> 2040 and 2106". That was the old behavior via the (u32) cast.
>> 
>> Your patch changes (u32) to (time64_t) in __hfsp_mt2ut/__hfs_m_to_utime,
>> which removes that wrapping. For Mac time 0 (Jan 1, 1904):
>> 
>>   Old: (u32)     (0 - 2082844800) =  2212122496 -> 2040
>>   New: (time64_t) 0 - 2082844800  = -2082844800 -> 1904
>> 
>> The new s_time_min/s_time_max also confirm the range is now 1904-2040,
>> not 1970-2106. So the comment no longer matches the code.
>> 
>
> OK. I see your point. So, we cannot execute the wrong calculation for timestamps
> that are less than 1970. But it will be good to support the trick related to
> 1970-2106. How can we improve the patch? What's your suggestion?

Well... the wrapping was the bug that broke generic/258 right? So trying
to keep it would just reintroduce the problem. AFAIK since the on-disk
timestamp is a 32bit unsigned counter from 1904 it tops out at
2040. There are no spare bits to extend the range.

The only way I can think of to support both would be a mount option that
lets the user pick the mapping, something like "timerange=1904" and
"timerange=1970". But that will introduce a lot of complexities.

IMO patch is correct, just the comment needs updating to say 1904-2040
instead of 1970-2106.

Cheers,
C. Mitrodimas