From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mout01.posteo.de (mout01.posteo.de [185.67.36.65]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 569471FC0EA for ; Thu, 19 Feb 2026 00:44:36 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=185.67.36.65 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1771461878; cv=none; b=Fhj/e6PTr9iOWn8KpMcLfV4PmaFKhsrvpyVqbVFB6l/LzGAMw682GKanvbwnknvUTkZsB1jS3BWMNxK9nJpVnSvHp1zWMGYu30cbKXvsFarWaoXoGu0LLUiNPsRo3h0zY34TaJ91saSeN8vOcjv6fo0vxEMBfQBqTjq0UztsJ8w= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1771461878; c=relaxed/simple; bh=nQH7jYjmNyHUpdEDL1n3cOERApWtdjO11tQwfD3xwEY=; h=From:To:Cc:Subject:In-Reply-To:References:Date:Message-ID: MIME-Version:Content-Type; b=W4g+u4ee0Ohsvz3urHp7VdoAIWxkr0a8YwcBc3wHjd9pzqPSUtwRYCT2lXP7RQKOnHp2tAC5gWbqb0yg0srWv2JNC3F1nAaoCXCxiSmVzodHSLnLpWro2AD/LMFgzaP7M4Xf6URED5C4XK1uozuI7C4s0LhJTZCNVXslIPwo8Cc= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=posteo.net; spf=pass smtp.mailfrom=posteo.net; dkim=pass (2048-bit key) header.d=posteo.net header.i=@posteo.net header.b=EEeFZU7m; arc=none smtp.client-ip=185.67.36.65 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=posteo.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=posteo.net Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=posteo.net header.i=@posteo.net header.b="EEeFZU7m" Received: from submission (posteo.de [185.67.36.169]) by mout01.posteo.de (Postfix) with ESMTPS id 2BEA3240027 for ; Thu, 19 Feb 2026 01:44:27 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=posteo.net; s=2017; t=1771461868; bh=BsfF26EI6S3qTr6+RYJIEACJrK310ChY8MDFoZGqNt4=; h=From:To:Cc:Subject:Date:Message-ID:MIME-Version:Content-Type: From; b=EEeFZU7mXoijP2neLjfv2lplNp5L7BPIzzyKIB0FXSglMCd267IlBdYsv0BZ50orp ak5W6Z+cDlmYzpUZel2fgkq+RYbNA+kL14nuzYtSOAx0DzETBeoCewoN36XmycEN61 LNHL7y7qFJxa2fdRW9rT4mt2qS2WpKEEJ2MJK5O10Zlp/BB3d4VTPDtheDetU1/udr 7sAFLXGrjvAaSLThshOqWEsIp8Iseh/8T//iB6B50GDl5jJ96tkS4J71BOn/WITQOs sPrTKVT001HsqBhTXAnvI4dG2HJpAAdYv1KU5WZlMXf05KFg5hgOToqaChcjPSupaW JNnod+AbcC2IQ== Received: from customer (localhost [127.0.0.1]) by submission (posteo.de) with ESMTPSA id 4fGZQL1Rntz6tsb; Thu, 19 Feb 2026 01:44:25 +0100 (CET) From: Charalampos Mitrodimas To: Viacheslav Dubeyko Cc: "glaubitz@physik.fu-berlin.de" , "frank.li@vivo.com" , "slava@dubeyko.com" , "linux-fsdevel@vger.kernel.org" Subject: Re: [PATCH] hfs/hfsplus: fix timestamp wrapped issue In-Reply-To: <15b8136f279abd8320e2d4745a4f1e76c9f9aa83.camel@ibm.com> References: <20260216233556.4005400-1-slava@dubeyko.com> <87a4x8f5zq.fsf@posteo.net> <2b7b7a970926f56a3742cb76e394e9fb3d79b0eb.camel@ibm.com> <15b8136f279abd8320e2d4745a4f1e76c9f9aa83.camel@ibm.com> Date: Thu, 19 Feb 2026 00:44:27 +0000 Message-ID: <87jyw9blzq.fsf@posteo.net> Precedence: bulk X-Mailing-List: linux-fsdevel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain Viacheslav Dubeyko writes: > On Wed, 2026-02-18 at 02:00 +0000, Charalampos Mitrodimas wrote: >> Viacheslav Dubeyko writes: >> X-TUID: LHfQOjL/T+3i >> >> > On Tue, 2026-02-17 at 02:39 +0000, Charalampos Mitrodimas wrote: >> > > Viacheslav Dubeyko 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 >> > > > cc: John Paul Adrian Glaubitz >> > > > cc: Yangtao Li >> > > > 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