From: Arnd Bergmann <arnd@arndb.de>
To: Ben Hutchings <ben.hutchings@codethink.co.uk>
Cc: y2038 Mailman List <y2038@lists.linaro.org>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
"Gustavo A. R. Silva" <gustavo@embeddedor.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"# 3.4.x" <stable@vger.kernel.org>,
Bamvor Jian Zhang <bamv2005@gmail.com>,
Thomas Gleixner <tglx@linutronix.de>,
Sudip Mukherjee <sudipm.mukherjee@gmail.com>
Subject: Re: [Y2038] [PATCH 6/8] lp: fix sparc64 LPSETTIMEOUT ioctl
Date: Thu, 21 Nov 2019 15:04:00 +0100 [thread overview]
Message-ID: <CAK8P3a34sty4kTfFSKz8e-D+B14e3oTUPaACzGq_1SjYeuoytg@mail.gmail.com> (raw)
In-Reply-To: <a187cb75cc15ba8ee4a7b652fae8317cb9b03020.camel@codethink.co.uk>
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)
{
next prev parent reply other threads:[~2019-11-21 14:04 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-11-08 20:34 [PATCH 0/8] y2038: bug fixes from y2038 work Arnd Bergmann
2019-11-08 20:34 ` [PATCH 1/8] y2038: timex: remove incorrect time_t truncation Arnd Bergmann
2019-11-10 20:44 ` Deepa Dinamani
2019-11-12 7:16 ` [tip: timers/urgent] ntp/y2038: Remove " tip-bot2 for Arnd Bergmann
2019-11-08 20:34 ` [PATCH 2/8] timekeeping: optimize ns_to_timespec64 Arnd Bergmann
2019-11-10 20:46 ` Deepa Dinamani
2019-11-12 7:22 ` [tip: timers/core] time: Optimize ns_to_timespec64() tip-bot2 for Arnd Bergmann
2019-11-08 20:34 ` [PATCH 3/8] powerpc: fix vdso32 for ppc64le Arnd Bergmann
2019-11-20 19:13 ` [Y2038] " Ben Hutchings
2019-11-20 19:35 ` Arnd Bergmann
2019-11-20 21:49 ` Ben Hutchings
2019-11-21 10:02 ` Arnd Bergmann
2019-11-21 15:56 ` Ben Hutchings
2019-11-08 20:34 ` [PATCH 4/8] ipmi: kill off 'timespec' usage again Arnd Bergmann
2019-11-08 22:11 ` Corey Minyard
2019-11-09 11:23 ` Arnd Bergmann
2019-11-08 20:34 ` [PATCH 5/8] netfilter: xt_time: use time64_t Arnd Bergmann
2019-11-15 22:43 ` Pablo Neira Ayuso
2019-11-08 20:34 ` [PATCH 6/8] lp: fix sparc64 LPSETTIMEOUT ioctl Arnd Bergmann
2019-11-20 19:27 ` [Y2038] " Ben Hutchings
2019-11-20 19:46 ` Arnd Bergmann
2019-11-20 22:10 ` Ben Hutchings
2019-11-21 14:04 ` Arnd Bergmann [this message]
2019-11-21 16:00 ` Ben Hutchings
2019-11-08 20:34 ` [PATCH 7/8] ppdev: fix PPGETTIME/PPSETTIME ioctls Arnd Bergmann
2019-11-20 19:29 ` [Y2038] " Ben Hutchings
2019-11-21 14:06 ` Arnd Bergmann
2019-11-08 20:34 ` [PATCH 8/8] Input: input_event: fix struct padding on sparc64 Arnd Bergmann
2019-11-11 18:28 ` Dmitry Torokhov
2019-11-11 19:18 ` Arnd Bergmann
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAK8P3a34sty4kTfFSKz8e-D+B14e3oTUPaACzGq_1SjYeuoytg@mail.gmail.com \
--to=arnd@arndb.de \
--cc=bamv2005@gmail.com \
--cc=ben.hutchings@codethink.co.uk \
--cc=gregkh@linuxfoundation.org \
--cc=gustavo@embeddedor.com \
--cc=linux-kernel@vger.kernel.org \
--cc=stable@vger.kernel.org \
--cc=sudipm.mukherjee@gmail.com \
--cc=tglx@linutronix.de \
--cc=y2038@lists.linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).