[PATCH 1/3] hfs: stop using timespec based interfaces

[PATCH 1/3] hfs: stop using timespec based interfaces

[Arnd Bergmann]

The native HFS timestamps overflow in year 2040, two years after the Unix
y2038 overflow. However, the way that the conversion between on-disk
timestamps and in-kernel timestamps was implemented, 64-bit machines
actually ended up converting negative UTC timestamps (1902 through 1969)
into times between 2038 and 2106.

Rather than making all machines faithfully represent timestamps in the
ancient past but break after 2040, this changes the file system to
always use the unsigned UTC interpretation, reading back times between
1970 and 2106.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 fs/hfs/hfs_fs.h | 6 ++++--
 fs/hfs/inode.c  | 4 ++--
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/fs/hfs/hfs_fs.h b/fs/hfs/hfs_fs.h
index 6d0783e2e276..ff432931a5b1 100644
--- a/fs/hfs/hfs_fs.h
+++ b/fs/hfs/hfs_fs.h
@@ -245,6 +245,8 @@ extern void hfs_mark_mdb_dirty(struct super_block *sb);
  *	Unix:	unsigned lil-endian since 00:00 GMT, Jan. 1, 1970
  *	mac:	unsigned big-endian since 00:00 GMT, Jan. 1, 1904
  *
+ * We treat all timestamps before 1970 as times after 2038, so this
+ * actually works until year 2106
  */
 #define __hfs_u_to_mtime(sec)	cpu_to_be32(sec + 2082844800U - sys_tz.tz_minuteswest * 60)
 #define __hfs_m_to_utime(sec)	(be32_to_cpu(sec) - 2082844800U  + sys_tz.tz_minuteswest * 60)
@@ -252,9 +254,9 @@ extern void hfs_mark_mdb_dirty(struct super_block *sb);
 #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)
 
-#define hfs_m_to_utime(time)	(struct timespec){ .tv_sec = __hfs_m_to_utime(time) }
+#define hfs_m_to_utime(time)	(struct timespec64){ .tv_sec = __hfs_m_to_utime(time) }
 #define hfs_u_to_mtime(time)	__hfs_u_to_mtime((time).tv_sec)
