On Thu, 2021-03-04 at 11:08 +0100, Philippe Gerum wrote:
> Philippe Gerum <[email protected]> writes:
>
> > [email protected] <[email protected]> writes:
> >
> > > On Thu, 2021-03-04 at 10:35 +0100, Philippe Gerum wrote:
> > > > [email protected] <[email protected]> writes:
> > > >
> > > > > On Sat, 2021-02-20 at 16:18 +0100, Philippe Gerum via Xenomai wrote:
> > > > > [snip]
> > > Adding the full code:
> > >
> > > static inline int sem_fetch_timeout(struct timespec64 *ts,
> > > const void __user *u_ts)
> > > {
> > > return u_ts == NULL ? -EFAULT :
> > > cobalt_copy_from_user(ts, u_ts, sizeof(*ts));
> > > }
> > >
> > > Copying sizeof(*ts) is to much if the application provided
> > > "__old_time_t" (sizeof(time_t) =4) only. Isn't it? I would expect the
> > > result (in ts) to be garbage. Due to different padding the "sec" field
> > > (in ts) would now contain the nsec value from u_ts as well.
> > >
> >
> > Ah, I was looking at the wrong path (32emu). Yes, this is broken for
> > mere 32bit now (i.e. natively 32bit platform).
>
> We could simply use cobalt_get_u_timespec() instead of open coding the
> call to cobalt_copy_from_user(). Condvars have this right already,
> mutex, mq and sema4s have their fetch_timeout() helper broken
> indeed. Good spot.
>
I had a look at the affected mutex syscalls today.
pthread_mutex_timedlock() has the well known "The validity of the
abs_timeout parameter need not be checked if the mutex can be locked
immediately." as well.
Could someone please confirm that Xenomai is already validating the
supplied timeout even if the mutex could be taken immediately? Maybe
I'm missing something.
Should we fix that as well? Any advice how to do so?
Starting point:
__cobalt_mutex_timedlock_break()
__cobalt_mutex_timedlock_break(): Validation before calling
__cobalt_mutex_acquire_unchecked()
If I'm right, the problem was once fixed with:
860ea449a298 ("cobalt/posix/mutex: fix timeout handling in
pthread_mutex_timedlock()")
but broken again in:
c53a9dd3ebc7 ("cobalt/posix/mutex: prepare for 32bit syscall emulation")
