Message ID | 1533331532-25426-2-git-send-email-tyhicks@canonical.com |
---|---|
State | New |
Headers | show |
Series | posix-timer: Properly check sigevent->sigev_notify | expand |
On 08/03/18 23:25, Tyler Hicks wrote: > From: Thomas Gleixner <tglx@linutronix.de> > > timer_create() specifies via sigevent->sigev_notify the signal delivery for > the new timer. The valid modes are SIGEV_NONE, SIGEV_SIGNAL, SIGEV_THREAD > and (SIGEV_SIGNAL | SIGEV_THREAD_ID). > > The sanity check in good_sigevent() is only checking the valid combination > for the SIGEV_THREAD_ID bit, i.e. SIGEV_SIGNAL, but if SIGEV_THREAD_ID is > not set it accepts any random value. > > This has no real effects on the posix timer and signal delivery code, but > it affects show_timer() which handles the output of /proc/$PID/timers. That > function uses a string array to pretty print sigev_notify. The access to > that array has no bound checks, so random sigev_notify cause access beyond > the array bounds. > > Add proper checks for the valid notify modes and remove the SIGEV_THREAD_ID > masking from various code pathes as SIGEV_NONE can never be set in > combination with SIGEV_THREAD_ID. > > Reported-by: Eric Biggers <ebiggers3@gmail.com> > Reported-by: Dmitry Vyukov <dvyukov@google.com> > Reported-by: Alexey Dobriyan <adobriyan@gmail.com> > Signed-off-by: Thomas Gleixner <tglx@linutronix.de> > Cc: John Stultz <john.stultz@linaro.org> > Cc: stable@vger.kernel.org > > CVE-2017-18344 > > (backported from commit cef31d9af908243421258f1df35a4a644604efbe) > [tyhicks: Do not worry about removing the SIGEV_THREAD_ID masking since it is > irrelevant to the security fix] > Signed-off-by: Tyler Hicks <tyhicks@canonical.com> Acked-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com> > --- > kernel/posix-timers.c | 25 +++++++++++++++---------- > 1 file changed, 15 insertions(+), 10 deletions(-) > > diff --git a/kernel/posix-timers.c b/kernel/posix-timers.c > index 77e6b83c0431..26b910462a0c 100644 > --- a/kernel/posix-timers.c > +++ b/kernel/posix-timers.c > @@ -498,17 +498,22 @@ static struct pid *good_sigevent(sigevent_t * event) > { > struct task_struct *rtn = current->group_leader; > > - if ((event->sigev_notify & SIGEV_THREAD_ID ) && > - (!(rtn = find_task_by_vpid(event->sigev_notify_thread_id)) || > - !same_thread_group(rtn, current) || > - (event->sigev_notify & ~SIGEV_THREAD_ID) != SIGEV_SIGNAL)) > + switch (event->sigev_notify) { > + case SIGEV_SIGNAL | SIGEV_THREAD_ID: > + rtn = find_task_by_vpid(event->sigev_notify_thread_id); > + if (!rtn || !same_thread_group(rtn, current)) > + return NULL; > + /* FALLTHRU */ > + case SIGEV_SIGNAL: > + case SIGEV_THREAD: > + if (event->sigev_signo <= 0 || event->sigev_signo > SIGRTMAX) > + return NULL; > + /* FALLTHRU */ > + case SIGEV_NONE: > + return task_pid(rtn); > + default: > return NULL; > - > - if (((event->sigev_notify & ~SIGEV_THREAD_ID) != SIGEV_NONE) && > - ((event->sigev_signo <= 0) || (event->sigev_signo > SIGRTMAX))) > - return NULL; > - > - return task_pid(rtn); > + } > } > > void posix_timers_register_clock(const clockid_t clock_id, >
diff --git a/kernel/posix-timers.c b/kernel/posix-timers.c index 77e6b83c0431..26b910462a0c 100644 --- a/kernel/posix-timers.c +++ b/kernel/posix-timers.c @@ -498,17 +498,22 @@ static struct pid *good_sigevent(sigevent_t * event) { struct task_struct *rtn = current->group_leader; - if ((event->sigev_notify & SIGEV_THREAD_ID ) && - (!(rtn = find_task_by_vpid(event->sigev_notify_thread_id)) || - !same_thread_group(rtn, current) || - (event->sigev_notify & ~SIGEV_THREAD_ID) != SIGEV_SIGNAL)) + switch (event->sigev_notify) { + case SIGEV_SIGNAL | SIGEV_THREAD_ID: + rtn = find_task_by_vpid(event->sigev_notify_thread_id); + if (!rtn || !same_thread_group(rtn, current)) + return NULL; + /* FALLTHRU */ + case SIGEV_SIGNAL: + case SIGEV_THREAD: + if (event->sigev_signo <= 0 || event->sigev_signo > SIGRTMAX) + return NULL; + /* FALLTHRU */ + case SIGEV_NONE: + return task_pid(rtn); + default: return NULL; - - if (((event->sigev_notify & ~SIGEV_THREAD_ID) != SIGEV_NONE) && - ((event->sigev_signo <= 0) || (event->sigev_signo > SIGRTMAX))) - return NULL; - - return task_pid(rtn); + } } void posix_timers_register_clock(const clockid_t clock_id,