Skip to content
Permalink
Browse files

Fix unsafe use of ptsignal() in mi_switch().

ptsignal() has to be called with the kernel lock held. As ensuring the
locking in mi_switch() is not easy, and deferring the signaling using
the task API is not possible because of lock order issues in
mi_switch(), move the CPU time checking into a periodic timer where
the kernel can be locked without issues.

With this change, each process has a dedicated resource check timer.
The timer gets activated only when a CPU time limit is set. Because the
checking is not done as frequently as before, some precision is lost.

Use of timers adapted from FreeBSD.

OK tedu@

Reported-by: syzbot+2f5d62256e3280634623@syzkaller.appspotmail.com
  • Loading branch information...
visa
visa committed Jan 6, 2019
1 parent 4d255d6 commit ec412da11be49c25266553c64da7e06a018ba909
Showing with 51 additions and 24 deletions.
  1. +2 −1 sys/kern/kern_exit.c
  2. +4 −1 sys/kern/kern_fork.c
  3. +39 −1 sys/kern/kern_resource.c
  4. +1 −19 sys/kern/sched_bsd.c
  5. +2 −1 sys/sys/proc.h
  6. +3 −1 sys/sys/resourcevar.h
@@ -1,4 +1,4 @@
/* $OpenBSD: kern_exit.c,v 1.171 2018/11/12 15:09:17 visa Exp $ */
/* $OpenBSD: kern_exit.c,v 1.172 2019/01/06 12:59:45 visa Exp $ */
/* $NetBSD: kern_exit.c,v 1.39 1996/04/22 01:38:25 christos Exp $ */

/*
@@ -192,6 +192,7 @@ exit1(struct proc *p, int rv, int flags)
fdfree(p);

timeout_del(&pr->ps_realit_to);
timeout_del(&pr->ps_rucheck_to);
#ifdef SYSVSEM
semexit(pr);
#endif
@@ -1,4 +1,4 @@
/* $OpenBSD: kern_fork.c,v 1.208 2018/11/12 15:09:17 visa Exp $ */
/* $OpenBSD: kern_fork.c,v 1.209 2019/01/06 12:59:45 visa Exp $ */
/* $NetBSD: kern_fork.c,v 1.29 1996/02/09 18:59:34 christos Exp $ */

/*
@@ -211,6 +211,7 @@ process_initialize(struct process *pr, struct proc *p)
LIST_INIT(&pr->ps_sigiolst);

timeout_set(&pr->ps_realit_to, realitexpire, pr);
timeout_set(&pr->ps_rucheck_to, rucheck, pr);
}


@@ -240,6 +241,8 @@ process_new(struct proc *p, struct process *parent, int flags)
/* post-copy fixups */
pr->ps_pptr = parent;
pr->ps_limit->p_refcnt++;
if (pr->ps_limit->pl_rlimit[RLIMIT_CPU].rlim_cur != RLIM_INFINITY)
timeout_add_msec(&pr->ps_rucheck_to, RUCHECK_INTERVAL);

/* bump references to the text vnode (for sysctl) */
pr->ps_textvp = parent->ps_textvp;
@@ -1,4 +1,4 @@
/* $OpenBSD: kern_resource.c,v 1.58 2018/02/19 08:59:52 mpi Exp $ */
/* $OpenBSD: kern_resource.c,v 1.59 2019/01/06 12:59:45 visa Exp $ */
/* $NetBSD: kern_resource.c,v 1.38 1996/10/23 07:19:38 matthias Exp $ */

