[PATCH 0/8] y2038: bug fixes from y2038 work

[PATCH 0/8] y2038: bug fixes from y2038 work

[Arnd Bergmann]

I've gone through the remaining uses of time_t etc and come up with a
set of 90 patches of varying complexity and importance, to the point
of being able to remove the old time_t/timeval/timespec from the kernel
headers completely.

This set includes the eight patches that I think should be merged
right away and backported into stable kernels if possible.

Please apply individual patches to the respective maintainer trees
for either v5.4 or v5.5 as appropriate.

For reference, the full series of 90 patches can be found at
https://git.kernel.org/pub/scm/linux/kernel/git/arnd/playground.git/log/?h=y2038-endgame

      Arnd

Arnd Bergmann (8):
  y2038: timex: remove incorrect time_t truncation
  timekeeping: optimize ns_to_timespec64
  powerpc: fix vdso32 for ppc64le
  ipmi: kill off 'timespec' usage again
  netfilter: xt_time: use time64_t
  lp: fix sparc64 LPSETTIMEOUT ioctl
  ppdev: fix PPGETTIME/PPSETTIME ioctls
  Input: input_event: fix struct padding on sparc64

 arch/powerpc/kernel/vdso32/gettimeofday.S |  2 +-
 drivers/char/ipmi/ipmi_si_intf.c          | 40 ++++++++---------------
 drivers/char/lp.c                         |  4 +++
 drivers/char/ppdev.c                      | 16 ++++++---
 drivers/input/evdev.c                     |  3 ++
 drivers/input/misc/uinput.c               |  3 ++
 include/uapi/linux/input.h                |  1 +
 kernel/time/ntp.c                         |  2 +-
 kernel/time/time.c                        | 21 +++++++-----
 net/netfilter/xt_time.c                   | 19 ++++++-----
 10 files changed, 61 insertions(+), 50 deletions(-)

Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Corey Minyard <minyard@acm.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Stephen Boyd <sboyd@kernel.org>
Cc: Pablo Neira Ayuso <pablo@netfilter.org>
Cc: Jozsef Kadlecsik <kadlec@netfilter.org>
Cc: Florian Westphal <fw@strlen.de>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-kernel@vger.kernel.org
Cc: openipmi-developer@lists.sourceforge.net
Cc: linux-input@vger.kernel.org
Cc: netfilter-devel@vger.kernel.org
Cc: coreteam@netfilter.org
Cc: netdev@vger.kernel.org
Cc: sparclinux@vger.kernel.org

-- 
2.20.0

[PATCH 1/8] y2038: timex: remove incorrect time_t truncation

[Arnd Bergmann]

A cast to 'time_t' was accidentally left in place during the
conversion of __do_adjtimex() to 64-bit timestamps, so the
resulting value is incorrectly truncated.

Remove the cast so the 64-bit time gets propagated correctly.

