On Thu, 2021-03-04 at 16:11 +0100, Philippe Gerum wrote:
> Florian Bezdeka <[email protected]> writes:
> 
> > On systems using 32 bit for time_t the sem_timedwait syscall was broken
> > because the function used for copying the timeout value from userspace
> > to kernel (=sem_fetch_timeout()) was always copying
> > sizeof(struct timespec64).
> > 
> > A 32 bit application (or more specific an application with 4 byte
> > time_t) would only provide sizeof(struct old_timespec32).
> > 
> > Notable changes:
> >   - The copy operation from userspace to kernel is now already done in
> >     the syscall handler. So it is always done. Previously it was copied
> >     over and validated before the first use (when used at all).
> >     So we have some additional instructions now that may be
> >     unnecessary, but that simplifies the code.
> 
> Nack, there is a reason for this postponed copy of timespecs up to the
> point where they are actually needed. This patch would break POSIX
> compliance for some users. Please see the last sentence in the
> Description of sem_timedwait()
> https://pubs.opengroup.org/onlinepubs/009696699/functions/sem_timedwait.html

I can't find any comment regarding the point in time where we copy this
timespec to the kernel space. For the time of validation see below.
Could you point me to the failing tests or some more documentation?

> 
> I agree that the standard says "need not be checked", not "must not be
> checked" which may make this implementation-defined, but some test
> suites Xenomai runs against do expect this to pass.
> 

The validation did not change. It's done right before the first usage.
So if not needed (because sem could be taken without timeout) there is
still no validation.

Thanks for your feedback, Philippe!

Reply via email to