-#define hfs_mtime()		__hfs_u_to_mtime(get_seconds())
+#define hfs_mtime()		__hfs_u_to_mtime(ktime_get_real_seconds())
 
 static inline const char *hfs_mdb_name(struct super_block *sb)
 {
diff --git a/fs/hfs/inode.c b/fs/hfs/inode.c
index 2a16111d312f..b3309b83371a 100644
--- a/fs/hfs/inode.c
+++ b/fs/hfs/inode.c
@@ -351,7 +351,7 @@ static int hfs_read_inode(struct inode *inode, void *data)
 		inode->i_mode &= ~hsb->s_file_umask;
 		inode->i_mode |= S_IFREG;
 		inode->i_ctime = inode->i_atime = inode->i_mtime =
-				timespec_to_timespec64(hfs_m_to_utime(rec->file.MdDat));
+				hfs_m_to_utime(rec->file.MdDat);
 		inode->i_op = &hfs_file_inode_operations;
 		inode->i_fop = &hfs_file_operations;
 		inode->i_mapping->a_ops = &hfs_aops;
@@ -362,7 +362,7 @@ static int hfs_read_inode(struct inode *inode, void *data)
 		HFS_I(inode)->fs_blocks = 0;
 		inode->i_mode = S_IFDIR | (S_IRWXUGO & ~hsb->s_dir_umask);
 		inode->i_ctime = inode->i_atime = inode->i_mtime =
-				timespec_to_timespec64(hfs_m_to_utime(rec->dir.MdDat));
+				hfs_m_to_utime(rec->dir.MdDat);
 		inode->i_op = &hfs_dir_inode_operations;
 		inode->i_fop = &hfs_dir_operations;
 		break;
-- 
2.9.0

[PATCH 2/3] hfsplus: stop using timespec based interfaces

[Arnd Bergmann]

The native HFS+ timestamps overflow in year 2040, two years after the Unix
y2038 overflow. However, the way that the conversion between on-disk
timestamps and in-kernel timestamps was implemented, 64-bit machines
actually ended up converting negative UTC timestamps (1902 through 1969)
into times between 2038 and 2106.

Rather than making all machines faithfully represent timestamps in the
ancient past but break after 2040, this changes the file system to
always use the unsigned UTC interpretation, reading back times between
1970 and 2106.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 fs/hfsplus/hfsplus_fs.h |  7 ++++---
 fs/hfsplus/inode.c      | 12 ++++++------
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h
index d9255abafb81..646c207be38d 100644
--- a/fs/hfsplus/hfsplus_fs.h
+++ b/fs/hfsplus/hfsplus_fs.h
@@ -530,13 +530,14 @@ int hfsplus_submit_bio(struct super_block *sb, sector_t sector, void *buf,
 		       void **data, int op, int op_flags);
 int hfsplus_read_wrapper(struct super_block *sb);
 
-/* time macros */
+/* time macros: convert between 1904-2040 and 1970-2106 range,
+ * pre-1970 timestamps are interpreted as post-2038 times after wrap-around */
 #define __hfsp_mt2ut(t)		(be32_to_cpu(t) - 2082844800U)
 #define __hfsp_ut2mt(t)		(cpu_to_be32(t + 2082844800U))
 
 /* compatibility */
-#define hfsp_mt2ut(t)		(struct timespec){ .tv_sec = __hfsp_mt2ut(t) }
+#define hfsp_mt2ut(t)		(struct timespec64){ .tv_sec = __hfsp_mt2ut(t) }
 #define hfsp_ut2mt(t)		__hfsp_ut2mt((t).tv_sec)
-#define hfsp_now2mt()		__hfsp_ut2mt(get_seconds())
+#define hfsp_now2mt()		__hfsp_ut2mt(ktime_get_real_seconds())
 
 #endif
diff --git a/fs/hfsplus/inode.c b/fs/hfsplus/inode.c
index c824f702feec..c0c8d433864f 100644
--- a/fs/hfsplus/inode.c
+++ b/fs/hfsplus/inode.c
@@ -493,9 +493,9 @@ int hfsplus_cat_read_inode(struct inode *inode, struct hfs_find_data *fd)
 		hfsplus_get_perms(inode, &folder->permissions, 1);
 		set_nlink(inode, 1);
 		inode->i_size = 2 + be32_to_cpu(folder->valence);
-		inode->i_atime = timespec_to_timespec64(hfsp_mt2ut(folder->access_date));
-		inode->i_mtime = timespec_to_timespec64(hfsp_mt2ut(folder->content_mod_date));
-		inode->i_ctime = timespec_to_timespec64(hfsp_mt2ut(folder->attribute_mod_date));
+		inode->i_atime = hfsp_mt2ut(folder->access_date);
+		inode->i_mtime = hfsp_mt2ut(folder->content_mod_date);
+		inode->i_ctime = hfsp_mt2ut(folder->attribute_mod_date);
 		HFSPLUS_I(inode)->create_date = folder->create_date;
 		HFSPLUS_I(inode)->fs_blocks = 0;
 		if (folder->flags & cpu_to_be16(HFSPLUS_HAS_FOLDER_COUNT)) {
@@ -531,9 +531,9 @@ int hfsplus_cat_read_inode(struct inode *inode, struct hfs_find_data *fd)
 			init_special_inode(inode, inode->i_mode,
 					   be32_to_cpu(file->permissions.dev));
 		}
-		inode->i_atime = timespec_to_timespec64(hfsp_mt2ut(file->access_date));
-		inode->i_mtime = timespec_to_timespec64(hfsp_mt2ut(file->content_mod_date));
-		inode->i_ctime = timespec_to_timespec64(hfsp_mt2ut(file->attribute_mod_date));
+		inode->i_atime = hfsp_mt2ut(file->access_date);
+		inode->i_mtime = hfsp_mt2ut(file->content_mod_date);
+		inode->i_ctime = hfsp_mt2ut(file->attribute_mod_date);
 		HFSPLUS_I(inode)->create_date = file->create_date;
 	} else {
 		pr_err("bad catalog entry used to create inode\n");
-- 
2.9.0

Re: [PATCH 2/3] hfsplus: stop using timespec based interfaces

[Ernesto A. Fernández]

On Tue, Jun 19, 2018 at 06:02:08PM +0200, Arnd Bergmann wrote:
> The native HFS+ timestamps overflow in year 2040, two years after the Unix
> y2038 overflow. However, the way that the conversion between on-disk
> timestamps and in-kernel timestamps was implemented, 64-bit machines
> actually ended up converting negative UTC timestamps (1902 through 1969)
> into times between 2038 and 2106.
> 
> Rather than making all machines faithfully represent timestamps in the
> ancient past but break after 2040, this changes the file system to
> always use the unsigned UTC interpretation, reading back times between
> 1970 and 2106.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  fs/hfsplus/hfsplus_fs.h |  7 ++++---
>  fs/hfsplus/inode.c      | 12 ++++++------
>  2 files changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h
> index d9255abafb81..646c207be38d 100644
> --- a/fs/hfsplus/hfsplus_fs.h
> +++ b/fs/hfsplus/hfsplus_fs.h
> @@ -530,13 +530,14 @@ int hfsplus_submit_bio(struct super_block *sb, sector_t sector, void *buf,
>  		       void **data, int op, int op_flags);
>  int hfsplus_read_wrapper(struct super_block *sb);
>  
> -/* time macros */
> +/* time macros: convert between 1904-2040 and 1970-2106 range,
> + * pre-1970 timestamps are interpreted as post-2038 times after wrap-around
						  ^^^^
If we are talking about on-disk timestamps, this should be 2040.


Everything else looks fine, if it helps you can use:

Reviewed-by: Ernesto A. Fernández <ernesto.mnd.fernandez@gmail.com>

> */
>  #define __hfsp_mt2ut(t)		(be32_to_cpu(t) - 2082844800U)
>  #define __hfsp_ut2mt(t)		(cpu_to_be32(t + 2082844800U))
>  
>  /* compatibility */
> -#define hfsp_mt2ut(t)		(struct timespec){ .tv_sec = __hfsp_mt2ut(t) }
> +#define hfsp_mt2ut(t)		(struct timespec64){ .tv_sec = __hfsp_mt2ut(t) }
>  #define hfsp_ut2mt(t)		__hfsp_ut2mt((t).tv_sec)
> -#define hfsp_now2mt()		__hfsp_ut2mt(get_seconds())
> +#define hfsp_now2mt()		__hfsp_ut2mt(ktime_get_real_seconds())
>  
>  #endif
> diff --git a/fs/hfsplus/inode.c b/fs/hfsplus/inode.c
> index c824f702feec..c0c8d433864f 100644
> --- a/fs/hfsplus/inode.c
> +++ b/fs/hfsplus/inode.c
> @@ -493,9 +493,9 @@ int hfsplus_cat_read_inode(struct inode *inode, struct hfs_find_data *fd)
>  		hfsplus_get_perms(inode, &folder->permissions, 1);
>  		set_nlink(inode, 1);
>  		inode->i_size = 2 + be32_to_cpu(folder->valence);
> -		inode->i_atime = timespec_to_timespec64(hfsp_mt2ut(folder->access_date));
> -		inode->i_mtime = timespec_to_timespec64(hfsp_mt2ut(folder->content_mod_date));
> -		inode->i_ctime = timespec_to_timespec64(hfsp_mt2ut(folder->attribute_mod_date));
> +		inode->i_atime = hfsp_mt2ut(folder->access_date);
> +		inode->i_mtime = hfsp_mt2ut(folder->content_mod_date);
> +		inode->i_ctime = hfsp_mt2ut(folder->attribute_mod_date);
>  		HFSPLUS_I(inode)->create_date = folder->create_date;
>  		HFSPLUS_I(inode)->fs_blocks = 0;
>  		if (folder->flags & cpu_to_be16(HFSPLUS_HAS_FOLDER_COUNT)) {
> @@ -531,9 +531,9 @@ int hfsplus_cat_read_inode(struct inode *inode, struct hfs_find_data *fd)
>  			init_special_inode(inode, inode->i_mode,
>  					   be32_to_cpu(file->permissions.dev));
>  		}
> -		inode->i_atime = timespec_to_timespec64(hfsp_mt2ut(file->access_date));
> -		inode->i_mtime = timespec_to_timespec64(hfsp_mt2ut(file->content_mod_date));
> -		inode->i_ctime = timespec_to_timespec64(hfsp_mt2ut(file->attribute_mod_date));
> +		inode->i_atime = hfsp_mt2ut(file->access_date);
> +		inode->i_mtime = hfsp_mt2ut(file->content_mod_date);
> +		inode->i_ctime = hfsp_mt2ut(file->attribute_mod_date);
>  		HFSPLUS_I(inode)->create_date = file->create_date;
>  	} else {
>  		pr_err("bad catalog entry used to create inode\n");
> -- 
> 2.9.0
> 

[PATCH 3/3] hfsplus: return inode birthtime for statx

[Arnd Bergmann]

We have the data in the kernel, so we might just as well provide it to
user space.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 fs/hfsplus/dir.c        |  1 +
 fs/hfsplus/hfsplus_fs.h |  2 ++
 fs/hfsplus/inode.c      | 14 ++++++++++++++
 3 files changed, 17 insertions(+)

diff --git a/fs/hfsplus/dir.c b/fs/hfsplus/dir.c
index b5254378f011..df14b6dd5b5a 100644
--- a/fs/hfsplus/dir.c
+++ b/fs/hfsplus/dir.c
@@ -566,6 +566,7 @@ const struct inode_operations hfsplus_dir_inode_operations = {
 	.symlink		= hfsplus_symlink,
 	.mknod			= hfsplus_mknod,
 	.rename			= hfsplus_rename,
+	.getattr		= hfsplus_getattr,
 	.listxattr		= hfsplus_listxattr,
 #ifdef CONFIG_HFSPLUS_FS_POSIX_ACL
 	.get_acl		= hfsplus_get_posix_acl,
diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h
index 646c207be38d..1a6b469f8d22 100644
--- a/fs/hfsplus/hfsplus_fs.h
+++ b/fs/hfsplus/hfsplus_fs.h
@@ -489,6 +489,8 @@ int hfsplus_cat_read_inode(struct inode *inode, struct hfs_find_data *fd);
 int hfsplus_cat_write_inode(struct inode *inode);
 int hfsplus_file_fsync(struct file *file, loff_t start, loff_t end,
 		       int datasync);
+int hfsplus_getattr(const struct path *path, struct kstat *stat,
+		    u32 request_mask, unsigned int query_flags);
 
 /* ioctl.c */
 long hfsplus_ioctl(struct file *filp, unsigned int cmd, unsigned long arg);
diff --git a/fs/hfsplus/inode.c b/fs/hfsplus/inode.c
index c0c8d433864f..f9bb568f9479 100644
--- a/fs/hfsplus/inode.c
+++ b/fs/hfsplus/inode.c
@@ -276,6 +276,19 @@ static int hfsplus_setattr(struct dentry *dentry, struct iattr *attr)
 	return 0;
 }
 
+int hfsplus_getattr(const struct path *path, struct kstat *stat,
+		     u32 request_mask, unsigned int query_flags)
+{
+	struct inode *inode = d_backing_inode(path->dentry);
+
+	generic_fillattr(inode, stat);
+
+	stat->btime = hfsp_mt2ut(HFSPLUS_I(inode)->create_date);
+	stat->result_mask |= STATX_BTIME;
+
+	return 0;
+}
+
 int hfsplus_file_fsync(struct file *file, loff_t start, loff_t end,
 		       int datasync)
 {
@@ -335,6 +348,7 @@ int hfsplus_file_fsync(struct file *file, loff_t start, loff_t end,
 
 static const struct inode_operations hfsplus_file_inode_operations = {
 	.setattr	= hfsplus_setattr,
+	.getattr	= hfsplus_getattr,
 	.listxattr	= hfsplus_listxattr,
 #ifdef CONFIG_HFSPLUS_FS_POSIX_ACL
 	.get_acl	= hfsplus_get_posix_acl,
-- 
2.9.0

Re: [PATCH 3/3] hfsplus: return inode birthtime for statx

[Ernesto A. Fernández]

Hi:

On Tue, Jun 19, 2018 at 06:02:09PM +0200, Arnd Bergmann wrote:
> We have the data in the kernel, so we might just as well provide it to
> user space.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  fs/hfsplus/dir.c        |  1 +
>  fs/hfsplus/hfsplus_fs.h |  2 ++
>  fs/hfsplus/inode.c      | 14 ++++++++++++++
>  3 files changed, 17 insertions(+)
> 
> diff --git a/fs/hfsplus/dir.c b/fs/hfsplus/dir.c
> index b5254378f011..df14b6dd5b5a 100644
> --- a/fs/hfsplus/dir.c
> +++ b/fs/hfsplus/dir.c
> @@ -566,6 +566,7 @@ const struct inode_operations hfsplus_dir_inode_operations = {
>  	.symlink		= hfsplus_symlink,
>  	.mknod			= hfsplus_mknod,
>  	.rename			= hfsplus_rename,
> +	.getattr		= hfsplus_getattr,
>  	.listxattr		= hfsplus_listxattr,
>  #ifdef CONFIG_HFSPLUS_FS_POSIX_ACL
>  	.get_acl		= hfsplus_get_posix_acl,
> diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h
> index 646c207be38d..1a6b469f8d22 100644
> --- a/fs/hfsplus/hfsplus_fs.h
> +++ b/fs/hfsplus/hfsplus_fs.h
> @@ -489,6 +489,8 @@ int hfsplus_cat_read_inode(struct inode *inode, struct hfs_find_data *fd);
>  int hfsplus_cat_write_inode(struct inode *inode);
>  int hfsplus_file_fsync(struct file *file, loff_t start, loff_t end,
>  		       int datasync);
> +int hfsplus_getattr(const struct path *path, struct kstat *stat,
> +		    u32 request_mask, unsigned int query_flags);
>  
>  /* ioctl.c */
>  long hfsplus_ioctl(struct file *filp, unsigned int cmd, unsigned long arg);
> diff --git a/fs/hfsplus/inode.c b/fs/hfsplus/inode.c
> index c0c8d433864f..f9bb568f9479 100644
> --- a/fs/hfsplus/inode.c
> +++ b/fs/hfsplus/inode.c
> @@ -276,6 +276,19 @@ static int hfsplus_setattr(struct dentry *dentry, struct iattr *attr)
>  	return 0;
>  }
>  
> +int hfsplus_getattr(const struct path *path, struct kstat *stat,
> +		     u32 request_mask, unsigned int query_flags)
> +{
> +	struct inode *inode = d_backing_inode(path->dentry);

I think d_inode() is better. They work the same, but "normal filesystems
should not use this", according to the d_backing_inode() documentation.

> +
> +	generic_fillattr(inode, stat);
> +
> +	stat->btime = hfsp_mt2ut(HFSPLUS_I(inode)->create_date);
> +	stat->result_mask |= STATX_BTIME;
> +
> +	return 0;
> +}
> +
>  int hfsplus_file_fsync(struct file *file, loff_t start, loff_t end,
>  		       int datasync)
>  {
> @@ -335,6 +348,7 @@ int hfsplus_file_fsync(struct file *file, loff_t start, loff_t end,
>  
>  static const struct inode_operations hfsplus_file_inode_operations = {
>  	.setattr	= hfsplus_setattr,
> +	.getattr	= hfsplus_getattr,
>  	.listxattr	= hfsplus_listxattr,
>  #ifdef CONFIG_HFSPLUS_FS_POSIX_ACL
>  	.get_acl	= hfsplus_get_posix_acl,
> -- 
> 2.9.0
> 

What about symlinks and special files?

Thanks,
Ernest

Re: [PATCH 3/3] hfsplus: return inode birthtime for statx

[Arnd Bergmann]

On Thu, Jun 21, 2018 at 12:45 AM, Ernesto A. Fernández
<ernesto.mnd.fernandez@gmail.com> wrote:

>> --- a/fs/hfsplus/inode.c
>> +++ b/fs/hfsplus/inode.c
>> @@ -276,6 +276,19 @@ static int hfsplus_setattr(struct dentry *dentry, struct iattr *attr)
>>       return 0;
>>  }
>>
>> +int hfsplus_getattr(const struct path *path, struct kstat *stat,
>> +                  u32 request_mask, unsigned int query_flags)
>> +{
>> +     struct inode *inode = d_backing_inode(path->dentry);
>
> I think d_inode() is better. They work the same, but "normal filesystems
> should not use this", according to the d_backing_inode() documentation.
>

Right, definitely. I copied it from vfs_getattr_nosec() without thinking about
it much. I see how David Howells put that d_backing_inode() there, but
still don't understand it.

>> @@ -335,6 +348,7 @@ int hfsplus_file_fsync(struct file *file, loff_t start, loff_t end,
>>
>>  static const struct inode_operations hfsplus_file_inode_operations = {
>>       .setattr        = hfsplus_setattr,
>> +     .getattr        = hfsplus_getattr,
>>       .listxattr      = hfsplus_listxattr,
>>  #ifdef CONFIG_HFSPLUS_FS_POSIX_ACL
>>       .get_acl        = hfsplus_get_posix_acl,
>> --
>> 2.9.0
>>
>
> What about symlinks and special files?

My mistake again, thanks for pointing that out. Doing the symlinks correctly
here would actually add a bit more complexity as they use the generic
page_symlink_inode_operations at the moment.

I think I'd rather just retract this patch and let someone else handle it if
they actually want this feature. I only added it because it seemed trivial
to do, but that was clearly not true. ;-)

       Arnd

Re: [PATCH 1/3] hfs: stop using timespec based interfaces

[Viacheslav Dubeyko]

On Tue, 2018-06-19 at 18:02 +0200, Arnd Bergmann wrote:
> The native HFS timestamps overflow in year 2040, two years after the
> Unix
> y2038 overflow. However, the way that the conversion between on-disk
> timestamps and in-kernel timestamps was implemented, 64-bit machines
> actually ended up converting negative UTC timestamps (1902 through
> 1969)
> into times between 2038 and 2106.
> 
> Rather than making all machines faithfully represent timestamps in
> the
> ancient past but break after 2040, this changes the file system to
> always use the unsigned UTC interpretation, reading back times
> between
> 1970 and 2106.
> 

The trouble with HFS and HFS+ that the specification [1] declares this:

"HFS Plus stores dates in several data structures, including the volume
header and catalog records. These dates are stored in unsigned 32-bit
integers (UInt32) containing the number of seconds since midnight,
January 1, 1904, GMT. This is slightly different from HFS, where the
value represents local time. The maximum representable date is February
6, 2040 at 06:28:15 GMT."

So, I am not sure that we are able to support later dates because such
timestamps cannot be stored on HFS/HFS+ volumes and will be
incompatible with Mac OS X. Also, I am not sure that anybody will use
HFS/HFS+ after 2040.

Thanks,
Vyacheslav Dubeyko.

[1] https://developer.apple.com/library/archive/technotes/tn/tn1150.html


> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  fs/hfs/hfs_fs.h | 6 ++++--
>  fs/hfs/inode.c  | 4 ++--
>  2 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/hfs/hfs_fs.h b/fs/hfs/hfs_fs.h
> index 6d0783e2e276..ff432931a5b1 100644
> --- a/fs/hfs/hfs_fs.h
> +++ b/fs/hfs/hfs_fs.h
> @@ -245,6 +245,8 @@ extern void hfs_mark_mdb_dirty(struct super_block
> *sb);
>   *	Unix:	unsigned lil-endian since 00:00 GMT, Jan. 1,
> 1970
>   *	mac:	unsigned big-endian since 00:00 GMT, Jan. 1,
> 1904
>   *
> + * We treat all timestamps before 1970 as times after 2038, so this
> + * actually works until year 2106
>   */
>  #define __hfs_u_to_mtime(sec)	cpu_to_be32(sec + 2082844800U -
> sys_tz.tz_minuteswest * 60)
>  #define __hfs_m_to_utime(sec)	(be32_to_cpu(sec) -
> 2082844800U  + sys_tz.tz_minuteswest * 60)
> @@ -252,9 +254,9 @@ extern void hfs_mark_mdb_dirty(struct super_block
> *sb);
>  #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)
>  
> -#define hfs_m_to_utime(time)	(struct timespec){ .tv_sec =
> __hfs_m_to_utime(time) }
> +#define hfs_m_to_utime(time)	(struct timespec64){ .tv_sec =
> __hfs_m_to_utime(time) }
>  #define hfs_u_to_mtime(time)	__hfs_u_to_mtime((time).tv_sec)
> -#define hfs_mtime()		__hfs_u_to_mtime(get_seconds())
> +#define hfs_mtime()		__hfs_u_to_mtime(ktime_get_real_s
> econds())
>  
>  static inline const char *hfs_mdb_name(struct super_block *sb)
>  {
> diff --git a/fs/hfs/inode.c b/fs/hfs/inode.c
> index 2a16111d312f..b3309b83371a 100644
> --- a/fs/hfs/inode.c
> +++ b/fs/hfs/inode.c
> @@ -351,7 +351,7 @@ static int hfs_read_inode(struct inode *inode,
> void *data)
>  		inode->i_mode &= ~hsb->s_file_umask;
>  		inode->i_mode |= S_IFREG;
>  		inode->i_ctime = inode->i_atime = inode->i_mtime =
> -				timespec_to_timespec64(hfs_m_to_utim
> e(rec->file.MdDat));
> +				hfs_m_to_utime(rec->file.MdDat);
>  		inode->i_op = &hfs_file_inode_operations;
>  		inode->i_fop = &hfs_file_operations;
>  		inode->i_mapping->a_ops = &hfs_aops;
> @@ -362,7 +362,7 @@ static int hfs_read_inode(struct inode *inode,
> void *data)
>  		HFS_I(inode)->fs_blocks = 0;
>  		inode->i_mode = S_IFDIR | (S_IRWXUGO & ~hsb-
> >s_dir_umask);
>  		inode->i_ctime = inode->i_atime = inode->i_mtime =
> -				timespec_to_timespec64(hfs_m_to_utim
> e(rec->dir.MdDat));
> +				hfs_m_to_utime(rec->dir.MdDat);
>  		inode->i_op = &hfs_dir_inode_operations;
>  		inode->i_fop = &hfs_dir_operations;
>  		break;

Re: [PATCH 1/3] hfs: stop using timespec based interfaces

[Arnd Bergmann]

On Tue, Jun 19, 2018 at 7:03 PM, Viacheslav Dubeyko <slava@dubeyko.com> wrote:
> On Tue, 2018-06-19 at 18:02 +0200, Arnd Bergmann wrote:
>> The native HFS timestamps overflow in year 2040, two years after the
>> Unix
>> y2038 overflow. However, the way that the conversion between on-disk
>> timestamps and in-kernel timestamps was implemented, 64-bit machines
>> actually ended up converting negative UTC timestamps (1902 through
>> 1969)
>> into times between 2038 and 2106.
>>
>> Rather than making all machines faithfully represent timestamps in
>> the
>> ancient past but break after 2040, this changes the file system to
>> always use the unsigned UTC interpretation, reading back times
>> between
>> 1970 and 2106.
>>
>
> The trouble with HFS and HFS+ that the specification [1] declares this:
>
> "HFS Plus stores dates in several data structures, including the volume
> header and catalog records. These dates are stored in unsigned 32-bit
> integers (UInt32) containing the number of seconds since midnight,
> January 1, 1904, GMT. This is slightly different from HFS, where the
> value represents local time. The maximum representable date is February
> 6, 2040 at 06:28:15 GMT."
>
> So, I am not sure that we are able to support later dates because such
> timestamps cannot be stored on HFS/HFS+ volumes and will be
> incompatible with Mac OS X.

We never followed that interpretation in Linux though. As I wrote,
on 64-bit machines, these two calculations (hfs and hfs+,
respectively)

#define __hfs_m_to_utime(sec)   (be32_to_cpu(sec) - 2082844800U  +
sys_tz.tz_minuteswest * 60)
#define __hfsp_mt2ut(t)                (be32_to_cpu(t) - 2082844800U)

just wrap around when reading the timestamps before 1970 from
disk. On 32-bit machines they get wrapped another time when
we assign them to a signed 32-bit time_t.

> Also, I am not sure that anybody will use HFS/HFS+ after 2040.

I'm trying to fix all file systems to be unambiguous regarding
inode timestamps. This means it should behave the same way
on 32-bit and 64-bit kernels, and if possible in a sane way.

Even if you don't care about running HFS in the future, you
can trivially create files with arbitrary timestamps, just try

touch -d "Jan 1 1901" 1901
touch -d "Jan 1 1905" 1905
touch -d "Jan 1 1969" 1969
touch -d "Jan 1 2038" 2038
touch -d "Jan 1 2040" 2040
touch -d "Jan 1 2106" 2106
touch -d "Jan 1 2107" 2107

on HFS and do an 'ls -l' after an unmount/remount.

If you think it's important that we change the current behavior
to be compatible with MacOS and represent the 1904..2040
time range rather than 1970..2106, we can definitely do that
as well, using this patch:

diff --git a/fs/hfs/hfs_fs.h b/fs/hfs/hfs_fs.h
index ff432931a5b1..2c7366342656 100644
--- a/fs/hfs/hfs_fs.h
+++ b/fs/hfs/hfs_fs.h
@@ -249,7 +249,7 @@ extern void hfs_mark_mdb_dirty(struct super_block *sb);
  * actually works until year 2106
  */
 #define __hfs_u_to_mtime(sec)  cpu_to_be32(sec + 2082844800U -
sys_tz.tz_minuteswest * 60)
-#define __hfs_m_to_utime(sec)  (be32_to_cpu(sec) - 2082844800U  +
sys_tz.tz_minuteswest * 60)
+#define __hfs_m_to_utime(sec)  ((time64_t)be32_to_cpu(sec) -
2082844800U  + sys_tz.tz_minuteswest * 60)

 #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/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h