Cc: stable@vger.kernel.org
Fixes: ead25417f82e ("timex: use __kernel_timex internally")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 kernel/time/ntp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index 65eb796610dc..069ca78fb0bf 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
@@ -771,7 +771,7 @@ int __do_adjtimex(struct __kernel_timex *txc, const struct timespec64 *ts,
 	/* fill PPS status fields */
 	pps_fill_timex(txc);
 
-	txc->time.tv_sec = (time_t)ts->tv_sec;
+	txc->time.tv_sec = ts->tv_sec;
 	txc->time.tv_usec = ts->tv_nsec;
 	if (!(time_status & STA_NANO))
 		txc->time.tv_usec = ts->tv_nsec / NSEC_PER_USEC;
-- 
2.20.0

[PATCH 2/8] timekeeping: optimize ns_to_timespec64

[Arnd Bergmann]

I noticed that ns_to_timespec64() calls div_s64_rem(), which is
a rather slow function on 32-bit architectures, as it cannot take
advantage of the do_div() optimizations for constant arguments.

This open-codes the div_s64_rem() function here, so we can pass
a constant divider into the optimized div_u64_rem() function.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 kernel/time/time.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/kernel/time/time.c b/kernel/time/time.c
index 5c54ca632d08..45a358953f09 100644
--- a/kernel/time/time.c
+++ b/kernel/time/time.c
@@ -550,18 +550,21 @@ EXPORT_SYMBOL(set_normalized_timespec64);
  */
 struct timespec64 ns_to_timespec64(const s64 nsec)
 {
-	struct timespec64 ts;
+	struct timespec64 ts = { 0, 0 };
 	s32 rem;
 
-	if (!nsec)
-		return (struct timespec64) {0, 0};
-
-	ts.tv_sec = div_s64_rem(nsec, NSEC_PER_SEC, &rem);
-	if (unlikely(rem < 0)) {
-		ts.tv_sec--;
-		rem += NSEC_PER_SEC;
+	if (likely(nsec > 0)) {
+		ts.tv_sec = div_u64_rem(nsec, NSEC_PER_SEC, &rem);
+		ts.tv_nsec = rem;
+	} else if (nsec < 0) {
+		/*
+		 * With negative times, tv_sec points to the earlier
+		 * second, and tv_nsec counts the nanoseconds since
+		 * then, so tv_nsec is always a positive number.
+		 */
+		ts.tv_sec = -div_u64_rem(-nsec - 1, NSEC_PER_SEC, &rem) - 1;
+		ts.tv_nsec = NSEC_PER_SEC - rem - 1;
 	}
-	ts.tv_nsec = rem;
 
 	return ts;
 }
-- 
2.20.0

[PATCH 3/8] powerpc: fix vdso32 for ppc64le

[Arnd Bergmann]

On little-endian 32-bit application running on 64-bit kernels,
the current vdso would read the wrong half of the xtime seconds
field. Change it to return the lower half like it does on
big-endian.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/powerpc/kernel/vdso32/gettimeofday.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/vdso32/gettimeofday.S b/arch/powerpc/kernel/vdso32/gettimeofday.S
index becd9f8767ed..4327665ad86f 100644
--- a/arch/powerpc/kernel/vdso32/gettimeofday.S
+++ b/arch/powerpc/kernel/vdso32/gettimeofday.S
@@ -13,7 +13,7 @@
 #include <asm/unistd.h>
 
 /* Offset for the low 32-bit part of a field of long type */
-#ifdef CONFIG_PPC64
+#if defined(CONFIG_PPC64) && defined(CONFIG_CPU_BIG_ENDIAN)
 #define LOPART	4
 #define TSPEC_TV_SEC	TSPC64_TV_SEC+LOPART
 #else
-- 
2.20.0

[PATCH 4/8] ipmi: kill off 'timespec' usage again

[Arnd Bergmann]

'struct timespec' is getting removed from the kernel. The usage in ipmi
was fixed before in commit 48862ea2ce86 ("ipmi: Update timespec usage
to timespec64"), but unfortunately it crept back in.

The busy looping code can better use ktime_t anyway, so use that
there to simplify the implementation.

Fixes: cbb19cb1eef0 ("ipmi_si: Convert timespec64 to timespec")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/char/ipmi/ipmi_si_intf.c | 40 +++++++++++---------------------
 1 file changed, 13 insertions(+), 27 deletions(-)

diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c
index 6b9a0593d2eb..c7cc8538b84a 100644
--- a/drivers/char/ipmi/ipmi_si_intf.c
+++ b/drivers/char/ipmi/ipmi_si_intf.c
@@ -265,10 +265,10 @@ static void cleanup_ipmi_si(void);
 #ifdef DEBUG_TIMING
 void debug_timestamp(char *msg)
 {
-	struct timespec t;
+	struct timespec64 t;
 
-	ktime_get_ts(&t);
-	pr_debug("**%s: %ld.%9.9ld\n", msg, (long) t.tv_sec, t.tv_nsec);
+	ktime_get_ts64(&t);
+	pr_debug("**%s: %lld.%9.9ld\n", msg, t.tv_sec, t.tv_nsec);
 }
 #else
 #define debug_timestamp(x)
@@ -935,38 +935,25 @@ static void set_run_to_completion(void *send_info, bool i_run_to_completion)
 }
 
 /*
- * Use -1 in the nsec value of the busy waiting timespec to tell that
- * we are spinning in kipmid looking for something and not delaying
- * between checks
+ * Use -1 as a special constant to tell that we are spinning in kipmid
+ * looking for something and not delaying between checks
  */
-static inline void ipmi_si_set_not_busy(struct timespec *ts)
-{
-	ts->tv_nsec = -1;
-}
-static inline int ipmi_si_is_busy(struct timespec *ts)
-{
-	return ts->tv_nsec != -1;
-}
-
+#define IPMI_TIME_NOT_BUSY ns_to_ktime(-1ull)
 static inline bool ipmi_thread_busy_wait(enum si_sm_result smi_result,
 					 const struct smi_info *smi_info,
-					 struct timespec *busy_until)
+					 ktime_t *busy_until)
 {
 	unsigned int max_busy_us = 0;
 
 	if (smi_info->si_num < num_max_busy_us)
 		max_busy_us = kipmid_max_busy_us[smi_info->si_num];
 	if (max_busy_us == 0 || smi_result != SI_SM_CALL_WITH_DELAY)
-		ipmi_si_set_not_busy(busy_until);
-	else if (!ipmi_si_is_busy(busy_until)) {
-		ktime_get_ts(busy_until);
-		timespec_add_ns(busy_until, max_busy_us * NSEC_PER_USEC);
+		*busy_until = IPMI_TIME_NOT_BUSY;
+	else if (*busy_until == IPMI_TIME_NOT_BUSY) {
+		*busy_until = ktime_get() + max_busy_us * NSEC_PER_USEC;
 	} else {
-		struct timespec now;
-
-		ktime_get_ts(&now);
-		if (unlikely(timespec_compare(&now, busy_until) > 0)) {
-			ipmi_si_set_not_busy(busy_until);
+		if (unlikely(ktime_get() > *busy_until)) {
+			*busy_until = IPMI_TIME_NOT_BUSY;
 			return false;
 		}
 	}
@@ -988,9 +975,8 @@ static int ipmi_thread(void *data)
 	struct smi_info *smi_info = data;
 	unsigned long flags;
 	enum si_sm_result smi_result;
-	struct timespec busy_until = { 0, 0 };
+	ktime_t busy_until = IPMI_TIME_NOT_BUSY;
 
-	ipmi_si_set_not_busy(&busy_until);
 	set_user_nice(current, MAX_NICE);
 	while (!kthread_should_stop()) {
 		int busy_wait;
-- 
2.20.0

[PATCH 5/8] netfilter: xt_time: use time64_t

[Arnd Bergmann]

The current xt_time driver suffers from the y2038 overflow on 32-bit
architectures, when the time of day calculations break.

Also, on both 32-bit and 64-bit architectures, there is a problem with
info->date_start/stop, which is part of the user ABI and overflows in
in 2106.

Fix the first issue by using time64_t and explicit calls to div_u64()
and div_u64_rem(), and document the seconds issue.

The explicit 64-bit division is unfortunately slower on 32-bit
architectures, but doing it as unsigned lets us use the optimized
division-through-multiplication path in most configurations.  This should
be fine, as the code already does not allow any negative time of day
values.

Using u32 seconds values consistently would probably also work and
be a little more efficient, but that doesn't feel right as it would
propagate the y2106 overflow to more place rather than fewer.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 net/netfilter/xt_time.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/net/netfilter/xt_time.c b/net/netfilter/xt_time.c
index 8dbb4d48f2ed..67cb98489415 100644
--- a/net/netfilter/xt_time.c
+++ b/net/netfilter/xt_time.c
@@ -77,12 +77,12 @@ static inline bool is_leap(unsigned int y)
  * This is done in three separate functions so that the most expensive
  * calculations are done last, in case a "simple match" can be found earlier.
  */
-static inline unsigned int localtime_1(struct xtm *r, time_t time)
+static inline unsigned int localtime_1(struct xtm *r, time64_t time)
 {
 	unsigned int v, w;
 
 	/* Each day has 86400s, so finding the hour/minute is actually easy. */
-	v         = time % SECONDS_PER_DAY;
+	div_u64_rem(time, SECONDS_PER_DAY, &v);
 	r->second = v % 60;
 	w         = v / 60;
 	r->minute = w % 60;
@@ -90,13 +90,13 @@ static inline unsigned int localtime_1(struct xtm *r, time_t time)
 	return v;
 }
 
-static inline void localtime_2(struct xtm *r, time_t time)
+static inline void localtime_2(struct xtm *r, time64_t time)
 {
 	/*
 	 * Here comes the rest (weekday, monthday). First, divide the SSTE
 	 * by seconds-per-day to get the number of _days_ since the epoch.
 	 */
-	r->dse = time / 86400;
+	r->dse = div_u64(time, SECONDS_PER_DAY);
 
 	/*
 	 * 1970-01-01 (w=0) was a Thursday (4).
@@ -105,7 +105,7 @@ static inline void localtime_2(struct xtm *r, time_t time)
 	r->weekday = (4 + r->dse - 1) % 7 + 1;
 }
 
-static void localtime_3(struct xtm *r, time_t time)
+static void localtime_3(struct xtm *r, time64_t time)
 {
 	unsigned int year, i, w = r->dse;
 
@@ -160,7 +160,7 @@ time_mt(const struct sk_buff *skb, struct xt_action_param *par)
 	const struct xt_time_info *info = par->matchinfo;
 	unsigned int packet_time;
 	struct xtm current_time;
-	s64 stamp;
+	time64_t stamp;
 
 	/*
 	 * We need real time here, but we can neither use skb->tstamp
@@ -173,14 +173,14 @@ time_mt(const struct sk_buff *skb, struct xt_action_param *par)
 	 *	1. match before 13:00
 	 *	2. match after 13:00
 	 *
-	 * If you match against processing time (get_seconds) it
+	 * If you match against processing time (ktime_get_real_seconds) it
 	 * may happen that the same packet matches both rules if
 	 * it arrived at the right moment before 13:00, so it would be
 	 * better to check skb->tstamp and set it via __net_timestamp()
 	 * if needed.  This however breaks outgoing packets tx timestamp,
 	 * and causes them to get delayed forever by fq packet scheduler.
 	 */
-	stamp = get_seconds();
+	stamp = ktime_get_real_seconds();
 
 	if (info->flags & XT_TIME_LOCAL_TZ)
 		/* Adjust for local timezone */
@@ -193,6 +193,9 @@ time_mt(const struct sk_buff *skb, struct xt_action_param *par)
 	 *   - 'now' is in the weekday mask
 	 *   - 'now' is in the daytime range time_start..time_end
 	 * (and by default, libxt_time will set these so as to match)
+	 *
+	 * note: info->date_start/stop are unsigned 32-bit values that
+	 *	 can hold values beyond y2038, but not after y2106.
 	 */
 
 	if (stamp < info->date_start || stamp > info->date_stop)
-- 
2.20.0

[PATCH 6/8] lp: fix sparc64 LPSETTIMEOUT ioctl

[Arnd Bergmann]

The layout of struct timeval is different on sparc64 from
anything else, and the patch I did long ago failed to take
this into account.

Change it now to handle sparc64 user space correctly again.

Quite likely nobody cares about parallel ports on sparc64,
but there is no reason not to fix it.

Cc: stable@vger.kernel.org
Fixes: 9a450484089d ("lp: support 64-bit time_t user space")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/char/lp.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/char/lp.c b/drivers/char/lp.c
index 7c9269e3477a..bd95aba1f9fe 100644
--- a/drivers/char/lp.c
+++ b/drivers/char/lp.c
@@ -713,6 +713,10 @@ static int lp_set_timeout64(unsigned int minor, void __user *arg)
 	if (copy_from_user(karg, arg, sizeof(karg)))
 		return -EFAULT;
 
+	/* sparc64 suseconds_t is 32-bit only */
+	if (IS_ENABLED(CONFIG_SPARC64) && !in_compat_syscall())
+		karg[1] >>= 32;
+
 	return lp_set_timeout(minor, karg[0], karg[1]);
 }
 
-- 
2.20.0

[PATCH 7/8] ppdev: fix PPGETTIME/PPSETTIME ioctls

[Arnd Bergmann]

Going through the uses of timeval in the user space API,
I noticed two bugs in ppdev that were introduced in the y2038
conversion:

* The range check was accidentally moved from ppsettime to
  ppgettime

* On sparc64, the microseconds are in the other half of the
  64-bit word.

Fix both, and mark the fix for stable backports.

Cc: stable@vger.kernel.org
Fixes: 3b9ab374a1e6 ("ppdev: convert to y2038 safe")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/char/ppdev.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/char/ppdev.c b/drivers/char/ppdev.c
index c86f18aa8985..34bb88fe0b0a 100644
--- a/drivers/char/ppdev.c
+++ b/drivers/char/ppdev.c
@@ -619,20 +619,27 @@ static int pp_do_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 		if (copy_from_user(time32, argp, sizeof(time32)))
 			return -EFAULT;
 
+		if ((time32[0] < 0) || (time32[1] < 0))
+			return -EINVAL;
+
 		return pp_set_timeout(pp->pdev, time32[0], time32[1]);
 
 	case PPSETTIME64:
 		if (copy_from_user(time64, argp, sizeof(time64)))
 			return -EFAULT;
 
+		if ((time64[0] < 0) || (time64[1] < 0))
+			return -EINVAL;
+
+		if (IS_ENABLED(CONFIG_SPARC64) && !in_compat_syscall())
+			time64[1] >>= 32;
+
 		return pp_set_timeout(pp->pdev, time64[0], time64[1]);
 
 	case PPGETTIME32:
 		jiffies_to_timespec64(pp->pdev->timeout, &ts);
 		time32[0] = ts.tv_sec;
 		time32[1] = ts.tv_nsec / NSEC_PER_USEC;
-		if ((time32[0] < 0) || (time32[1] < 0))
-			return -EINVAL;
 
 		if (copy_to_user(argp, time32, sizeof(time32)))
 			return -EFAULT;
@@ -643,8 +650,9 @@ static int pp_do_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 		jiffies_to_timespec64(pp->pdev->timeout, &ts);
 		time64[0] = ts.tv_sec;
 		time64[1] = ts.tv_nsec / NSEC_PER_USEC;
-		if ((time64[0] < 0) || (time64[1] < 0))
-			return -EINVAL;
+
+		if (IS_ENABLED(CONFIG_SPARC64) && !in_compat_syscall())
+			time64[1] <<= 32;
 
 		if (copy_to_user(argp, time64, sizeof(time64)))
 			return -EFAULT;
-- 
2.20.0

[PATCH 8/8] Input: input_event: fix struct padding on sparc64

[Arnd Bergmann]

Going through all uses of timeval, I noticed that we screwed up
input_event in the previous attempts to fix it:

The time fields now match between kernel and user space, but
all following fields are in the wrong place.

Add the required padding that is implied by the glibc timeval
definition to fix the layout, and add explicit initialization
to avoid leaking kernel stack data.

Cc: sparclinux@vger.kernel.org
Cc: "David S. Miller" <davem@davemloft.net>
Cc: stable@vger.kernel.org
Fixes: 141e5dcaa735 ("Input: input_event - fix the CONFIG_SPARC64 mixup")
Fixes: 2e746942ebac ("Input: input_event - provide override for sparc64")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/input/evdev.c       | 3 +++
 drivers/input/misc/uinput.c | 3 +++
 include/uapi/linux/input.h  | 1 +
 3 files changed, 7 insertions(+)

diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
index d7dd6fcf2db0..24a90793caf0 100644
--- a/drivers/input/evdev.c
+++ b/drivers/input/evdev.c
@@ -228,6 +228,9 @@ static void __pass_event(struct evdev_client *client,
 						event->input_event_sec;
 		client->buffer[client->tail].input_event_usec =
 						event->input_event_usec;
+#ifdef CONFIG_SPARC64
+		client->buffer[client->tail].__pad = 0;
+#endif
 		client->buffer[client->tail].type = EV_SYN;
 		client->buffer[client->tail].code = SYN_DROPPED;
 		client->buffer[client->tail].value = 0;
diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
index 84051f20b18a..1d8c09e9fd47 100644
--- a/drivers/input/misc/uinput.c
+++ b/drivers/input/misc/uinput.c
@@ -80,6 +80,9 @@ static int uinput_dev_event(struct input_dev *dev,
 	ktime_get_ts64(&ts);
 	udev->buff[udev->head].input_event_sec = ts.tv_sec;
 	udev->buff[udev->head].input_event_usec = ts.tv_nsec / NSEC_PER_USEC;
+#ifdef CONFIG_SPARC64
+	udev->buff[udev->head].__pad = 0;
+#endif
 	udev->head = (udev->head + 1) % UINPUT_BUFFER_SIZE;
 
 	wake_up_interruptible(&udev->waitq);
diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
index f056b2a00d5c..9a61c28ed3ae 100644
--- a/include/uapi/linux/input.h
+++ b/include/uapi/linux/input.h
@@ -34,6 +34,7 @@ struct input_event {
 	__kernel_ulong_t __sec;
 #if defined(__sparc__) && defined(__arch64__)
 	unsigned int __usec;
+	unsigned int __pad;
 #else
 	__kernel_ulong_t __usec;
 #endif
-- 
2.20.0

Re: [PATCH 4/8] ipmi: kill off 'timespec' usage again

[Corey Minyard]

On Fri, Nov 08, 2019 at 09:34:27PM +0100, Arnd Bergmann wrote:
> 'struct timespec' is getting removed from the kernel. The usage in ipmi
> was fixed before in commit 48862ea2ce86 ("ipmi: Update timespec usage
> to timespec64"), but unfortunately it crept back in.
> 
> The busy looping code can better use ktime_t anyway, so use that
> there to simplify the implementation.

Thanks, this is a big improvement.  I have this queued, but if you
are going to submit this, I can remove it, and:

Reviewed-by: Corey Minyard <cminyard@mvista.com>

Do you think this should go in to 5.4?

-corey

> 
> Fixes: cbb19cb1eef0 ("ipmi_si: Convert timespec64 to timespec")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/char/ipmi/ipmi_si_intf.c | 40 +++++++++++---------------------
>  1 file changed, 13 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c
> index 6b9a0593d2eb..c7cc8538b84a 100644
> --- a/drivers/char/ipmi/ipmi_si_intf.c
> +++ b/drivers/char/ipmi/ipmi_si_intf.c
> @@ -265,10 +265,10 @@ static void cleanup_ipmi_si(void);
>  #ifdef DEBUG_TIMING
>  void debug_timestamp(char *msg)
>  {
> -	struct timespec t;
> +	struct timespec64 t;
>  
> -	ktime_get_ts(&t);
> -	pr_debug("**%s: %ld.%9.9ld\n", msg, (long) t.tv_sec, t.tv_nsec);
> +	ktime_get_ts64(&t);
> +	pr_debug("**%s: %lld.%9.9ld\n", msg, t.tv_sec, t.tv_nsec);
>  }
>  #else
>  #define debug_timestamp(x)
> @@ -935,38 +935,25 @@ static void set_run_to_completion(void *send_info, bool i_run_to_completion)
>  }
>  
>  /*
> - * Use -1 in the nsec value of the busy waiting timespec to tell that
> - * we are spinning in kipmid looking for something and not delaying
> - * between checks
> + * Use -1 as a special constant to tell that we are spinning in kipmid
> + * looking for something and not delaying between checks
>   */
> -static inline void ipmi_si_set_not_busy(struct timespec *ts)
> -{
> -	ts->tv_nsec = -1;
> -}
> -static inline int ipmi_si_is_busy(struct timespec *ts)
> -{
> -	return ts->tv_nsec != -1;
> -}
> -
> +#define IPMI_TIME_NOT_BUSY ns_to_ktime(-1ull)
>  static inline bool ipmi_thread_busy_wait(enum si_sm_result smi_result,
>  					 const struct smi_info *smi_info,
> -					 struct timespec *busy_until)
> +					 ktime_t *busy_until)
>  {
>  	unsigned int max_busy_us = 0;
>  
>  	if (smi_info->si_num < num_max_busy_us)
>  		max_busy_us = kipmid_max_busy_us[smi_info->si_num];
>  	if (max_busy_us == 0 || smi_result != SI_SM_CALL_WITH_DELAY)
> -		ipmi_si_set_not_busy(busy_until);
> -	else if (!ipmi_si_is_busy(busy_until)) {
> -		ktime_get_ts(busy_until);
> -		timespec_add_ns(busy_until, max_busy_us * NSEC_PER_USEC);
> +		*busy_until = IPMI_TIME_NOT_BUSY;
> +	else if (*busy_until == IPMI_TIME_NOT_BUSY) {
> +		*busy_until = ktime_get() + max_busy_us * NSEC_PER_USEC;
>  	} else {
> -		struct timespec now;
> -
> -		ktime_get_ts(&now);
> -		if (unlikely(timespec_compare(&now, busy_until) > 0)) {
> -			ipmi_si_set_not_busy(busy_until);
> +		if (unlikely(ktime_get() > *busy_until)) {
> +			*busy_until = IPMI_TIME_NOT_BUSY;
>  			return false;
>  		}
>  	}
> @@ -988,9 +975,8 @@ static int ipmi_thread(void *data)
>  	struct smi_info *smi_info = data;
>  	unsigned long flags;
>  	enum si_sm_result smi_result;
> -	struct timespec busy_until = { 0, 0 };
> +	ktime_t busy_until = IPMI_TIME_NOT_BUSY;
>  
> -	ipmi_si_set_not_busy(&busy_until);
>  	set_user_nice(current, MAX_NICE);
>  	while (!kthread_should_stop()) {
>  		int busy_wait;
> -- 
> 2.20.0
> 

Re: [PATCH 4/8] ipmi: kill off 'timespec' usage again

[Arnd Bergmann]

On Fri, Nov 8, 2019 at 11:11 PM Corey Minyard <cminyard@mvista.com> wrote:
>
> On Fri, Nov 08, 2019 at 09:34:27PM +0100, Arnd Bergmann wrote:
> > 'struct timespec' is getting removed from the kernel. The usage in ipmi
> > was fixed before in commit 48862ea2ce86 ("ipmi: Update timespec usage
> > to timespec64"), but unfortunately it crept back in.
> >
> > The busy looping code can better use ktime_t anyway, so use that
> > there to simplify the implementation.
>
> Thanks, this is a big improvement.  I have this queued, but if you
> are going to submit this, I can remove it, and:
>
> Reviewed-by: Corey Minyard <cminyard@mvista.com>

I'd prefer to have this go through your tree, one less thing for me
to worry about (out of the 90 patches).

> Do you think this should go in to 5.4?

Up to you, it probably depends on how well you can test that the change
is correct beyond the review.

       Arnd

Re: [PATCH 1/8] y2038: timex: remove incorrect time_t truncation

[Deepa Dinamani]

Thanks for fixing the bug.

Acked-by: Deepa Dinamani <deepa.kernel@gmail.com>

Re: [PATCH 2/8] timekeeping: optimize ns_to_timespec64

[Deepa Dinamani]

The arithmetic looks right to me.

Acked-by: Deepa Dinamani <deepa.kernel@gmail.com>

Re: [PATCH 8/8] Input: input_event: fix struct padding on sparc64

[Dmitry Torokhov]

Hi Arnd,

On Fri, Nov 08, 2019 at 09:34:31PM +0100, Arnd Bergmann wrote:
> Going through all uses of timeval, I noticed that we screwed up
> input_event in the previous attempts to fix it:
> 
> The time fields now match between kernel and user space, but
> all following fields are in the wrong place.
> 
> Add the required padding that is implied by the glibc timeval
> definition to fix the layout, and add explicit initialization
> to avoid leaking kernel stack data.
> 
> Cc: sparclinux@vger.kernel.org
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: stable@vger.kernel.org
> Fixes: 141e5dcaa735 ("Input: input_event - fix the CONFIG_SPARC64 mixup")
> Fixes: 2e746942ebac ("Input: input_event - provide override for sparc64")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/input/evdev.c       | 3 +++
>  drivers/input/misc/uinput.c | 3 +++
>  include/uapi/linux/input.h  | 1 +
>  3 files changed, 7 insertions(+)
> 
> diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
> index d7dd6fcf2db0..24a90793caf0 100644
> --- a/drivers/input/evdev.c
> +++ b/drivers/input/evdev.c
> @@ -228,6 +228,9 @@ static void __pass_event(struct evdev_client *client,
>  						event->input_event_sec;
>  		client->buffer[client->tail].input_event_usec =
>  						event->input_event_usec;
> +#ifdef CONFIG_SPARC64
> +		client->buffer[client->tail].__pad = 0;
> +#endif
>  		client->buffer[client->tail].type = EV_SYN;
>  		client->buffer[client->tail].code = SYN_DROPPED;
>  		client->buffer[client->tail].value = 0;

I do not like ifdefs here, do you think we could write:

		client->buffer[client->tail] = (struct input_event) {
			.input_event_sec = event->input_event_sec,
			.input_event_usec = event->input_event_usec,
			.type = EV_SYN,
			.code = SYN_DROPPED,
		};

to ensure all padded fields are initialized? This is not hot path as we
do not expect queue to overfill too often.

Thanks.

-- 
Dmitry

Re: [PATCH 8/8] Input: input_event: fix struct padding on sparc64

[Arnd Bergmann]

On Mon, Nov 11, 2019 at 7:28 PM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:

> I do not like ifdefs here, do you think we could write:
>
>                 client->buffer[client->tail] = (struct input_event) {
>                         .input_event_sec = event->input_event_sec,
>                         .input_event_usec = event->input_event_usec,
>                         .type = EV_SYN,
>                         .code = SYN_DROPPED,
>                 };
>
> to ensure all padded fields are initialized? This is not hot path as we
> do not expect queue to overfill too often.

Good idea, changed both instances now. Thanks for taking a look!

      Arnd

[tip: timers/urgent] ntp/y2038: Remove incorrect time_t truncation

[tip-bot2 for Arnd Bergmann]

The following commit has been merged into the timers/urgent branch of tip:

Commit-ID:     2f5841349df281ecf8f81cc82d869b8476f0db0b
Gitweb:        https://git.kernel.org/tip/2f5841349df281ecf8f81cc82d869b8476f0db0b
Author:        Arnd Bergmann <arnd@arndb.de>
AuthorDate:    Fri, 08 Nov 2019 21:34:24 +01:00
Committer:     Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Tue, 12 Nov 2019 08:13:44 +01:00

ntp/y2038: Remove incorrect time_t truncation

A cast to 'time_t' was accidentally left in place during the
conversion of __do_adjtimex() to 64-bit timestamps, so the
resulting value is incorrectly truncated.

Remove the cast so the 64-bit time gets propagated correctly.

Fixes: ead25417f82e ("timex: use __kernel_timex internally")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: stable@vger.kernel.org
Link: https://lkml.kernel.org/r/20191108203435.112759-2-arnd@arndb.de

---
 kernel/time/ntp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index 65eb796..069ca78 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
@@ -771,7 +771,7 @@ int __do_adjtimex(struct __kernel_timex *txc, const struct timespec64 *ts,
 	/* fill PPS status fields */
 	pps_fill_timex(txc);
 
-	txc->time.tv_sec = (time_t)ts->tv_sec;
+	txc->time.tv_sec = ts->tv_sec;
 	txc->time.tv_usec = ts->tv_nsec;
 	if (!(time_status & STA_NANO))
 		txc->time.tv_usec = ts->tv_nsec / NSEC_PER_USEC;

[tip: timers/core] time: Optimize ns_to_timespec64()

[tip-bot2 for Arnd Bergmann]

The following commit has been merged into the timers/core branch of tip:

Commit-ID:     20d087368d38c7350a4519a3b316ef7eb2504692
Gitweb:        https://git.kernel.org/tip/20d087368d38c7350a4519a3b316ef7eb2504692
Author:        Arnd Bergmann <arnd@arndb.de>
AuthorDate:    Fri, 08 Nov 2019 21:34:25 +01:00
Committer:     Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Tue, 12 Nov 2019 08:15:15 +01:00

time: Optimize ns_to_timespec64()

ns_to_timespec64() calls div_s64_rem(), which is a rather slow function on
32-bit architectures, as it cannot take advantage of the do_div()
optimizations for constant arguments.

Open-code the div_s64_rem() function in ns_to_timespec64(), so a constant
divider can be passed into the optimized div_u64_rem() function.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Link: https://lkml.kernel.org/r/20191108203435.112759-3-arnd@arndb.de
---
 kernel/time/time.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/kernel/time/time.c b/kernel/time/time.c
index 5c54ca6..45a3589 100644
--- a/kernel/time/time.c
+++ b/kernel/time/time.c
@@ -550,18 +550,21 @@ EXPORT_SYMBOL(set_normalized_timespec64);
  */
 struct timespec64 ns_to_timespec64(const s64 nsec)
 {
-	struct timespec64 ts;
+	struct timespec64 ts = { 0, 0 };
 	s32 rem;
 
-	if (!nsec)
-		return (struct timespec64) {0, 0};
-
-	ts.tv_sec = div_s64_rem(nsec, NSEC_PER_SEC, &rem);
-	if (unlikely(rem < 0)) {
-		ts.tv_sec--;
-		rem += NSEC_PER_SEC;
+	if (likely(nsec > 0)) {
+		ts.tv_sec = div_u64_rem(nsec, NSEC_PER_SEC, &rem);
+		ts.tv_nsec = rem;
+	} else if (nsec < 0) {
+		/*
+		 * With negative times, tv_sec points to the earlier
+		 * second, and tv_nsec counts the nanoseconds since
+		 * then, so tv_nsec is always a positive number.
+		 */
+		ts.tv_sec = -div_u64_rem(-nsec - 1, NSEC_PER_SEC, &rem) - 1;
+		ts.tv_nsec = NSEC_PER_SEC - rem - 1;
 	}
-	ts.tv_nsec = rem;
 
 	return ts;
 }

Re: [PATCH 5/8] netfilter: xt_time: use time64_t

[Pablo Neira Ayuso]

On Fri, Nov 08, 2019 at 09:34:28PM +0100, Arnd Bergmann wrote:
> The current xt_time driver suffers from the y2038 overflow on 32-bit
> architectures, when the time of day calculations break.
> 
> Also, on both 32-bit and 64-bit architectures, there is a problem with
> info->date_start/stop, which is part of the user ABI and overflows in
> in 2106.
> 
> Fix the first issue by using time64_t and explicit calls to div_u64()
> and div_u64_rem(), and document the seconds issue.
> 
> The explicit 64-bit division is unfortunately slower on 32-bit
> architectures, but doing it as unsigned lets us use the optimized
> division-through-multiplication path in most configurations.  This should
> be fine, as the code already does not allow any negative time of day
> values.
> 
> Using u32 seconds values consistently would probably also work and
> be a little more efficient, but that doesn't feel right as it would
> propagate the y2106 overflow to more place rather than fewer.

Applied, thanks.

Re: [Y2038] [PATCH 3/8] powerpc: fix vdso32 for ppc64le

[Ben Hutchings]

On Fri, 2019-11-08 at 21:34 +0100, Arnd Bergmann wrote:
> On little-endian 32-bit application running on 64-bit kernels,
> the current vdso would read the wrong half of the xtime seconds
> field. Change it to return the lower half like it does on
> big-endian.

ppc64le doesn't have 32-bit compat so this is only theoretical.

Ben.

> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  arch/powerpc/kernel/vdso32/gettimeofday.S | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/kernel/vdso32/gettimeofday.S b/arch/powerpc/kernel/vdso32/gettimeofday.S
> index becd9f8767ed..4327665ad86f 100644
> --- a/arch/powerpc/kernel/vdso32/gettimeofday.S
> +++ b/arch/powerpc/kernel/vdso32/gettimeofday.S
> @@ -13,7 +13,7 @@
>  #include <asm/unistd.h>
>  
>  /* Offset for the low 32-bit part of a field of long type */
> -#ifdef CONFIG_PPC64
> +#if defined(CONFIG_PPC64) && defined(CONFIG_CPU_BIG_ENDIAN)
>  #define LOPART	4
>  #define TSPEC_TV_SEC	TSPC64_TV_SEC+LOPART
>  #else
-- 
Ben Hutchings, Software Developer                         Codethink Ltd
https://www.codethink.co.uk/                 Dale House, 35 Dale Street
                                     Manchester, M1 2HF, United Kingdom

Re: [Y2038] [PATCH 6/8] lp: fix sparc64 LPSETTIMEOUT ioctl

[Ben Hutchings]

On Fri, 2019-11-08 at 21:34 +0100, Arnd Bergmann wrote:
> The layout of struct timeval is different on sparc64 from
> anything else, and the patch I did long ago failed to take
> this into account.
> 
> Change it now to handle sparc64 user space correctly again.
> 
> Quite likely nobody cares about parallel ports on sparc64,
> but there is no reason not to fix it.
> 
> Cc: stable@vger.kernel.org
> Fixes: 9a450484089d ("lp: support 64-bit time_t user space")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/char/lp.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/char/lp.c b/drivers/char/lp.c
> index 7c9269e3477a..bd95aba1f9fe 100644
> --- a/drivers/char/lp.c
> +++ b/drivers/char/lp.c
> @@ -713,6 +713,10 @@ static int lp_set_timeout64(unsigned int minor, void __user *arg)
>  	if (copy_from_user(karg, arg, sizeof(karg)))
>  		return -EFAULT;
>  
> +	/* sparc64 suseconds_t is 32-bit only */
> +	if (IS_ENABLED(CONFIG_SPARC64) && !in_compat_syscall())
> +		karg[1] >>= 32;
> +
>  	return lp_set_timeout(minor, karg[0], karg[1]);
>  }
>

It seems like it would make way more sense to use __kernel_old_timeval.
Then you don't have to explicitly handle the sparc64 oddity.

As it is, this still over-reads from user-space which might result in a
spurious -EFAULT.

Ben.

-- 
Ben Hutchings, Software Developer                         Codethink Ltd
https://www.codethink.co.uk/                 Dale House, 35 Dale Street
                                     Manchester, M1 2HF, United Kingdom

Re: [Y2038] [PATCH 7/8] ppdev: fix PPGETTIME/PPSETTIME ioctls

[Ben Hutchings]

On Fri, 2019-11-08 at 21:34 +0100, Arnd Bergmann wrote:
> Going through the uses of timeval in the user space API,
> I noticed two bugs in ppdev that were introduced in the y2038
> conversion:
> 
> * The range check was accidentally moved from ppsettime to
>   ppgettime
> 
> * On sparc64, the microseconds are in the other half of the
>   64-bit word.
> 
> Fix both, and mark the fix for stable backports.

Like the patch for lpdev, this also doesn't completely fix sparc64.

Ben.

> Cc: stable@vger.kernel.org
> Fixes: 3b9ab374a1e6 ("ppdev: convert to y2038 safe")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/char/ppdev.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/char/ppdev.c b/drivers/char/ppdev.c
> index c86f18aa8985..34bb88fe0b0a 100644
> --- a/drivers/char/ppdev.c
> +++ b/drivers/char/ppdev.c
> @@ -619,20 +619,27 @@ static int pp_do_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
>  		if (copy_from_user(time32, argp, sizeof(time32)))
>  			return -EFAULT;
>  
> +		if ((time32[0] < 0) || (time32[1] < 0))
> +			return -EINVAL;
> +
>  		return pp_set_timeout(pp->pdev, time32[0], time32[1]);
>  
>  	case PPSETTIME64:
>  		if (copy_from_user(time64, argp, sizeof(time64)))
>  			return -EFAULT;
>  
> +		if ((time64[0] < 0) || (time64[1] < 0))
> +			return -EINVAL;
> +
> +		if (IS_ENABLED(CONFIG_SPARC64) && !in_compat_syscall())
> +			time64[1] >>= 32;
> +
>  		return pp_set_timeout(pp->pdev, time64[0], time64[1]);
>  
>  	case PPGETTIME32:
>  		jiffies_to_timespec64(pp->pdev->timeout, &ts);
>  		time32[0] = ts.tv_sec;
>  		time32[1] = ts.tv_nsec / NSEC_PER_USEC;
> -		if ((time32[0] < 0) || (time32[1] < 0))
> -			return -EINVAL;
>  
>  		if (copy_to_user(argp, time32, sizeof(time32)))
>  			return -EFAULT;
> @@ -643,8 +650,9 @@ static int pp_do_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
>  		jiffies_to_timespec64(pp->pdev->timeout, &ts);
>  		time64[0] = ts.tv_sec;
>  		time64[1] = ts.tv_nsec / NSEC_PER_USEC;
> -		if ((time64[0] < 0) || (time64[1] < 0))
> -			return -EINVAL;
> +
> +		if (IS_ENABLED(CONFIG_SPARC64) && !in_compat_syscall())
> +			time64[1] <<= 32;
>  
>  		if (copy_to_user(argp, time64, sizeof(time64)))
>  			return -EFAULT;
-- 
Ben Hutchings, Software Developer                         Codethink Ltd
https://www.codethink.co.uk/                 Dale House, 35 Dale Street
                                     Manchester, M1 2HF, United Kingdom

Re: [Y2038] [PATCH 3/8] powerpc: fix vdso32 for ppc64le

[Arnd Bergmann]

On Wed, Nov 20, 2019 at 8:13 PM Ben Hutchings
<ben.hutchings@codethink.co.uk> wrote:
>
> On Fri, 2019-11-08 at 21:34 +0100, Arnd Bergmann wrote:
> > On little-endian 32-bit application running on 64-bit kernels,
> > the current vdso would read the wrong half of the xtime seconds
> > field. Change it to return the lower half like it does on
> > big-endian.
>
> ppc64le doesn't have 32-bit compat so this is only theoretical.

That is probably true. I only looked at the kernel, which today still
supports compat mode for ppc64le, but I saw the patches to disable
it, and I don't think anyone has even attempted building user space
for it.

       Arnd

Re: [Y2038] [PATCH 6/8] lp: fix sparc64 LPSETTIMEOUT ioctl

[Arnd Bergmann]

On Wed, Nov 20, 2019 at 8:27 PM Ben Hutchings
<ben.hutchings@codethink.co.uk> wrote:
>
> On Fri, 2019-11-08 at 21:34 +0100, Arnd Bergmann wrote:
> > The layout of struct timeval is different on sparc64 from
> > anything else, and the patch I did long ago failed to take
> > this into account.
> >
> > Change it now to handle sparc64 user space correctly again.
> >
> > Quite likely nobody cares about parallel ports on sparc64,
> > but there is no reason not to fix it.
> >
> > Cc: stable@vger.kernel.org
> > Fixes: 9a450484089d ("lp: support 64-bit time_t user space")
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> >  drivers/char/lp.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/char/lp.c b/drivers/char/lp.c
> > index 7c9269e3477a..bd95aba1f9fe 100644
> > --- a/drivers/char/lp.c
> > +++ b/drivers/char/lp.c
> > @@ -713,6 +713,10 @@ static int lp_set_timeout64(unsigned int minor, void __user *arg)
> >       if (copy_from_user(karg, arg, sizeof(karg)))
> >               return -EFAULT;
> >
> > +     /* sparc64 suseconds_t is 32-bit only */
> > +     if (IS_ENABLED(CONFIG_SPARC64) && !in_compat_syscall())
> > +             karg[1] >>= 32;
> > +
> >       return lp_set_timeout(minor, karg[0], karg[1]);
> >  }
> >
>
> It seems like it would make way more sense to use __kernel_old_timeval.

Right, that would work. I tried to keep the patch small here, changing
it to __kernel_old_timeval would require make it all more complicated
since it would still need to check some conditional to tell the difference
between sparc32 and sparc64.

I think this patch (relative to the version I posted) would work the same:

diff --git a/drivers/char/lp.c b/drivers/char/lp.c
index bd95aba1f9fe..86994421ee97 100644
--- a/drivers/char/lp.c
+++ b/drivers/char/lp.c
@@ -713,13 +713,19 @@ static int lp_set_timeout64(unsigned int minor,
void __user *arg)
        if (copy_from_user(karg, arg, sizeof(karg)))
                return -EFAULT;

-       /* sparc64 suseconds_t is 32-bit only */
-       if (IS_ENABLED(CONFIG_SPARC64) && !in_compat_syscall())
-               karg[1] >>= 32;
-
        return lp_set_timeout(minor, karg[0], karg[1]);
 }

+static int lp_set_timeout(unsigned int minor, void __user *arg)
+{
+       __kernel_old_timeval tv;
+
+       if (copy_from_user(tv, arg, sizeof(karg)))
+               return -EFAULT;
+
+       return lp_set_timeout(minor, tv->tv_sec, tv->tv_usec);
+}
+
 static long lp_ioctl(struct file *file, unsigned int cmd,
                        unsigned long arg)
 {
@@ -730,11 +736,8 @@ static long lp_ioctl(struct file *file, unsigned int cmd,
        mutex_lock(&lp_mutex);
        switch (cmd) {
        case LPSETTIMEOUT_OLD:
-               if (BITS_PER_LONG == 32) {
-                       ret = lp_set_timeout32(minor, (void __user *)arg);
-                       break;
-               }
-               /* fall through - for 64-bit */
+               ret = lp_set_timeout(minor, (void __user *)arg);
+               break;
        case LPSETTIMEOUT_NEW:
                ret = lp_set_timeout64(minor, (void __user *)arg);
                break;

Do you like that better? One difference here is the handling of
LPSETTIMEOUT_NEW on sparc64, which would continue to use
the 64/64 layout rather than the 64/32/pad layout, but that should
be ok, since sparc64 user space using ppdev (if any exists)
would use LPSETTIMEOUT_OLD, not LPSETTIMEOUT_NEW.

> Then you don't have to explicitly handle the sparc64 oddity.
>
> As it is, this still over-reads from user-space which might result in a
> spurious -EFAULT.

I think you got this wrong: sparc64 like most architectures naturally
aligns 64-bit members, so 'struct timeval' still uses 16 bytes including
the four padding bytes at the end, it just has the nanoseconds in
a different position from all other big-endian architectures.

      Arnd

Re: [Y2038] [PATCH 3/8] powerpc: fix vdso32 for ppc64le

[Ben Hutchings]

On Wed, 2019-11-20 at 20:35 +0100, Arnd Bergmann wrote:
> On Wed, Nov 20, 2019 at 8:13 PM Ben Hutchings
> <ben.hutchings@codethink.co.uk> wrote:
> > On Fri, 2019-11-08 at 21:34 +0100, Arnd Bergmann wrote:
> > > On little-endian 32-bit application running on 64-bit kernels,
> > > the current vdso would read the wrong half of the xtime seconds
> > > field. Change it to return the lower half like it does on
> > > big-endian.
> > 
> > ppc64le doesn't have 32-bit compat so this is only theoretical.
> 
> That is probably true. I only looked at the kernel, which today still
> supports compat mode for ppc64le, but I saw the patches to disable
> it, and I don't think anyone has even attempted building user space
> for it.

COMPAT is still enabled for some reason, but VDSO32 isn't (since 4.2).

Ben.

-- 
Ben Hutchings, Software Developer                         Codethink Ltd
https://www.codethink.co.uk/                 Dale House, 35 Dale Street
                                     Manchester, M1 2HF, United Kingdom

Re: [Y2038] [PATCH 6/8] lp: fix sparc64 LPSETTIMEOUT ioctl

[Ben Hutchings]

On Wed, 2019-11-20 at 20:46 +0100, Arnd Bergmann wrote:
> On Wed, Nov 20, 2019 at 8:27 PM Ben Hutchings
> <ben.hutchings@codethink.co.uk> wrote:
> > On Fri, 2019-11-08 at 21:34 +0100, Arnd Bergmann wrote:
> > > The layout of struct timeval is different on sparc64 from
> > > anything else, and the patch I did long ago failed to take
> > > this into account.
> > > 
> > > Change it now to handle sparc64 user space correctly again.
> > > 
> > > Quite likely nobody cares about parallel ports on sparc64,
> > > but there is no reason not to fix it.
> > > 
> > > Cc: stable@vger.kernel.org
> > > Fixes: 9a450484089d ("lp: support 64-bit time_t user space")
> > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > > ---
> > >  drivers/char/lp.c | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > > 
> > > diff --git a/drivers/char/lp.c b/drivers/char/lp.c
> > > index 7c9269e3477a..bd95aba1f9fe 100644
> > > --- a/drivers/char/lp.c
> > > +++ b/drivers/char/lp.c
> > > @@ -713,6 +713,10 @@ static int lp_set_timeout64(unsigned int minor, void __user *arg)
> > >       if (copy_from_user(karg, arg, sizeof(karg)))
> > >               return -EFAULT;
> > > 
> > > +     /* sparc64 suseconds_t is 32-bit only */
> > > +     if (IS_ENABLED(CONFIG_SPARC64) && !in_compat_syscall())
> > > +             karg[1] >>= 32;
> > > +
> > >       return lp_set_timeout(minor, karg[0], karg[1]);
> > >  }
> > > 
> > 
> > It seems like it would make way more sense to use __kernel_old_timeval.
> 
> Right, that would work. I tried to keep the patch small here, changing
> it to __kernel_old_timeval would require make it all more complicated
> since it would still need to check some conditional to tell the difference
> between sparc32 and sparc64.

Right.

> I think this patch (relative to the version I posted) would work the same:
> 
> diff --git a/drivers/char/lp.c b/drivers/char/lp.c
> index bd95aba1f9fe..86994421ee97 100644
> --- a/drivers/char/lp.c
> +++ b/drivers/char/lp.c
> @@ -713,13 +713,19 @@ static int lp_set_timeout64(unsigned int minor,
> void __user *arg)
>         if (copy_from_user(karg, arg, sizeof(karg)))
>                 return -EFAULT;
> 
> -       /* sparc64 suseconds_t is 32-bit only */
> -       if (IS_ENABLED(CONFIG_SPARC64) && !in_compat_syscall())
> -               karg[1] >>= 32;
> -
>         return lp_set_timeout(minor, karg[0], karg[1]);
>  }
> 
> +static int lp_set_timeout(unsigned int minor, void __user *arg)

That function name is already used!  Maybe this should be
lp_set_timeout_old()?

> +{
> +       __kernel_old_timeval tv;
> +
> +       if (copy_from_user(tv, arg, sizeof(karg)))
> +               return -EFAULT;
> +
> +       return lp_set_timeout(minor, tv->tv_sec, tv->tv_usec);
> +}
> +
>  static long lp_ioctl(struct file *file, unsigned int cmd,
>                         unsigned long arg)
>  {
> @@ -730,11 +736,8 @@ static long lp_ioctl(struct file *file, unsigned int cmd,
>         mutex_lock(&lp_mutex);
>         switch (cmd) {
>         case LPSETTIMEOUT_OLD:
> -               if (BITS_PER_LONG == 32) {
> -                       ret = lp_set_timeout32(minor, (void __user *)arg);
> -                       break;
> -               }
> -               /* fall through - for 64-bit */
> +               ret = lp_set_timeout(minor, (void __user *)arg);
> +               break;
>         case LPSETTIMEOUT_NEW:
>                 ret = lp_set_timeout64(minor, (void __user *)arg);
>                 break;
> 
> Do you like that better?

Yes.  Aside from the duplicate function name, it looks correct and
cleaner than the current version.

> One difference here is the handling of
> LPSETTIMEOUT_NEW on sparc64, which would continue to use
> the 64/64 layout rather than the 64/32/pad layout, but that should
> be ok, since sparc64 user space using ppdev (if any exists)
> would use LPSETTIMEOUT_OLD, not LPSETTIMEOUT_NEW.

Right, that's a little weird but appears to be consistent with "new"
socket timestamps.

> > Then you don't have to explicitly handle the sparc64 oddity.
> > 
> > As it is, this still over-reads from user-space which might result in a
> > spurious -EFAULT.
> 
> I think you got this wrong: sparc64 like most architectures naturally
> aligns 64-bit members, so 'struct timeval' still uses 16 bytes including
> the four padding bytes at the end, it just has the nanoseconds in
> a different position from all other big-endian architectures.

Oh of course, yes.

Ben.

-- 
Ben Hutchings, Software Developer                         Codethink Ltd
https://www.codethink.co.uk/                 Dale House, 35 Dale Street
                                     Manchester, M1 2HF, United Kingdom

Re: [Y2038] [PATCH 3/8] powerpc: fix vdso32 for ppc64le

[Arnd Bergmann]

On Wed, Nov 20, 2019 at 10:49 PM Ben Hutchings
<ben.hutchings@codethink.co.uk> wrote:
> On Wed, 2019-11-20 at 20:35 +0100, Arnd Bergmann wrote:
> > On Wed, Nov 20, 2019 at 8:13 PM Ben Hutchings
> > <ben.hutchings@codethink.co.uk> wrote:
> > > On Fri, 2019-11-08 at 21:34 +0100, Arnd Bergmann wrote:
> > > > On little-endian 32-bit application running on 64-bit kernels,
> > > > the current vdso would read the wrong half of the xtime seconds
> > > > field. Change it to return the lower half like it does on
> > > > big-endian.
> > >
> > > ppc64le doesn't have 32-bit compat so this is only theoretical.
> >
> > That is probably true. I only looked at the kernel, which today still
> > supports compat mode for ppc64le, but I saw the patches to disable
> > it, and I don't think anyone has even attempted building user space
> > for it.
>
> COMPAT is still enabled for some reason, but VDSO32 isn't (since 4.2).

Ok, I had missed that detail. Should I just drop my patch then?

      Arnd

Re: [Y2038] [PATCH 6/8] lp: fix sparc64 LPSETTIMEOUT ioctl

[Arnd Bergmann]

On Wed, Nov 20, 2019 at 11:10 PM Ben Hutchings
<ben.hutchings@codethink.co.uk> wrote:
> On Wed, 2019-11-20 at 20:46 +0100, Arnd Bergmann wrote:
> > On Wed, Nov 20, 2019 at 8:27 PM Ben Hutchings <ben.hutchings@codethink.co.uk> wrote:
> > -
> >         return lp_set_timeout(minor, karg[0], karg[1]);
> >  }
> >
> > +static int lp_set_timeout(unsigned int minor, void __user *arg)
>
> That function name is already used!  Maybe this should be
> lp_set_timeout_old()?

Yes, that's what I used after actually compile-testing and running
into a couple of issues with my draft.

> > @@ -730,11 +736,8 @@ static long lp_ioctl(struct file *file, unsigned int cmd,
> >         mutex_lock(&lp_mutex);
> >         switch (cmd) {
> >         case LPSETTIMEOUT_OLD:
> > -               if (BITS_PER_LONG == 32) {
> > -                       ret = lp_set_timeout32(minor, (void __user *)arg);
> > -                       break;
> > -               }
> > -               /* fall through - for 64-bit */
> > +               ret = lp_set_timeout(minor, (void __user *)arg);
> > +               break;
> >         case LPSETTIMEOUT_NEW:
> >                 ret = lp_set_timeout64(minor, (void __user *)arg);
> >                 break;
> >
> > Do you like that better?
>
> Yes.  Aside from the duplicate function name, it looks correct and
> cleaner than the current version.

As Greg has already merged the original patch, and that version works
just as well, I'd probably just leave what I did at first. One benefit is
that in case we decide to kill off sparc64 support before drivers/char/lp.c,
the special case can be removed more easily.

I don't think either of them is going any time soon, but working on y2038
patches has made me think ahead longer term ;-)

If you still think we should change it I can send the below patch (now
actually build-tested) with your Ack.

       Arnd
---
commit 93efbb1768a5071a0a98bf4627f0104075cf83a6 (HEAD -> y2038)
Author: Arnd Bergmann <arnd@arndb.de>
Date:   Thu Nov 21 14:45:14 2019 +0100

    lp: clean up set_timeout handling

    As Ben Hutchings noticed, we can avoid the special case for sparc64
    by dealing with '__kernel_old_timeval' arguments separately from
    the fixed-length 32-bit and 64-bit arguments.

    Note that the behavior for LPSETTIMEOUT_NEW changes on sparc64 to
    expect the same argument as other architectures, but this is ok
    because sparc64 users would pass LPSETTIMEOUT_OLD anyway.

    Suggested-by: Ben Hutchings <ben.hutchings@codethink.co.uk>
    Signed-off-by: Arnd Bergmann <arnd@arndb.de>

diff --git a/drivers/char/lp.c b/drivers/char/lp.c
index bd95aba1f9fe..cc17d5a387c5 100644
--- a/drivers/char/lp.c
+++ b/drivers/char/lp.c
@@ -696,14 +696,14 @@ static int lp_set_timeout(unsigned int minor,
s64 tv_sec, long tv_usec)
        return 0;
 }

-static int lp_set_timeout32(unsigned int minor, void __user *arg)
+static int lp_set_timeout_old(unsigned int minor, void __user *arg)
 {
-       s32 karg[2];
+       struct __kernel_old_timeval tv;

-       if (copy_from_user(karg, arg, sizeof(karg)))
+       if (copy_from_user(&tv, arg, sizeof(tv)))
                return -EFAULT;

-       return lp_set_timeout(minor, karg[0], karg[1]);
+       return lp_set_timeout(minor, tv.tv_sec, tv.tv_usec);
 }

 static int lp_set_timeout64(unsigned int minor, void __user *arg)
@@ -713,10 +713,6 @@ static int lp_set_timeout64(unsigned int minor,
void __user *arg)
        if (copy_from_user(karg, arg, sizeof(karg)))
                return -EFAULT;

-       /* sparc64 suseconds_t is 32-bit only */
-       if (IS_ENABLED(CONFIG_SPARC64) && !in_compat_syscall())
-               karg[1] >>= 32;
-
        return lp_set_timeout(minor, karg[0], karg[1]);
 }

@@ -730,11 +726,8 @@ static long lp_ioctl(struct file *file, unsigned int cmd,
        mutex_lock(&lp_mutex);
        switch (cmd) {
        case LPSETTIMEOUT_OLD:
-               if (BITS_PER_LONG == 32) {
-                       ret = lp_set_timeout32(minor, (void __user *)arg);
-                       break;
-               }
-               /* fall through - for 64-bit */
+               ret = lp_set_timeout_old(minor, (void __user *)arg);
+               break;
        case LPSETTIMEOUT_NEW:
                ret = lp_set_timeout64(minor, (void __user *)arg);
                break;
@@ -748,6 +741,16 @@ static long lp_ioctl(struct file *file, unsigned int cmd,
 }

 #ifdef CONFIG_COMPAT
+static int lp_set_timeout32(unsigned int minor, void __user *arg)
+{
+       s32 karg[2];
+
+       if (copy_from_user(karg, arg, sizeof(karg)))
+               return -EFAULT;
+
+       return lp_set_timeout(minor, karg[0], karg[1]);
+}
+
 static long lp_compat_ioctl(struct file *file, unsigned int cmd,
                        unsigned long arg)
 {

Re: [Y2038] [PATCH 7/8] ppdev: fix PPGETTIME/PPSETTIME ioctls

[Arnd Bergmann]

On Wed, Nov 20, 2019 at 8:29 PM Ben Hutchings
<ben.hutchings@codethink.co.uk> wrote:
>
> On Fri, 2019-11-08 at 21:34 +0100, Arnd Bergmann wrote:
> > Going through the uses of timeval in the user space API,
> > I noticed two bugs in ppdev that were introduced in the y2038
> > conversion:
> >
> > * The range check was accidentally moved from ppsettime to
> >   ppgettime
> >
> > * On sparc64, the microseconds are in the other half of the
> >   64-bit word.
> >
> > Fix both, and mark the fix for stable backports.
>
> Like the patch for lpdev, this also doesn't completely fix sparc64.

I think the same applies as in the other patch:

- it actually works correctly because of the alignment
- it's already merged in linux-next
- if you wish, I can add another cleanup patch on top, but I'd
  prefer to just leave it as it is.

      Arnd

Re: [Y2038] [PATCH 3/8] powerpc: fix vdso32 for ppc64le

[Ben Hutchings]

On Thu, 2019-11-21 at 11:02 +0100, Arnd Bergmann wrote:
> On Wed, Nov 20, 2019 at 10:49 PM Ben Hutchings
> <ben.hutchings@codethink.co.uk> wrote:
> > On Wed, 2019-11-20 at 20:35 +0100, Arnd Bergmann wrote:
> > > On Wed, Nov 20, 2019 at 8:13 PM Ben Hutchings
> > > <ben.hutchings@codethink.co.uk> wrote:
> > > > On Fri, 2019-11-08 at 21:34 +0100, Arnd Bergmann wrote:
> > > > > On little-endian 32-bit application running on 64-bit kernels,
> > > > > the current vdso would read the wrong half of the xtime seconds
> > > > > field. Change it to return the lower half like it does on
> > > > > big-endian.
> > > > 
> > > > ppc64le doesn't have 32-bit compat so this is only theoretical.
> > > 
> > > That is probably true. I only looked at the kernel, which today still
> > > supports compat mode for ppc64le, but I saw the patches to disable
> > > it, and I don't think anyone has even attempted building user space
> > > for it.
> > 
> > COMPAT is still enabled for some reason, but VDSO32 isn't (since 4.2).
> 
> Ok, I had missed that detail. Should I just drop my patch then?

I think so.

Ben.

-- 
Ben Hutchings, Software Developer                         Codethink Ltd
https://www.codethink.co.uk/                 Dale House, 35 Dale Street
                                     Manchester, M1 2HF, United Kingdom

Re: [Y2038] [PATCH 6/8] lp: fix sparc64 LPSETTIMEOUT ioctl

[Ben Hutchings]

On Thu, 2019-11-21 at 15:04 +0100, Arnd Bergmann wrote:
[...]
> As Greg has already merged the original patch, and that version works
> just as well, I'd probably just leave what I did at first. One benefit is
> that in case we decide to kill off sparc64 support before drivers/char/lp.c,
> the special case can be removed more easily.
> 
> I don't think either of them is going any time soon, but working on y2038
> patches has made me think ahead longer term ;-)
> 
> If you still think we should change it I can send the below patch (now
> actually build-tested) with your Ack.
[...]

I would like it, but since you convinced me the current version works
correctly it's obvious lower priority than the other changes you have.

Ben.

-- 
Ben Hutchings, Software Developer                         Codethink Ltd
https://www.codethink.co.uk/                 Dale House, 35 Dale Street
                                     Manchester, M1 2HF, United Kingdom