proc_show only output string, so we can keep userspace untouched while replacing timeval with timespec64 in kernel.
Not all timer in i8042 have y2038 risk(handshake, match timer, etc), Replacements in those timer are just for consistency.
Signed-off-by: WEN Pingbo pingbo.wen@linaro.org Cc: Y2038 y2038@lists.linaro.org --- drivers/input/misc/hp_sdc_rtc.c | 52 ++++++++++++++++++++--------------------- 1 file changed, 26 insertions(+), 26 deletions(-)
diff --git a/drivers/input/misc/hp_sdc_rtc.c b/drivers/input/misc/hp_sdc_rtc.c index 45e0e3e..0cd6d64 100644 --- a/drivers/input/misc/hp_sdc_rtc.c +++ b/drivers/input/misc/hp_sdc_rtc.c @@ -198,7 +198,7 @@ static int64_t hp_sdc_rtc_read_i8042timer (uint8_t loadcmd, int numreg)
/* Read the i8042 real-time clock */ -static inline int hp_sdc_rtc_read_rt(struct timeval *res) { +static inline int hp_sdc_rtc_read_rt(struct timespec64 *res) { int64_t raw; uint32_t tenms; unsigned int days; @@ -209,15 +209,15 @@ static inline int hp_sdc_rtc_read_rt(struct timeval *res) { tenms = (uint32_t)raw & 0xffffff; days = (unsigned int)(raw >> 24) & 0xffff;
- res->tv_usec = (suseconds_t)(tenms % 100) * 10000; - res->tv_sec = (time_t)(tenms / 100) + days * 86400; + res->tv_nsec = (long)(tenms % 100) * 10000 * 1000; + res->tv_sec = (time64_t)(tenms / 100) + days * 86400;
return 0; }
/* Read the i8042 fast handshake timer */ -static inline int hp_sdc_rtc_read_fhs(struct timeval *res) { +static inline int hp_sdc_rtc_read_fhs(struct timespec64 *res) { int64_t raw; unsigned int tenms;
@@ -226,15 +226,15 @@ static inline int hp_sdc_rtc_read_fhs(struct timeval *res) {
tenms = (unsigned int)raw & 0xffff;
- res->tv_usec = (suseconds_t)(tenms % 100) * 10000; - res->tv_sec = (time_t)(tenms / 100); + res->tv_nsec = (long)(tenms % 100) * 10000 * 1000; + res->tv_sec = (time64_t)(tenms / 100);
return 0; }
/* Read the i8042 match timer (a.k.a. alarm) */ -static inline int hp_sdc_rtc_read_mt(struct timeval *res) { +static inline int hp_sdc_rtc_read_mt(struct timespec64 *res) { int64_t raw; uint32_t tenms;
@@ -243,15 +243,15 @@ static inline int hp_sdc_rtc_read_mt(struct timeval *res) {
tenms = (uint32_t)raw & 0xffffff;
- res->tv_usec = (suseconds_t)(tenms % 100) * 10000; - res->tv_sec = (time_t)(tenms / 100); + res->tv_nsec = (long)(tenms % 100) * 10000 * 1000; + res->tv_sec = (time64_t)(tenms / 100);
return 0; }
/* Read the i8042 delay timer */ -static inline int hp_sdc_rtc_read_dt(struct timeval *res) { +static inline int hp_sdc_rtc_read_dt(struct timespec64 *res) { int64_t raw; uint32_t tenms;
@@ -260,15 +260,15 @@ static inline int hp_sdc_rtc_read_dt(struct timeval *res) {
tenms = (uint32_t)raw & 0xffffff;
- res->tv_usec = (suseconds_t)(tenms % 100) * 10000; - res->tv_sec = (time_t)(tenms / 100); + res->tv_nsec = (long)(tenms % 100) * 10000 * 1000; + res->tv_sec = (time64_t)(tenms / 100);
return 0; }
/* Read the i8042 cycle timer (a.k.a. periodic) */ -static inline int hp_sdc_rtc_read_ct(struct timeval *res) { +static inline int hp_sdc_rtc_read_ct(struct timespec64 *res) { int64_t raw; uint32_t tenms;
@@ -277,8 +277,8 @@ static inline int hp_sdc_rtc_read_ct(struct timeval *res) {
tenms = (uint32_t)raw & 0xffffff;
- res->tv_usec = (suseconds_t)(tenms % 100) * 10000; - res->tv_sec = (time_t)(tenms / 100); + res->tv_nsec = (long)(tenms % 100) * 10000 * 1000; + res->tv_sec = (time64_t)(tenms / 100);
return 0; } @@ -433,7 +433,7 @@ static int hp_sdc_rtc_proc_show(struct seq_file *m, void *v) #define YN(bit) ("no") #define NY(bit) ("yes") struct rtc_time tm; - struct timeval tv; + struct timespec64 tv;
memset(&tm, 0, sizeof(struct rtc_time));
@@ -452,36 +452,36 @@ static int hp_sdc_rtc_proc_show(struct seq_file *m, void *v) if (hp_sdc_rtc_read_rt(&tv)) { seq_puts(m, "i8042 rtc\t: READ FAILED!\n"); } else { - seq_printf(m, "i8042 rtc\t: %ld.%02d seconds\n", - tv.tv_sec, (int)tv.tv_usec/1000); + seq_printf(m, "i8042 rtc\t: %lld.%02ld seconds\n", + tv.tv_sec, (long)tv.tv_nsec/1000000L); }
if (hp_sdc_rtc_read_fhs(&tv)) { seq_puts(m, "handshake\t: READ FAILED!\n"); } else { - seq_printf(m, "handshake\t: %ld.%02d seconds\n", - tv.tv_sec, (int)tv.tv_usec/1000); + seq_printf(m, "handshake\t: %lld.%02ld seconds\n", + tv.tv_sec, (long)tv.tv_nsec/1000000L); }
if (hp_sdc_rtc_read_mt(&tv)) { seq_puts(m, "alarm\t\t: READ FAILED!\n"); } else { - seq_printf(m, "alarm\t\t: %ld.%02d seconds\n", - tv.tv_sec, (int)tv.tv_usec/1000); + seq_printf(m, "alarm\t\t: %lld.%02ld seconds\n", + tv.tv_sec, (long)tv.tv_nsec/1000000L); }
if (hp_sdc_rtc_read_dt(&tv)) { seq_puts(m, "delay\t\t: READ FAILED!\n"); } else { - seq_printf(m, "delay\t\t: %ld.%02d seconds\n", - tv.tv_sec, (int)tv.tv_usec/1000); + seq_printf(m, "delay\t\t: %lld.%02ld seconds\n", + tv.tv_sec, (long)tv.tv_nsec/1000000L); }
if (hp_sdc_rtc_read_ct(&tv)) { seq_puts(m, "periodic\t: READ FAILED!\n"); } else { - seq_printf(m, "periodic\t: %ld.%02d seconds\n", - tv.tv_sec, (int)tv.tv_usec/1000); + seq_printf(m, "periodic\t: %lld.%02ld seconds\n", + tv.tv_sec, (long)tv.tv_nsec/1000000L); }
seq_printf(m,
On Sunday 13 September 2015 19:00:11 WEN Pingbo wrote:
proc_show only output string, so we can keep userspace untouched while replacing timeval with timespec64 in kernel.
Not all timer in i8042 have y2038 risk(handshake, match timer, etc), Replacements in those timer are just for consistency.
The patch looks mostly ok, a few details here:
- Please start the changelog with an explanation of what the driver does and why that is a problem, e.g.
"The SDC real time clock driver internally uses an 'struct timeval' to represent times, which will overflow on 32-bit machines in 2038."
@@ -209,15 +209,15 @@ static inline int hp_sdc_rtc_read_rt(struct timeval *res) { tenms = (uint32_t)raw & 0xffffff; days = (unsigned int)(raw >> 24) & 0xffff;
- res->tv_usec = (suseconds_t)(tenms % 100) * 10000;
- res->tv_sec = (time_t)(tenms / 100) + days * 86400;
- res->tv_nsec = (long)(tenms % 100) * 10000 * 1000;
For clarity, I would suggest changing the code to use the USEC_PER_SEC macro. We don't have a global SECONDS_PER_DAY macro unfortunately. If you are really keen, you could introduce one in include/linux/time64.h, otherwise just leave it as it is.
- res->tv_sec = (time64_t)(tenms / 100) + days * 86400;
If I read this right, you still have a bug here: "days" is an "unsigned int" variable, and multiplying that with 86400 will overflow in 2106. We don't care that much about this overflow yet, but there is no reason to not get it right here if it's cheap.
If you move the type cast, it will be fine.
@@ -452,36 +452,36 @@ static int hp_sdc_rtc_proc_show(struct seq_file *m, void *v) if (hp_sdc_rtc_read_rt(&tv)) { seq_puts(m, "i8042 rtc\t: READ FAILED!\n"); } else {
seq_printf(m, "i8042 rtc\t: %ld.%02d seconds\n",tv.tv_sec, (int)tv.tv_usec/1000);
seq_printf(m, "i8042 rtc\t: %lld.%02ld seconds\n",tv.tv_sec, (long)tv.tv_nsec/1000000L);
The change from (int) to (long) is not necessary here, because the nanoseconds always fit in a 32bit int type.
However, the tv_sec variable is currently defined in a slightly awkward way, and is 'long' on 64-bit architectures but 'long long' on 32-bit architectures.
This means you get a compile warning for a type mismatch on 64-bit compilers that print a 'long' argument with %lld. The best workaround I've come up with is to add a cast to 'long long' or 's64', which is defined as 'long long' on all architectures.
To avoid problems like this, it's useful to build test with both 32-bit and 64-bit compilers, and to list in the changelog text what kind of testing you have done to verify the driver.
Arnd
在 2015年9月14日，16:37，Arnd Bergmann arnd@arndb.de 写道：
On Sunday 13 September 2015 19:00:11 WEN Pingbo wrote:
proc_show only output string, so we can keep userspace untouched while replacing timeval with timespec64 in kernel.
Not all timer in i8042 have y2038 risk(handshake, match timer, etc), Replacements in those timer are just for consistency.
The patch looks mostly ok, a few details here:
- Please start the changelog with an explanation of what the driver does
and why that is a problem, e.g.
"The SDC real time clock driver internally uses an 'struct timeval' to represent times, which will overflow on 32-bit machines in 2038.”
Ok, I will rearrange it later. Thanks.
@@ -209,15 +209,15 @@ static inline int hp_sdc_rtc_read_rt(struct timeval *res) { tenms = (uint32_t)raw & 0xffffff; days = (unsigned int)(raw >> 24) & 0xffff;
- res->tv_usec = (suseconds_t)(tenms % 100) * 10000;
- res->tv_sec = (time_t)(tenms / 100) + days * 86400;
- res->tv_nsec = (long)(tenms % 100) * 10000 * 1000;
For clarity, I would suggest changing the code to use the USEC_PER_SEC macro. We don't have a global SECONDS_PER_DAY macro unfortunately. If you are really keen, you could introduce one in include/linux/time64.h, otherwise just leave it as it is.
That make sense.
- res->tv_sec = (time64_t)(tenms / 100) + days * 86400;
If I read this right, you still have a bug here: "days" is an "unsigned int" variable, and multiplying that with 86400 will overflow in 2106. We don't care that much about this overflow yet, but there is no reason to not get it right here if it's cheap.
If you move the type cast, it will be fine.
Yes, we need one more type cast here.
@@ -452,36 +452,36 @@ static int hp_sdc_rtc_proc_show(struct seq_file *m, void *v) if (hp_sdc_rtc_read_rt(&tv)) { seq_puts(m, "i8042 rtc\t: READ FAILED!\n"); } else {
seq_printf(m, "i8042 rtc\t: %ld.%02d seconds\n",tv.tv_sec, (int)tv.tv_usec/1000);
seq_printf(m, "i8042 rtc\t: %lld.%02ld seconds\n",tv.tv_sec, (long)tv.tv_nsec/1000000L);The change from (int) to (long) is not necessary here, because the nanoseconds always fit in a 32bit int type.
However, the tv_sec variable is currently defined in a slightly awkward way, and is 'long' on 64-bit architectures but 'long long' on 32-bit architectures.
This means you get a compile warning for a type mismatch on 64-bit compilers that print a 'long' argument with %lld. The best workaround I've come up with is to add a cast to 'long long' or 's64', which is defined as 'long long' on all architectures.
To avoid problems like this, it's useful to build test with both 32-bit and 64-bit compilers, and to list in the changelog text what kind of testing you have done to verify the driver.
I didn’t notice that. It seems hp_sdc_rtc only available in m68k & hppa arch. I am still on the way to compile it correctly.
Arnd
On Tuesday 15 September 2015 10:08:01 Pingbo Wen wrote:
The change from (int) to (long) is not necessary here, because the nanoseconds always fit in a 32bit int type.
However, the tv_sec variable is currently defined in a slightly awkward way, and is 'long' on 64-bit architectures but 'long long' on 32-bit architectures.
This means you get a compile warning for a type mismatch on 64-bit compilers that print a 'long' argument with %lld. The best workaround I've come up with is to add a cast to 'long long' or 's64', which is defined as 'long long' on all architectures.
To avoid problems like this, it's useful to build test with both 32-bit and 64-bit compilers, and to list in the changelog text what kind of testing you have done to verify the driver.
I didn’t notice that. It seems hp_sdc_rtc only available in m68k & hppa arch. I am still on the way to compile it correctly.
Ok. There are two options for this:
a) get a cross-compiler for the respective architectures. https://www.kernel.org/pub/tools/crosstool/ has prebuilt binaries for most, otherwise use the scripts from http://git.infradead.org/users/segher/buildall.git to easily build your own.
b) temporarily change Kconfig to not list a dependency on those architectures and build it for x86 or arm (whichever is easier) in both 32-bit and 64-bit. Most of the time that works, but occasionally get you get dependencies on arch headers that are nontrivial to resolve.
Arnd
hp_sdc_rtc_proc_show() use timeval to store the time, which will overflowed in 2038.
This patch fixes this problem by replacing timeval with timespec64. hp_sdc_rtc_proc_show() only output string, so that userspace will work normally if we apply this patch.
Not all timer in i8042 have y2038 risk(handshake, match timer, etc), Replacements in those timer are just for consistency.
Version 2 Updates: - compiled in m68k gcc cross compiler(4.6.3), no extra warnings - placed s64 type cast in tv.tv_sec, making sure it work properly in both 32bit and 64bit platform.
Signed-off-by: WEN Pingbo pingbo.wen@linaro.org Cc: Y2038 y2038@lists.linaro.org --- drivers/input/misc/hp_sdc_rtc.c | 52 ++++++++++++++++++++--------------------- 1 file changed, 26 insertions(+), 26 deletions(-)
diff --git a/drivers/input/misc/hp_sdc_rtc.c b/drivers/input/misc/hp_sdc_rtc.c index 45e0e3e..1c8c56e 100644 --- a/drivers/input/misc/hp_sdc_rtc.c +++ b/drivers/input/misc/hp_sdc_rtc.c @@ -198,7 +198,7 @@ static int64_t hp_sdc_rtc_read_i8042timer (uint8_t loadcmd, int numreg)
/* Read the i8042 real-time clock */ -static inline int hp_sdc_rtc_read_rt(struct timeval *res) { +static inline int hp_sdc_rtc_read_rt(struct timespec64 *res) { int64_t raw; uint32_t tenms; unsigned int days; @@ -209,15 +209,15 @@ static inline int hp_sdc_rtc_read_rt(struct timeval *res) { tenms = (uint32_t)raw & 0xffffff; days = (unsigned int)(raw >> 24) & 0xffff;
- res->tv_usec = (suseconds_t)(tenms % 100) * 10000; - res->tv_sec = (time_t)(tenms / 100) + days * 86400; + res->tv_nsec = (long)(tenms % 100) * 10000 * 1000; + res->tv_sec = (tenms / 100) + (time64_t)days * 86400;
return 0; }
/* Read the i8042 fast handshake timer */ -static inline int hp_sdc_rtc_read_fhs(struct timeval *res) { +static inline int hp_sdc_rtc_read_fhs(struct timespec64 *res) { int64_t raw; unsigned int tenms;
@@ -226,15 +226,15 @@ static inline int hp_sdc_rtc_read_fhs(struct timeval *res) {
tenms = (unsigned int)raw & 0xffff;
- res->tv_usec = (suseconds_t)(tenms % 100) * 10000; - res->tv_sec = (time_t)(tenms / 100); + res->tv_nsec = (long)(tenms % 100) * 10000 * 1000; + res->tv_sec = (time64_t)(tenms / 100);
return 0; }
/* Read the i8042 match timer (a.k.a. alarm) */ -static inline int hp_sdc_rtc_read_mt(struct timeval *res) { +static inline int hp_sdc_rtc_read_mt(struct timespec64 *res) { int64_t raw; uint32_t tenms;
@@ -243,15 +243,15 @@ static inline int hp_sdc_rtc_read_mt(struct timeval *res) {
tenms = (uint32_t)raw & 0xffffff;
- res->tv_usec = (suseconds_t)(tenms % 100) * 10000; - res->tv_sec = (time_t)(tenms / 100); + res->tv_nsec = (long)(tenms % 100) * 10000 * 1000; + res->tv_sec = (time64_t)(tenms / 100);
return 0; }
/* Read the i8042 delay timer */ -static inline int hp_sdc_rtc_read_dt(struct timeval *res) { +static inline int hp_sdc_rtc_read_dt(struct timespec64 *res) { int64_t raw; uint32_t tenms;
@@ -260,15 +260,15 @@ static inline int hp_sdc_rtc_read_dt(struct timeval *res) {
tenms = (uint32_t)raw & 0xffffff;
- res->tv_usec = (suseconds_t)(tenms % 100) * 10000; - res->tv_sec = (time_t)(tenms / 100); + res->tv_nsec = (long)(tenms % 100) * 10000 * 1000; + res->tv_sec = (time64_t)(tenms / 100);
return 0; }
/* Read the i8042 cycle timer (a.k.a. periodic) */ -static inline int hp_sdc_rtc_read_ct(struct timeval *res) { +static inline int hp_sdc_rtc_read_ct(struct timespec64 *res) { int64_t raw; uint32_t tenms;
@@ -277,8 +277,8 @@ static inline int hp_sdc_rtc_read_ct(struct timeval *res) {
tenms = (uint32_t)raw & 0xffffff;
- res->tv_usec = (suseconds_t)(tenms % 100) * 10000; - res->tv_sec = (time_t)(tenms / 100); + res->tv_nsec = (long)(tenms % 100) * 10000 * 1000; + res->tv_sec = (time64_t)(tenms / 100);
return 0; } @@ -433,7 +433,7 @@ static int hp_sdc_rtc_proc_show(struct seq_file *m, void *v) #define YN(bit) ("no") #define NY(bit) ("yes") struct rtc_time tm; - struct timeval tv; + struct timespec64 tv;
memset(&tm, 0, sizeof(struct rtc_time));
@@ -452,36 +452,36 @@ static int hp_sdc_rtc_proc_show(struct seq_file *m, void *v) if (hp_sdc_rtc_read_rt(&tv)) { seq_puts(m, "i8042 rtc\t: READ FAILED!\n"); } else { - seq_printf(m, "i8042 rtc\t: %ld.%02d seconds\n", - tv.tv_sec, (int)tv.tv_usec/1000); + seq_printf(m, "i8042 rtc\t: %lld.%02ld seconds\n", + (s64)tv.tv_sec, (long)tv.tv_nsec/1000000L); }
if (hp_sdc_rtc_read_fhs(&tv)) { seq_puts(m, "handshake\t: READ FAILED!\n"); } else { - seq_printf(m, "handshake\t: %ld.%02d seconds\n", - tv.tv_sec, (int)tv.tv_usec/1000); + seq_printf(m, "handshake\t: %lld.%02ld seconds\n", + (s64)tv.tv_sec, (long)tv.tv_nsec/1000000L); }
if (hp_sdc_rtc_read_mt(&tv)) { seq_puts(m, "alarm\t\t: READ FAILED!\n"); } else { - seq_printf(m, "alarm\t\t: %ld.%02d seconds\n", - tv.tv_sec, (int)tv.tv_usec/1000); + seq_printf(m, "alarm\t\t: %lld.%02ld seconds\n", + (s64)tv.tv_sec, (long)tv.tv_nsec/1000000L); }
if (hp_sdc_rtc_read_dt(&tv)) { seq_puts(m, "delay\t\t: READ FAILED!\n"); } else { - seq_printf(m, "delay\t\t: %ld.%02d seconds\n", - tv.tv_sec, (int)tv.tv_usec/1000); + seq_printf(m, "delay\t\t: %lld.%02ld seconds\n", + (s64)tv.tv_sec, (long)tv.tv_nsec/1000000L); }
if (hp_sdc_rtc_read_ct(&tv)) { seq_puts(m, "periodic\t: READ FAILED!\n"); } else { - seq_printf(m, "periodic\t: %ld.%02d seconds\n", - tv.tv_sec, (int)tv.tv_usec/1000); + seq_printf(m, "periodic\t: %lld.%02ld seconds\n", + (s64)tv.tv_sec, (long)tv.tv_nsec/1000000L); }
seq_printf(m,
On Wednesday 16 September 2015 21:45:38 WEN Pingbo wrote:
hp_sdc_rtc_proc_show() use timeval to store the time, which will overflowed in 2038.
This patch fixes this problem by replacing timeval with timespec64. hp_sdc_rtc_proc_show() only output string, so that userspace will work normally if we apply this patch.
Not all timer in i8042 have y2038 risk(handshake, match timer, etc), Replacements in those timer are just for consistency.
Version 2 Updates:
- compiled in m68k gcc cross compiler(4.6.3), no extra warnings
- placed s64 type cast in tv.tv_sec, making sure it work properly in
both 32bit and 64bit platform.
Signed-off-by: WEN Pingbo pingbo.wen@linaro.org Cc: Y2038 y2038@lists.linaro.org
Looks very good,
Reviewed-by: Arnd Bergmann arnd@arndb.de
On Wed, Sep 16, 2015 at 03:55:37PM +0200, Arnd Bergmann wrote:
On Wednesday 16 September 2015 21:45:38 WEN Pingbo wrote:
hp_sdc_rtc_proc_show() use timeval to store the time, which will overflowed in 2038.
This patch fixes this problem by replacing timeval with timespec64. hp_sdc_rtc_proc_show() only output string, so that userspace will work normally if we apply this patch.
Not all timer in i8042 have y2038 risk(handshake, match timer, etc), Replacements in those timer are just for consistency.
Version 2 Updates:
- compiled in m68k gcc cross compiler(4.6.3), no extra warnings
- placed s64 type cast in tv.tv_sec, making sure it work properly in
both 32bit and 64bit platform.
Signed-off-by: WEN Pingbo pingbo.wen@linaro.org Cc: Y2038 y2038@lists.linaro.org
Looks very good,
Reviewed-by: Arnd Bergmann arnd@arndb.de
Applied, thank you.
-
Arnd Bergmann -
Dmitry Torokhov -
Pingbo Wen -
WEN Pingbo