index 1a6b469f8d22..4eaee8bdfcb2 100644
--- a/fs/hfsplus/hfsplus_fs.h
+++ b/fs/hfsplus/hfsplus_fs.h
@@ -534,7 +534,7 @@ int hfsplus_read_wrapper(struct super_block *sb);

 /* time macros: convert between 1904-2040 and 1970-2106 range,
  * pre-1970 timestamps are interpreted as post-2038 times after wrap-around */
-#define __hfsp_mt2ut(t)                (be32_to_cpu(t) - 2082844800U)
+#define __hfsp_mt2ut(t)                ((time64_t)be32_to_cpu(t) - 2082844800U)
 #define __hfsp_ut2mt(t)                (cpu_to_be32(t + 2082844800U))

 /* compatibility */

I can submit that separately so that it can get backported into
stable kernels if you like, with the type changes as a follow-up
on top.

    Arnd

Re: [PATCH 1/3] hfs: stop using timespec based interfaces

[Viacheslav Dubeyko]

On Tue, 2018-06-19 at 21:42 +0200, Arnd Bergmann wrote:
> On Tue, Jun 19, 2018 at 7:03 PM, Viacheslav Dubeyko <slava@dubeyko.co
> m> wrote:
> > 
> > On Tue, 2018-06-19 at 18:02 +0200, Arnd Bergmann wrote:
> > > 
> > > The native HFS timestamps overflow in year 2040, two years after
> > > the
> > > Unix
> > > y2038 overflow. However, the way that the conversion between on-
> > > disk
> > > timestamps and in-kernel timestamps was implemented, 64-bit
> > > machines
> > > actually ended up converting negative UTC timestamps (1902
> > > through
> > > 1969)
> > > into times between 2038 and 2106.
> > > 
> > > Rather than making all machines faithfully represent timestamps
> > > in
> > > the
> > > ancient past but break after 2040, this changes the file system
> > > to
> > > always use the unsigned UTC interpretation, reading back times
> > > between
> > > 1970 and 2106.
> > > 
> > The trouble with HFS and HFS+ that the specification [1] declares
> > this:
> > 
> > "HFS Plus stores dates in several data structures, including the
> > volume
> > header and catalog records. These dates are stored in unsigned 32-
> > bit
> > integers (UInt32) containing the number of seconds since midnight,
> > January 1, 1904, GMT. This is slightly different from HFS, where
> > the
> > value represents local time. The maximum representable date is
> > February
> > 6, 2040 at 06:28:15 GMT."
> > 
> > So, I am not sure that we are able to support later dates because
> > such
> > timestamps cannot be stored on HFS/HFS+ volumes and will be
> > incompatible with Mac OS X.
> We never followed that interpretation in Linux though. As I wrote,
> on 64-bit machines, these two calculations (hfs and hfs+,
> respectively)
> 
> #define __hfs_m_to_utime(sec)   (be32_to_cpu(sec) - 2082844800U  +
> sys_tz.tz_minuteswest * 60)
> #define __hfsp_mt2ut(t)                (be32_to_cpu(t) - 2082844800U)
> 
> just wrap around when reading the timestamps before 1970 from
> disk. On 32-bit machines they get wrapped another time when
> we assign them to a signed 32-bit time_t.
> 

