| Message ID | 20191108203435.112759-8-arnd@arndb.de |
|---|---|
| State | Accepted |
| Commit | 998174042da229e2cf5841f574aba4a743e69650 |
| Headers | show |
| Series | [1/8] y2038: timex: remove incorrect time_t truncation | expand |
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
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
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;
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(-) -- 2.20.0