/*-
@@ -46,6 +46,7 @@
#include <sys/proc.h>
#include <sys/ktrace.h>
#include <sys/sched.h>
#include <sys/signalvar.h>

#include <sys/mount.h>
#include <sys/syscallargs.h>
@@ -267,6 +268,10 @@ dosetrlimit(struct proc *p, u_int which, struct rlimit *limp)
if (limp->rlim_cur > limp->rlim_max)
limp->rlim_cur = limp->rlim_max;

if (which == RLIMIT_CPU && limp->rlim_cur != RLIM_INFINITY &&
alimp->rlim_cur == RLIM_INFINITY)
timeout_add_msec(&p->p_p->ps_rucheck_to, RUCHECK_INTERVAL);

if (which == RLIMIT_STACK) {
/*
* Stack is allocated to the max at exec time with only
@@ -492,6 +497,39 @@ ruadd(struct rusage *ru, struct rusage *ru2)
*ip++ += *ip2++;
}

/*
* Check if the process exceeds its cpu resource allocation.
* If over max, kill it.
*/
void
rucheck(void *arg)
{
struct process *pr = arg;
struct rlimit *rlim;
rlim_t runtime;
int s;

KERNEL_ASSERT_LOCKED();

SCHED_LOCK(s);
runtime = pr->ps_tu.tu_runtime.tv_sec;
SCHED_UNLOCK(s);

rlim = &pr->ps_limit->pl_rlimit[RLIMIT_CPU];
if (runtime >= rlim->rlim_cur) {
if (runtime >= rlim->rlim_max) {
prsignal(pr, SIGKILL);
} else {
prsignal(pr, SIGXCPU);
if (rlim->rlim_cur < rlim->rlim_max)
rlim->rlim_cur = MIN(rlim->rlim_cur + 5,
rlim->rlim_max);
}
}

timeout_add_msec(&pr->ps_rucheck_to, RUCHECK_INTERVAL);
}

struct pool plimit_pool;

/*
@@ -1,4 +1,4 @@
/* $OpenBSD: sched_bsd.c,v 1.47 2017/12/04 09:38:20 mpi Exp $ */
/* $OpenBSD: sched_bsd.c,v 1.48 2019/01/06 12:59:45 visa Exp $ */
/* $NetBSD: kern_synch.c,v 1.37 1996/04/22 01:38:37 christos Exp $ */

/*-
@@ -336,8 +336,6 @@ mi_switch(void)
struct proc *p = curproc;
struct proc *nextproc;
struct process *pr = p->p_p;
struct rlimit *rlim;
rlim_t secs;
struct timespec ts;
#ifdef MULTIPROCESSOR
int hold_count;
@@ -381,22 +379,6 @@ mi_switch(void)
/* add the time counts for this thread to the process's total */
tuagg_unlocked(pr, p);

/*
* Check if the process exceeds its cpu resource allocation.
* If over max, kill it.
*/
rlim = &pr->ps_limit->pl_rlimit[RLIMIT_CPU];
secs = pr->ps_tu.tu_runtime.tv_sec;
if (secs >= rlim->rlim_cur) {
if (secs >= rlim->rlim_max) {
psignal(p, SIGKILL);
} else {
psignal(p, SIGXCPU);
if (rlim->rlim_cur < rlim->rlim_max)
rlim->rlim_cur += 5;
}
}

/*
* Process is about to yield the CPU; clear the appropriate
* scheduling flags.
@@ -1,4 +1,4 @@
/* $OpenBSD: proc.h,v 1.262 2019/01/03 21:52:31 beck Exp $ */
/* $OpenBSD: proc.h,v 1.263 2019/01/06 12:59:45 visa Exp $ */
/* $NetBSD: proc.h,v 1.44 1996/04/22 01:23:21 christos Exp $ */

/*-
@@ -202,6 +202,7 @@ struct process {
struct tusage ps_tu; /* accumulated times. */
struct rusage ps_cru; /* sum of stats for reaped children */
struct itimerval ps_timer[3]; /* timers, indexed by ITIMER_* */
struct timeout ps_rucheck_to; /* resource limit check timer */

u_int64_t ps_wxcounter;

@@ -1,4 +1,4 @@
/* $OpenBSD: resourcevar.h,v 1.18 2014/01/24 04:26:51 guenther Exp $ */
/* $OpenBSD: resourcevar.h,v 1.19 2019/01/06 12:59:45 visa Exp $ */
/* $NetBSD: resourcevar.h,v 1.12 1995/11/22 23:01:53 cgd Exp $ */

/*
@@ -69,5 +69,7 @@ struct plimit *limcopy(struct plimit *);
void limfree(struct plimit *);

void ruadd(struct rusage *, struct rusage *);
void rucheck(void *);
#define RUCHECK_INTERVAL 1000 /* check interval in msec */
#endif
#endif /* !_SYS_RESOURCEVAR_H_ */

0 comments on commit ec412da

Please sign in to comment.
You can’t perform that action at this time.