The whole patchset looks reasonable for me. I simply guess what the
correct behaviour of HFS/HFS+ file system driver could look like for
the case of achieving 2040 year. So, maybe the good way could be to
mount in the READ-ONLY mode. What do you think? 

> > 
> > Also, I am not sure that anybody will use HFS/HFS+ after 2040.
> I'm trying to fix all file systems to be unambiguous regarding
> inode timestamps. This means it should behave the same way
> on 32-bit and 64-bit kernels, and if possible in a sane way.
> 
> Even if you don't care about running HFS in the future, you
> can trivially create files with arbitrary timestamps, just try
> 
> touch -d "Jan 1 1901" 1901
> touch -d "Jan 1 1905" 1905
> touch -d "Jan 1 1969" 1969
> touch -d "Jan 1 2038" 2038
> touch -d "Jan 1 2040" 2040
> touch -d "Jan 1 2106" 2106
> touch -d "Jan 1 2107" 2107
> 
> on HFS and do an 'ls -l' after an unmount/remount.
> 
> If you think it's important that we change the current behavior
> to be compatible with MacOS and represent the 1904..2040
> time range rather than 1970..2106, we can definitely do that
> as well, using this patch:
> 
> diff --git a/fs/hfs/hfs_fs.h b/fs/hfs/hfs_fs.h
> index ff432931a5b1..2c7366342656 100644
> --- a/fs/hfs/hfs_fs.h
> +++ b/fs/hfs/hfs_fs.h
> @@ -249,7 +249,7 @@ extern void hfs_mark_mdb_dirty(struct super_block
> *sb);
>   * actually works until year 2106
>   */
>  #define __hfs_u_to_mtime(sec)  cpu_to_be32(sec + 2082844800U -
> sys_tz.tz_minuteswest * 60)
> -#define __hfs_m_to_utime(sec)  (be32_to_cpu(sec) - 2082844800U  +
> sys_tz.tz_minuteswest * 60)
> +#define __hfs_m_to_utime(sec)  ((time64_t)be32_to_cpu(sec) -
> 2082844800U  + sys_tz.tz_minuteswest * 60)
> 
>  #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/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h
> index 1a6b469f8d22..4eaee8bdfcb2 100644
> --- a/fs/hfsplus/hfsplus_fs.h
> +++ b/fs/hfsplus/hfsplus_fs.h
> @@ -534,7 +534,7 @@ int hfsplus_read_wrapper(struct super_block *sb);
> 
>  /* time macros: convert between 1904-2040 and 1970-2106 range,
>   * pre-1970 timestamps are interpreted as post-2038 times after
> wrap-around */
> -#define __hfsp_mt2ut(t)                (be32_to_cpu(t) -
> 2082844800U)
> +#define __hfsp_mt2ut(t)                ((time64_t)be32_to_cpu(t) -
> 2082844800U)
>  #define __hfsp_ut2mt(t)                (cpu_to_be32(t +
> 2082844800U))
> 
>  /* compatibility */
> 
> I can submit that separately so that it can get backported into
> stable kernels if you like, with the type changes as a follow-up
> on top.
> 

