This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH 13/19] y2038: add compat handling for sys_semtimedop
- From: Thomas Gleixner <tglx at linutronix dot de>
- To: Arnd Bergmann <arnd at arndb dot de>
- Cc: y2038 at lists dot linaro dot org, baolin dot wang at linaro dot org, albert dot aribaud at 3adev dot fr, john dot stultz at linaro dot org, bamvor dot zhangjian at linaro dot org, ruchandani dot tina at gmail dot com, linux-api at vger dot kernel dot org, linux-kernel at vger dot kernel dot org, libc-alpha at sourceware dot org
- Date: Tue, 19 May 2015 11:19:29 +0200 (CEST)
- Subject: Re: [PATCH 13/19] y2038: add compat handling for sys_semtimedop
- Authentication-results: sourceware.org; auth=none
- References: <1430929826-318934-1-git-send-email-arnd at arndb dot de> <1430929826-318934-14-git-send-email-arnd at arndb dot de> <alpine dot DEB dot 2 dot 11 dot 1505160041370 dot 4225 at nanos> <5686498 dot JnFHvQTH21 at wuerfel>
On Sat, 16 May 2015, Arnd Bergmann wrote:
> On Saturday 16 May 2015 00:46:44 Thomas Gleixner wrote:
> > On Wed, 6 May 2015, Arnd Bergmann wrote:
> > > +SYSCALL_DEFINE4(semtimedop, int, semid, struct sembuf __user *, tsops,
> > > + unsigned, nsops,
> > > + const struct __kernel_timespec __user *, timeout)
> > > +{
> > > + unsigned long jiffies_left = 0;
> > > +
> > > + if (timeout) {
> > > + struct timespec64 _timeout;
> > > + if (get_timespec64(&_timeout, timeout))
> >
> > Moo. I had to look 3 times to get not confused by the extra
> > underscore. What's wrong with a proper variable name which is easy to
> > distinguish?
> >
> > > + return -EFAULT;
> >
> > > + if (_timeout.tv_sec < 0 || _timeout.tv_nsec < 0 ||
> > > + _timeout.tv_nsec >= 1000000000L)
> > > + return -EINVAL;
> >
> > We have proper helper functions to validate time specs.
>
> I ended up fixing both issues you noticed in the same patch
> after all, and also simplified it slightly more.
>
> Finally, I also noticed that I had not done a timespec64_to_jiffies()
> call at the time when I wrote this patch, but it actually exists now,
> so I've reordered my series and am using it in the new version, as
> I should have done to start with.
Indeed. I didn't notice either.
> >From e04b14d49273c27d92f1799233b82bcdafb43d9a Mon Sep 17 00:00:00 2001
> From: Arnd Bergmann <arnd@arndb.de>
> Date: Mon, 27 Apr 2015 23:30:39 +0200
> Subject: [UPDATED PATCH] y2038: add compat handling for sys_semtimedop
>
> This moves the compat_sys_semtimedop function to ipc/sem.c so it
> can be shared with 32-bit architectures efficiently. Instead of
> copying the timespec back to user space, we take a shortcut and
> pass the kernel timespec64 value to the low-level implementation
> directly.
>
> The native sys_semtimedop() function is modified to take a
> __kernel_timespec structure, which will be based on a 64-bit
> time_t in the future.
>
> There is a small API change here: if multiple errors are present,
> and the timespec argument is an invalid pointer, we now return
> -EFAULT before checking any of the other error conditions.
> This is what the compat version has always done, but if it is a
> problem, we need a more sophisticated approach.
The important part of error checks is that they catch all
cases and combinations. In which order is completely irrelevant.
If something relies on the ordering of error check returns, it's
broken by definition.
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Reviewed-by: Thomas Gleixner <tglx@linutronix.de>