Re: setitimer(2): increase interval upper bound to UINT_MAX seconds
On Fri, Jun 18, 2021 at 09:29:44AM +0200, Claudio Jeker wrote:
> On Thu, Jun 17, 2021 at 08:41:39PM -0500, Scott Cheloha wrote:
> > On Fri, Jun 11, 2021 at 12:17:02PM -0500, Scott Cheloha wrote:
> > > Hi,
> > >
> > > setitimer(2) has a one hundred million second upper bound for timers.
> > > Any timer interval larger than this is considered invalid and we set
> > > EINVAL.
> > >
> > > There is no longer any reason to use this particular limit. Kclock
> > > timeouts support the full range of a timespec, so we can trivially
> > > increase the upper bound without any practical risk of overflow.
> > >
> > > This patch increases the upper bound to UINT_MAX seconds.
> > >
> > > Why UINT_MAX? UINT_MAX is the largest possible input to alarm(3). We
> > > could then simplify the alarm(3) manpage and the libc alarm.c code in
> > > a subsequent patch. POSIX says alarm(3) "is always successful". Our
> > > implementation can fail. It would be nicer/simpler if ours were free
> > > of failure modes.
> > >
> > > ok?
> >
> > 1 week bump.
> >
> > Updated patch: make the maximum value ("max") static and const.
>
> OK claudio@
>
> I wonder if we need a max at all? I guess there is an upper limit to not
> overflow the time_t when calculating the absolute timeout but that is
> probably close to LLONG_MAX / 2.
Exactly. LLONG_MAX / 2 would be a reasonable maximum value. It's
stupid large, simple, and overflow is theoretically impossible.
However I'd rather stick with UINT_MAX to start because it addresses a
concrete problem: supporting the full input range of alarm(3).
If in the future we come across software in the wild that misbehaves
due to this upper bound we can bump it at that time to address that
particular issue. Until then, UINT_MAX is plenty.
Re: setitimer(2): increase interval upper bound to UINT_MAX seconds
> Date: Fri, 18 Jun 2021 09:29:44 +0200
> From: Claudio Jeker
>
> On Thu, Jun 17, 2021 at 08:41:39PM -0500, Scott Cheloha wrote:
> > On Fri, Jun 11, 2021 at 12:17:02PM -0500, Scott Cheloha wrote:
> > > Hi,
> > >
> > > setitimer(2) has a one hundred million second upper bound for timers.
> > > Any timer interval larger than this is considered invalid and we set
> > > EINVAL.
> > >
> > > There is no longer any reason to use this particular limit. Kclock
> > > timeouts support the full range of a timespec, so we can trivially
> > > increase the upper bound without any practical risk of overflow.
> > >
> > > This patch increases the upper bound to UINT_MAX seconds.
> > >
> > > Why UINT_MAX? UINT_MAX is the largest possible input to alarm(3). We
> > > could then simplify the alarm(3) manpage and the libc alarm.c code in
> > > a subsequent patch. POSIX says alarm(3) "is always successful". Our
> > > implementation can fail. It would be nicer/simpler if ours were free
> > > of failure modes.
> > >
> > > ok?
> >
> > 1 week bump.
> >
> > Updated patch: make the maximum value ("max") static and const.
>
> OK claudio@
>
> I wonder if we need a max at all? I guess there is an upper limit to not
> overflow the time_t when calculating the absolute timeout but that is
> probably close to LLONG_MAX / 2.
Not really worth worrying about. Youu'll be long dead once that alarm
expires ;).
> I think a simplified version of alarm(3) that never fails would be nice.
>
> > Index: kern_time.c
> > ===
> > RCS file: /cvs/src/sys/kern/kern_time.c,v
> > retrieving revision 1.153
> > diff -u -p -r1.153 kern_time.c
> > --- kern_time.c 11 Jun 2021 16:36:34 - 1.153
> > +++ kern_time.c 18 Jun 2021 01:40:42 -
> > @@ -709,15 +709,16 @@ out:
> > int
> > itimerfix(struct itimerval *itv)
> > {
> > + static const struct timeval max = { .tv_sec = UINT_MAX, .tv_usec = 0 };
> > struct timeval min_interval = { .tv_sec = 0, .tv_usec = tick };
> >
> > if (itv->it_value.tv_sec < 0 || !timerisvalid(&itv->it_value))
> > return EINVAL;
> > - if (itv->it_value.tv_sec > 1)
> > + if (timercmp(&itv->it_value, &max, >))
> > return EINVAL;
> > if (itv->it_interval.tv_sec < 0 || !timerisvalid(&itv->it_interval))
> > return EINVAL;
> > - if (itv->it_interval.tv_sec > 1)
> > + if (timercmp(&itv->it_interval, &max, >))
> > return EINVAL;
> >
> > if (!timerisset(&itv->it_value))
> >
>
> --
> :wq Claudio
>
>
Re: setitimer(2): increase interval upper bound to UINT_MAX seconds
On Thu, Jun 17, 2021 at 08:41:39PM -0500, Scott Cheloha wrote:
> On Fri, Jun 11, 2021 at 12:17:02PM -0500, Scott Cheloha wrote:
> > Hi,
> >
> > setitimer(2) has a one hundred million second upper bound for timers.
> > Any timer interval larger than this is considered invalid and we set
> > EINVAL.
> >
> > There is no longer any reason to use this particular limit. Kclock
> > timeouts support the full range of a timespec, so we can trivially
> > increase the upper bound without any practical risk of overflow.
> >
> > This patch increases the upper bound to UINT_MAX seconds.
> >
> > Why UINT_MAX? UINT_MAX is the largest possible input to alarm(3). We
> > could then simplify the alarm(3) manpage and the libc alarm.c code in
> > a subsequent patch. POSIX says alarm(3) "is always successful". Our
> > implementation can fail. It would be nicer/simpler if ours were free
> > of failure modes.
> >
> > ok?
>
> 1 week bump.
>
> Updated patch: make the maximum value ("max") static and const.
OK claudio@
I wonder if we need a max at all? I guess there is an upper limit to not
overflow the time_t when calculating the absolute timeout but that is
probably close to LLONG_MAX / 2.
I think a simplified version of alarm(3) that never fails would be nice.
> Index: kern_time.c
> ===
> RCS file: /cvs/src/sys/kern/kern_time.c,v
> retrieving revision 1.153
> diff -u -p -r1.153 kern_time.c
> --- kern_time.c 11 Jun 2021 16:36:34 - 1.153
> +++ kern_time.c 18 Jun 2021 01:40:42 -
> @@ -709,15 +709,16 @@ out:
> int
> itimerfix(struct itimerval *itv)
> {
> + static const struct timeval max = { .tv_sec = UINT_MAX, .tv_usec = 0 };
> struct timeval min_interval = { .tv_sec = 0, .tv_usec = tick };
>
> if (itv->it_value.tv_sec < 0 || !timerisvalid(&itv->it_value))
> return EINVAL;
> - if (itv->it_value.tv_sec > 1)
> + if (timercmp(&itv->it_value, &max, >))
> return EINVAL;
> if (itv->it_interval.tv_sec < 0 || !timerisvalid(&itv->it_interval))
> return EINVAL;
> - if (itv->it_interval.tv_sec > 1)
> + if (timercmp(&itv->it_interval, &max, >))
> return EINVAL;
>
> if (!timerisset(&itv->it_value))
>
--
:wq Claudio
Re: setitimer(2): increase interval upper bound to UINT_MAX seconds
On Fri, Jun 11, 2021 at 12:17:02PM -0500, Scott Cheloha wrote:
> Hi,
>
> setitimer(2) has a one hundred million second upper bound for timers.
> Any timer interval larger than this is considered invalid and we set
> EINVAL.
>
> There is no longer any reason to use this particular limit. Kclock
> timeouts support the full range of a timespec, so we can trivially
> increase the upper bound without any practical risk of overflow.
>
> This patch increases the upper bound to UINT_MAX seconds.
>
> Why UINT_MAX? UINT_MAX is the largest possible input to alarm(3). We
> could then simplify the alarm(3) manpage and the libc alarm.c code in
> a subsequent patch. POSIX says alarm(3) "is always successful". Our
> implementation can fail. It would be nicer/simpler if ours were free
> of failure modes.
>
> ok?
1 week bump.
Updated patch: make the maximum value ("max") static and const.
Index: kern_time.c
===
RCS file: /cvs/src/sys/kern/kern_time.c,v
retrieving revision 1.153
diff -u -p -r1.153 kern_time.c
--- kern_time.c 11 Jun 2021 16:36:34 - 1.153
+++ kern_time.c 18 Jun 2021 01:40:42 -
@@ -709,15 +709,16 @@ out:
int
itimerfix(struct itimerval *itv)
{
+ static const struct timeval max = { .tv_sec = UINT_MAX, .tv_usec = 0 };
struct timeval min_interval = { .tv_sec = 0, .tv_usec = tick };
if (itv->it_value.tv_sec < 0 || !timerisvalid(&itv->it_value))
return EINVAL;
- if (itv->it_value.tv_sec > 1)
+ if (timercmp(&itv->it_value, &max, >))
return EINVAL;
if (itv->it_interval.tv_sec < 0 || !timerisvalid(&itv->it_interval))
return EINVAL;
- if (itv->it_interval.tv_sec > 1)
+ if (timercmp(&itv->it_interval, &max, >))
return EINVAL;
if (!timerisset(&itv->it_value))
setitimer(2): increase interval upper bound to UINT_MAX seconds
Hi,
setitimer(2) has a one hundred million second upper bound for timers.
Any timer interval larger than this is considered invalid and we set
EINVAL.
There is no longer any reason to use this particular limit. Kclock
timeouts support the full range of a timespec, so we can trivially
increase the upper bound without any practical risk of overflow.
This patch increases the upper bound to UINT_MAX seconds.
Why UINT_MAX? UINT_MAX is the largest possible input to alarm(3). We
could then simplify the alarm(3) manpage and the libc alarm.c code in
a subsequent patch. POSIX says alarm(3) "is always successful". Our
implementation can fail. It would be nicer/simpler if ours were free
of failure modes.
ok?
Index: kern_time.c
===
RCS file: /cvs/src/sys/kern/kern_time.c,v
retrieving revision 1.153
diff -u -p -r1.153 kern_time.c
--- kern_time.c 11 Jun 2021 16:36:34 - 1.153
+++ kern_time.c 11 Jun 2021 17:13:49 -
@@ -709,15 +709,16 @@ out:
int
itimerfix(struct itimerval *itv)
{
+ struct timeval max_interval = { .tv_sec = UINT_MAX, .tv_usec = 0 };
struct timeval min_interval = { .tv_sec = 0, .tv_usec = tick };
if (itv->it_value.tv_sec < 0 || !timerisvalid(&itv->it_value))
return EINVAL;
- if (itv->it_value.tv_sec > 1)
+ if (timercmp(&itv->it_value, &max_interval, >))
return EINVAL;
if (itv->it_interval.tv_sec < 0 || !timerisvalid(&itv->it_interval))
return EINVAL;
- if (itv->it_interval.tv_sec > 1)
+ if (timercmp(&itv->it_interval, &max_interval, >))
return EINVAL;
if (!timerisset(&itv->it_value))