Sounds good.

Thanks,
Vyacheslav Dubeyko.

Re: [PATCH 1/3] hfs: stop using timespec based interfaces

[Arnd Bergmann]

On Wed, Jun 20, 2018 at 6:55 PM, Viacheslav Dubeyko <slava@dubeyko.com> wrote:
> On Tue, 2018-06-19 at 21:42 +0200, Arnd Bergmann wrote:
>> On Tue, Jun 19, 2018 at 7:03 PM, Viacheslav Dubeyko <slava@dubeyko.com> wrote:

>> We never followed that interpretation in Linux though. As I wrote,
>> on 64-bit machines, these two calculations (hfs and hfs+,
>> respectively)
>>
>> #define __hfs_m_to_utime(sec)   (be32_to_cpu(sec) - 2082844800U  +
>> sys_tz.tz_minuteswest * 60)
>> #define __hfsp_mt2ut(t)                (be32_to_cpu(t) - 2082844800U)
>>
>> just wrap around when reading the timestamps before 1970 from
>> disk. On 32-bit machines they get wrapped another time when
>> we assign them to a signed 32-bit time_t.
>>
>
> The whole patchset looks reasonable for me. I simply guess what the
> correct behaviour of HFS/HFS+ file system driver could look like for
> the case of achieving 2040 year. So, maybe the good way could be to
> mount in the READ-ONLY mode. What do you think?

