On Wed, Oct 14, 2020 at 08:06:52PM -0500, Scott Cheloha wrote:
> _exit(2) and execve(2) need to obey the locking protocol described in
> proc.h when manipulating the per-process interval timer state.
>
> While we're here we can also remove the now pointless splclock/splx
> dance from execve(2).
>
> The easiest way to obey the locking protocol is to reuse the interface
> the syscalls are using: setitimer() in kern_time.c.
>
> Given that we only want to cancel the timers I wrote a small helper
> function, cancelitimer(). I think it's tidier than putting the
> prototype for setitimer() into sys/time.h and requiring the caller to
> prepare an itimerval struct before calling.
>
> Compare:
>
> struct itimerval itv;
> timerclear(&itv.it_value);
> timerclear(&itv.it_interval);
> setitimer(ITIMER_REAL, &itv, NULL);
>
> with:
>
> cancelitimer(ITIMER_REAL);
>
> ... should I shove the for-loop into the helper function too? Maybe
> call it "cancel_all_itimers()"? I have a vague feeling that showing
> the reader that there are multiple timers is a good thing here, but
> then again maybe I'm wrong and nobody cares.
>
> Preferences? ok?
Whoops, forgot the kern_time.c part of the diff.
Index: kern/kern_exit.c
===================================================================
RCS file: /cvs/src/sys/kern/kern_exit.c,v
retrieving revision 1.188
diff -u -p -r1.188 kern_exit.c
--- kern/kern_exit.c 18 Mar 2020 15:48:21 -0000 1.188
+++ kern/kern_exit.c 15 Oct 2020 01:12:50 -0000
@@ -194,7 +194,11 @@ exit1(struct proc *p, int xexit, int xsi
/* close open files and release open-file table */
fdfree(p);
- timeout_del(&pr->ps_realit_to);
+ /* cancel all interval timers */
+ int i;
+ for (i = 0; i < nitems(pr->ps_timer); i++)
+ cancelitimer(i);
+
timeout_del(&pr->ps_rucheck_to);
#ifdef SYSVSEM
semexit(pr);
Index: kern/kern_exec.c
===================================================================
RCS file: /cvs/src/sys/kern/kern_exec.c,v
retrieving revision 1.217
diff -u -p -r1.217 kern_exec.c
--- kern/kern_exec.c 11 Jul 2020 22:59:05 -0000 1.217
+++ kern/kern_exec.c 15 Oct 2020 01:12:50 -0000
@@ -656,14 +656,9 @@ sys_execve(struct proc *p, void *v, regi
}
if (pr->ps_flags & PS_SUGIDEXEC) {
- int i, s = splclock();
-
- timeout_del(&pr->ps_realit_to);
- for (i = 0; i < nitems(pr->ps_timer); i++) {
- timespecclear(&pr->ps_timer[i].it_interval);
- timespecclear(&pr->ps_timer[i].it_value);
- }
- splx(s);
+ int i;
+ for (i = 0; i < nitems(pr->ps_timer); i++)
+ cancelitimer(i);
}
/* reset CPU time usage for the thread, but not the process */
Index: kern/kern_time.c
===================================================================
RCS file: /cvs/src/sys/kern/kern_time.c,v
retrieving revision 1.146
diff -u -p -r1.146 kern_time.c
--- kern/kern_time.c 13 Oct 2020 17:33:39 -0000 1.146
+++ kern/kern_time.c 15 Oct 2020 01:12:50 -0000
@@ -572,6 +572,16 @@ setitimer(int which, const struct itimer
}
}
+void
+cancelitimer(int which)
+{
+ struct itimerval itv;
+
+ timerclear(&itv.it_value);
+ timerclear(&itv.it_interval);
+ setitimer(which, &itv, NULL);
+}
+
int
sys_getitimer(struct proc *p, void *v, register_t *retval)
{
Index: sys/time.h
===================================================================
RCS file: /cvs/src/sys/sys/time.h,v
retrieving revision 1.55
diff -u -p -r1.55 time.h
--- sys/time.h 6 Jul 2020 13:33:09 -0000 1.55
+++ sys/time.h 15 Oct 2020 01:12:50 -0000
@@ -307,6 +307,7 @@ time_t getuptime(void);
struct proc;
int clock_gettime(struct proc *, clockid_t, struct timespec *);
+void cancelitimer(int);
int itimerfix(struct timeval *);
int itimerdecr(struct itimerspec *, long);
int settime(const struct timespec *);
