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

[Charalampos Mitrodimas <charmitro@posteo.net>]

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://github.com/hfs-linux-kernel/hfs-linux-kernel/issues/133
>
> 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.

Cheers,
C. Mitrodimas

> +#define HFS_UTC_OFFSET 2082844800U
> +
> +/* January 1, 1904, 00:00:00 UTC */
> +#define HFS_MIN_TIMESTAMP_SECS		-2082844800LL
> +/* February 6, 2040, 06:28:15 UTC */
> +#define HFS_MAX_TIMESTAMP_SECS		2212122495LL
> +
>  #endif /* _HFS_COMMON_H_ */