We've discussed doing that in VFS before, this is something we
need to revisit, but I'd like to do it in common code rather than
in every file system with a particular limit.

Deepa has a patch set to introduce minimum/maximum timestamps
in the superblock for this. We definitely want to use that for limiting
the range of utimensat() arguments from user space, and the idea
we had discussed in the past was to have a way to enforce
read-only mounting of file systems that cannot write current i_mtime
values past a certain (user-defined) future date.

We actually need something like that soon, as there are some
organizations that want to support super-long service lifetimes
for Linux systems (e.g. cars, industrial machines, ...) and want
an early-fail behavior to ensure that everything that works today
can in principle keep working for the foreseeable future, while
everything that is known to break can be forced to break already.

This is clearly not a priority for HFS in particular, but there is no
reason for HFS to be different from ext3 here, which has a similar
problem (timestamps are defined to range from 1902 to 2038).

>>  /* time macros: convert between 1904-2040 and 1970-2106 range,
>>   * pre-1970 timestamps are interpreted as post-2038 times after
>> wrap-around */
>> -#define __hfsp_mt2ut(t)                (be32_to_cpu(t) -
>> 2082844800U)
>> +#define __hfsp_mt2ut(t)                ((time64_t)be32_to_cpu(t) -
>> 2082844800U)
>>  #define __hfsp_ut2mt(t)                (cpu_to_be32(t +
>> 2082844800U))
>>
>>  /* compatibility */
>>
>> I can submit that separately so that it can get backported into
>> stable kernels if you like, with the type changes as a follow-up
>> on top.
>>
>
> Sounds good.

