The hpfs code uses time_t which will overflow at 2038. If time_t is only internal used without being stored on disk, simply replacing it with time64_t is fine. Otherwise, since the range of time has been already lost when it is stored on disk, the only thing we can do is a cast between 32-bit value and time64_t so as to remove time_t.
DengChao (2): fs:hpfs:Remove internal using time_t fs:hpfs:Remove time_t used on disk
fs/hpfs/hpfs_fn.h | 26 ++++++++++++++++++++++---- fs/hpfs/namei.c | 21 +++++++++++++++------ 2 files changed, 37 insertions(+), 10 deletions(-)
The hpfs code uses "get_seconds()" and "time_t", this will cause problems on 32-bit architectures in 2038 when time_t overflows. This patch replaces "get_seconds()" and "time_t" with "ktime_get_real_seconds()" and "time64_t".
Signed-off-by: DengChao chao.deng@linaro.org --- fs/hpfs/namei.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/hpfs/namei.c b/fs/hpfs/namei.c index 9e92c9c..9fcb6f0 100644 --- a/fs/hpfs/namei.c +++ b/fs/hpfs/namei.c @@ -10,7 +10,7 @@
static void hpfs_update_directory_times(struct inode *dir) { - time_t t = get_seconds(); + time64_t t = ktime_get_real_seconds(); if (t == dir->i_mtime.tv_sec && t == dir->i_ctime.tv_sec) return;
On Thursday 12 November 2015 21:26:02 DengChao wrote:
static void hpfs_update_directory_times(struct inode *dir) {
time_t t = get_seconds();
time64_t t = ktime_get_real_seconds(); if (t == dir->i_mtime.tv_sec && t == dir->i_ctime.tv_sec) return;
It's too early for this change, as the i_mtime and i_ctime fields are still using time_t. We first have to change the generic VFS code.
Arnd
Many time_t issues in filesystems is involved with VFS i_mtime/i_ctime. for example:
static void hpfs_update_directory_times(struct inode *dir) { - time_t t = get_seconds(); + time64_t t = ktime_get_real_seconds(); - if (t == dir->i_mtime.tv_sec && - t == dir->i_ctime.tv_sec) + if ((time_t)t == dir->i_mtime.tv_sec && + (time_t)t == dir->i_ctime.tv_sec) return; Can I replace get_seconds with ktime_get_real_seconds first, and do a 64_to_32 cast so as to be compatible with VFS i_mtime/i_ctime like above? Or just leave all the stuff which is involved with VFS i_mtime/i_ctime until then change of generic VFS code.
Arnd Bergmann arnd@arndb.de
2015-11-12 21:29 To y2038@lists.linaro.org, cc DengChao chao.deng@linaro.org Subject Re: [Y2038] [PATCH 1/2] fs:hpfs:Remove internal using time_t
On Thursday 12 November 2015 21:26:02 DengChao wrote:
static void hpfs_update_directory_times(struct inode *dir) {
time_t t = get_seconds();
time64_t t = ktime_get_real_seconds(); if (t == dir->i_mtime.tv_sec && t == dir->i_ctime.tv_sec) return;
It's too early for this change, as the i_mtime and i_ctime fields are still using time_t. We first have to change the generic VFS code.
Arnd
On Tuesday 17 November 2015 17:04:42 deng.chao1@zte.com.cn wrote:
Many time_t issues in filesystems is involved with VFS i_mtime/i_ctime. for example:
static void hpfs_update_directory_times(struct inode *dir) {
time_t t = get_seconds();
time64_t t = ktime_get_real_seconds();
if (t == dir->i_mtime.tv_sec &&
t == dir->i_ctime.tv_sec)
if ((time_t)t == dir->i_mtime.tv_sec &&
(time_t)t == dir->i_ctime.tv_sec) return;
Can I replace get_seconds with ktime_get_real_seconds first, and do a 64_to_32 cast so as to be compatible with VFS i_mtime/i_ctime like above? Or just leave all the stuff which is involved with VFS i_mtime/i_ctime until then change of generic VFS code.
Better just leave it alone for now. Deepa is looking into the VFS changes, and I think it will make her work easier that way.
Arnd
On Nov 17, 2015, at 1:07 AM, Arnd Bergmann arnd@arndb.de wrote:
On Tuesday 17 November 2015 17:04:42 deng.chao1@zte.com.cn wrote: Many time_t issues in filesystems is involved with VFS i_mtime/i_ctime. for example:
static void hpfs_update_directory_times(struct inode *dir) {
time_t t = get_seconds();
time64_t t = ktime_get_real_seconds();
if (t == dir->i_mtime.tv_sec &&
t == dir->i_ctime.tv_sec)
if ((time_t)t == dir->i_mtime.tv_sec &&
(time_t)t == dir->i_ctime.tv_sec) return;
Can I replace get_seconds with ktime_get_real_seconds first, and do a 64_to_32 cast so as to be compatible with VFS i_mtime/i_ctime like above? Or just leave all the stuff which is involved with VFS i_mtime/i_ctime until then change of generic VFS code.
Better just leave it alone for now. Deepa is looking into the VFS changes, and I think it will make her work easier that way.
Yes, I'm looking at vfs. I haven't decided what to do with this yet. Let's leave it as is for now.
-Deepa
"get_seconds()" and "time_t" will cause problems on 32-bit architectures in 2038 when time_t overflows. The hpfs code uses time32_t to represent time_t on disk, the range of time has been already lost, so that this patch just makes directly cast between time32_t and time_64t so as to remove time_t. Fortunately time32_t won't overflow until 2106.
Signed-off-by: DengChao chao.deng@linaro.org --- fs/hpfs/hpfs_fn.h | 26 ++++++++++++++++++++++---- fs/hpfs/namei.c | 19 ++++++++++++++----- 2 files changed, 36 insertions(+), 9 deletions(-)
diff --git a/fs/hpfs/hpfs_fn.h b/fs/hpfs/hpfs_fn.h index 975654a..0ef8f75 100644 --- a/fs/hpfs/hpfs_fn.h +++ b/fs/hpfs/hpfs_fn.h @@ -331,19 +331,37 @@ unsigned hpfs_get_free_dnodes(struct super_block *); long hpfs_ioctl(struct file *file, unsigned cmd, unsigned long arg);
/* + * Since time32_t is stored into media, the range has been already lost + * for time64_t anyway. Only thing we can do is directly cast between + * time32_t and time64_t. Forunately, time32_t won't overflow until 2106. + */ + +static inline time64_t cast_to_time64(time32_t t) +{ + return (time64_t)t; +} + +static inline time32_t cast_to_time32(time64_t t) +{ + return (time32_t)t; +} + +/* * local time (HPFS) to GMT (Unix) */
-static inline time_t local_to_gmt(struct super_block *s, time32_t t) +static inline time64_t local_to_gmt(struct super_block *s, time32_t t) { extern struct timezone sys_tz; - return t + sys_tz.tz_minuteswest * 60 + hpfs_sb(s)->sb_timeshift; + return cast_to_time64(t + sys_tz.tz_minuteswest * 60 + + hpfs_sb(s)->sb_timeshift); }
-static inline time32_t gmt_to_local(struct super_block *s, time_t t) +static inline time32_t gmt_to_local(struct super_block *s, time64_t t) { extern struct timezone sys_tz; - return t - sys_tz.tz_minuteswest * 60 - hpfs_sb(s)->sb_timeshift; + return cast_to_time32(t - sys_tz.tz_minuteswest * 60 + - hpfs_sb(s)->sb_timeshift); }
/* diff --git a/fs/hpfs/namei.c b/fs/hpfs/namei.c index 9fcb6f0..5fd608b 100644 --- a/fs/hpfs/namei.c +++ b/fs/hpfs/namei.c @@ -49,7 +49,9 @@ static int hpfs_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode) /*dee.archive = 0;*/ dee.hidden = name[0] == '.'; dee.fnode = cpu_to_le32(fno); - dee.creation_date = dee.write_date = dee.read_date = cpu_to_le32(gmt_to_local(dir->i_sb, get_seconds())); + dee.creation_date = dee.write_date = dee.read_date = + cpu_to_le32(gmt_to_local(dir->i_sb, + ktime_get_real_seconds())); result = new_inode(dir->i_sb); if (!result) goto bail2; @@ -90,7 +92,9 @@ static int hpfs_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode) dnode->root_dnode = 1; dnode->up = cpu_to_le32(fno); de = hpfs_add_de(dir->i_sb, dnode, "\001\001", 2, 0); - de->creation_date = de->write_date = de->read_date = cpu_to_le32(gmt_to_local(dir->i_sb, get_seconds())); + de->creation_date = de->write_date = de->read_date + = cpu_to_le32(gmt_to_local(dir->i_sb, + ktime_get_real_seconds())); if (!(mode & 0222)) de->read_only = 1; de->first = de->directory = 1; /*de->hidden = de->system = 0;*/ @@ -150,7 +154,9 @@ static int hpfs_create(struct inode *dir, struct dentry *dentry, umode_t mode, b dee.archive = 1; dee.hidden = name[0] == '.'; dee.fnode = cpu_to_le32(fno); - dee.creation_date = dee.write_date = dee.read_date = cpu_to_le32(gmt_to_local(dir->i_sb, get_seconds())); + dee.creation_date = dee.write_date = dee.read_date + = cpu_to_le32(gmt_to_local(dir->i_sb, + ktime_get_real_seconds()));
result = new_inode(dir->i_sb); if (!result) @@ -239,7 +245,9 @@ static int hpfs_mknod(struct inode *dir, struct dentry *dentry, umode_t mode, de dee.archive = 1; dee.hidden = name[0] == '.'; dee.fnode = cpu_to_le32(fno); - dee.creation_date = dee.write_date = dee.read_date = cpu_to_le32(gmt_to_local(dir->i_sb, get_seconds())); + dee.creation_date = dee.write_date = dee.read_date + = cpu_to_le32(gmt_to_local(dir->i_sb, + ktime_get_real_seconds()));
result = new_inode(dir->i_sb); if (!result) @@ -315,7 +323,8 @@ static int hpfs_symlink(struct inode *dir, struct dentry *dentry, const char *sy dee.archive = 1; dee.hidden = name[0] == '.'; dee.fnode = cpu_to_le32(fno); - dee.creation_date = dee.write_date = dee.read_date = cpu_to_le32(gmt_to_local(dir->i_sb, get_seconds())); + dee.creation_date = dee.write_date = dee.read_date = + cpu_to_le32(gmt_to_local(dir->i_sb, ktime_get_real_seconds()));
result = new_inode(dir->i_sb); if (!result)
On Thursday 12 November 2015 21:26:03 DengChao wrote:
/*
- Since time32_t is stored into media, the range has been already lost
- for time64_t anyway. Only thing we can do is directly cast between
- time32_t and time64_t. Forunately, time32_t won't overflow until 2106.
- */
+static inline time64_t cast_to_time64(time32_t t) +{
- return (time64_t)t;
+}
+static inline time32_t cast_to_time32(time64_t t) +{
- return (time32_t)t;
+}
+/*
- local time (HPFS) to GMT (Unix)
*/ -static inline time_t local_to_gmt(struct super_block *s, time32_t t) +static inline time64_t local_to_gmt(struct super_block *s, time32_t t) { extern struct timezone sys_tz;
- return t + sys_tz.tz_minuteswest * 60 + hpfs_sb(s)->sb_timeshift;
- return cast_to_time64(t + sys_tz.tz_minuteswest * 60
+ hpfs_sb(s)->sb_timeshift);
} -static inline time32_t gmt_to_local(struct super_block *s, time_t t) +static inline time32_t gmt_to_local(struct super_block *s, time64_t t) { extern struct timezone sys_tz;
- return t - sys_tz.tz_minuteswest * 60 - hpfs_sb(s)->sb_timeshift;
- return cast_to_time32(t - sys_tz.tz_minuteswest * 60
- hpfs_sb(s)->sb_timeshift);
}
You already have conversion functions here, I would fold the new handlers into these and add the comment right above them. Note that the time of the overflow also depends on the timeshift value.
If we make change sb_timeshift to a time64_t, we could support much later times as well.
diff --git a/fs/hpfs/namei.c b/fs/hpfs/namei.c index 9fcb6f0..5fd608b 100644 --- a/fs/hpfs/namei.c +++ b/fs/hpfs/namei.c @@ -49,7 +49,9 @@ static int hpfs_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode) /*dee.archive = 0;*/ dee.hidden = name[0] == '.'; dee.fnode = cpu_to_le32(fno);
- dee.creation_date = dee.write_date = dee.read_date = cpu_to_le32(gmt_to_local(dir->i_sb, get_seconds()));
- dee.creation_date = dee.write_date = dee.read_date =
cpu_to_le32(gmt_to_local(dir->i_sb,
result = new_inode(dir->i_sb); if (!result) goto bail2;ktime_get_real_seconds()));
I think it would be a nice change to fold the cpu_to_le32()/le32_to_cpu() into the local_to_gmt()/gmt_to_local() functions and replace time32_t with __le32.
Otherwise this looks like a useful change.
Arnd