Ok, I'll send an updated version with that patch first then.

       Arnd

Re: [PATCH 1/3] hfs: stop using timespec based interfaces

[Arnd Bergmann]

On Wed, Jun 20, 2018 at 9:55 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Wed, Jun 20, 2018 at 6:55 PM, Viacheslav Dubeyko <slava@dubeyko.com> wrote:
>> On Tue, 2018-06-19 at 21:42 +0200, Arnd Bergmann wrote:
>>> On Tue, Jun 19, 2018 at 7:03 PM, Viacheslav Dubeyko <slava@dubeyko.com> wrote:

>>>  /* time macros: convert between 1904-2040 and 1970-2106 range,
>>>   * pre-1970 timestamps are interpreted as post-2038 times after
>>> wrap-around */
>>> -#define __hfsp_mt2ut(t)                (be32_to_cpu(t) -
>>> 2082844800U)
>>> +#define __hfsp_mt2ut(t)                ((time64_t)be32_to_cpu(t) -
>>> 2082844800U)
>>>  #define __hfsp_ut2mt(t)                (cpu_to_be32(t +
>>> 2082844800U))
>>>
>>>  /* compatibility */
>>>
>>> I can submit that separately so that it can get backported into
>>> stable kernels if you like, with the type changes as a follow-up
>>> on top.
>>>
>>
>> Sounds good.
>
> Ok, I'll send an updated version with that patch first then.

I've now sent that patch with additional information that I got from reading the
XNU sources. Interestingly, that also uses the 1970-2106 time range that
I had in my original series, not the 1904-2040 time range that is documented.

      Arnd

Re: [PATCH 1/3] hfs: stop using timespec based interfaces

[Ernesto A. Fernández]

If you decide to go with this series after all:

On Tue, Jun 19, 2018 at 06:02:07PM +0200, Arnd Bergmann wrote:
> The native HFS timestamps overflow in year 2040, two years after the Unix
> y2038 overflow. However, the way that the conversion between on-disk
> timestamps and in-kernel timestamps was implemented, 64-bit machines
> actually ended up converting negative UTC timestamps (1902 through 1969)
> into times between 2038 and 2106.
> 
> Rather than making all machines faithfully represent timestamps in the
> ancient past but break after 2040, this changes the file system to
> always use the unsigned UTC interpretation, reading back times between
> 1970 and 2106.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  fs/hfs/hfs_fs.h | 6 ++++--
>  fs/hfs/inode.c  | 4 ++--
>  2 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/hfs/hfs_fs.h b/fs/hfs/hfs_fs.h
> index 6d0783e2e276..ff432931a5b1 100644
> --- a/fs/hfs/hfs_fs.h
> +++ b/fs/hfs/hfs_fs.h
> @@ -245,6 +245,8 @@ extern void hfs_mark_mdb_dirty(struct super_block *sb);
>   *	Unix:	unsigned lil-endian since 00:00 GMT, Jan. 1, 1970
>   *	mac:	unsigned big-endian since 00:00 GMT, Jan. 1, 1904
>   *
> + * We treat all timestamps before 1970 as times after 2038, so this
							 ^^^^
Same as the hfsplus patch, I find this confusing. I think it should say 2040.


Everything else looks good.

Reviewed-by: Ernesto A. Fernández <ernesto.mnd.fernandez@gmail.com>

> + * actually works until year 2106
>   */
>  #define __hfs_u_to_mtime(sec)	cpu_to_be32(sec + 2082844800U - sys_tz.tz_minuteswest * 60)
>  #define __hfs_m_to_utime(sec)	(be32_to_cpu(sec) - 2082844800U  + sys_tz.tz_minuteswest * 60)
> @@ -252,9 +254,9 @@ extern void hfs_mark_mdb_dirty(struct super_block *sb);
>  #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)
>  
> -#define hfs_m_to_utime(time)	(struct timespec){ .tv_sec = __hfs_m_to_utime(time) }
> +#define hfs_m_to_utime(time)	(struct timespec64){ .tv_sec = __hfs_m_to_utime(time) }
>  #define hfs_u_to_mtime(time)	__hfs_u_to_mtime((time).tv_sec)
> -#define hfs_mtime()		__hfs_u_to_mtime(get_seconds())
> +#define hfs_mtime()		__hfs_u_to_mtime(ktime_get_real_seconds())
>  
>  static inline const char *hfs_mdb_name(struct super_block *sb)
>  {
> diff --git a/fs/hfs/inode.c b/fs/hfs/inode.c
> index 2a16111d312f..b3309b83371a 100644
> --- a/fs/hfs/inode.c
> +++ b/fs/hfs/inode.c
> @@ -351,7 +351,7 @@ static int hfs_read_inode(struct inode *inode, void *data)
>  		inode->i_mode &= ~hsb->s_file_umask;
>  		inode->i_mode |= S_IFREG;
>  		inode->i_ctime = inode->i_atime = inode->i_mtime =
> -				timespec_to_timespec64(hfs_m_to_utime(rec->file.MdDat));
> +				hfs_m_to_utime(rec->file.MdDat);
>  		inode->i_op = &hfs_file_inode_operations;
>  		inode->i_fop = &hfs_file_operations;
>  		inode->i_mapping->a_ops = &hfs_aops;
> @@ -362,7 +362,7 @@ static int hfs_read_inode(struct inode *inode, void *data)
>  		HFS_I(inode)->fs_blocks = 0;
>  		inode->i_mode = S_IFDIR | (S_IRWXUGO & ~hsb->s_dir_umask);
>  		inode->i_ctime = inode->i_atime = inode->i_mtime =
> -				timespec_to_timespec64(hfs_m_to_utime(rec->dir.MdDat));
> +				hfs_m_to_utime(rec->dir.MdDat);
>  		inode->i_op = &hfs_dir_inode_operations;
>  		inode->i_fop = &hfs_dir_operations;
>  		break;
> -- 
> 2.9.0
>