Re: [PATCH] hrtimer: make epoll_wait() use the hrtimer range feature

[Mike Frysinger]

On Sun, Aug 8, 2010 at 18:45, Shawn Bohrer wrote:
> @@ -1116,18 +1113,22 @@ static int ep_send_events(struct eventpoll *ep,
> Âstatic int ep_poll(struct eventpoll *ep, struct epoll_event __user *events,
> Â Â Â Â Â Â Â Â Â int maxevents, long timeout)
> Â{
> - Â Â Â int res, eavail;
> + Â Â Â int res, eavail, timed_out = 0;
> Â Â Â Âunsigned long flags;
> - Â Â Â long jtimeout;
> + Â Â Â long slack;
> Â Â Â Âwait_queue_t wait;
> -
> - Â Â Â /*
> - Â Â Â Â* Calculate the timeout by checking for the "infinite" value (-1)
> - Â Â Â Â* and the overflow condition. The passed timeout is in milliseconds,
> - Â Â Â Â* that why (t * HZ) / 1000.
> - Â Â Â Â*/
> - Â Â Â jtimeout = (timeout < 0 || timeout >= EP_MAX_MSTIMEO) ?
> - Â Â Â Â Â Â Â MAX_SCHEDULE_TIMEOUT : (timeout * HZ + 999) / 1000;
> + Â Â Â struct timespec end_time;
> + Â Â Â ktime_t expires, *to = NULL;
> +
> + Â Â Â if (timeout > 0) {
> + Â Â Â Â Â Â Â ktime_get_ts(&end_time);
> + Â Â Â Â Â Â Â timespec_add_ns(&end_time, (u64)timeout * NSEC_PER_MSEC);
> + Â Â Â Â Â Â Â slack = estimate_accuracy(&end_time);
> + Â Â Â Â Â Â Â to = &expires;
> + Â Â Â Â Â Â Â *to = timespec_to_ktime(end_time);
> + Â Â Â } else if (timeout == 0) {
> + Â Â Â Â Â Â Â timed_out = 1;
> + Â Â Â }
>
> Âretry:
> Â Â Â Âspin_lock_irqsave(&ep->lock, flags);
> @@ -1149,7 +1150,7 @@ retry:
> Â Â Â Â Â Â Â Â Â Â Â Â * to TASK_INTERRUPTIBLE before doing the checks.
> Â Â Â Â Â Â Â Â Â Â Â Â */
> Â Â Â Â Â Â Â Â Â Â Â Âset_current_state(TASK_INTERRUPTIBLE);
> - Â Â Â Â Â Â Â Â Â Â Â if (!list_empty(&ep->rdllist) || !jtimeout)
> + Â Â Â Â Â Â Â Â Â Â Â if (!list_empty(&ep->rdllist) || timed_out)
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âbreak;
> Â Â Â Â Â Â Â Â Â Â Â Âif (signal_pending(current)) {
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âres = -EINTR;
> @@ -1157,7 +1158,9 @@ retry:
> Â Â Â Â Â Â Â Â Â Â Â Â}
>
> Â Â Â Â Â Â Â Â Â Â Â Âspin_unlock_irqrestore(&ep->lock, flags);
> - Â Â Â Â Â Â Â Â Â Â Â jtimeout = schedule_timeout(jtimeout);
> + Â Â Â Â Â Â Â Â Â Â Â if (!schedule_hrtimeout_range(to, slack, HRTIMER_MODE_ABS))
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â timed_out = 1;
> +
> Â Â Â Â Â Â Â Â Â Â Â Âspin_lock_irqsave(&ep->lock, flags);
> Â Â Â Â Â Â Â Â}
> Â Â Â Â Â Â Â Â__remove_wait_queue(&ep->wq, &wait);

this code introduces a warning:
fs/eventpoll.c: In function âep_pollâ:
fs/eventpoll.c:1119: warning: âslackâ may be used uninitialized in this function

looks to me like you arent properly handling negative timeouts.
certainly epoll_wait() passes the timeout value straight from
userspace to ep_poll() without any error checking, so if userspace
passes a negative timeout value, it looks like "slack" will be used
uninitialized.
-mike
¢éì®&Þ~º&¶¬–+-±éÝ¥Šw®žË±Êâmébžìdz¹Þ)í…æèw*jg¬±¨¶‰šŽŠÝj/êäz¹ÞŠà2ŠÞ¨è­Ú&¢)ß«a¶Úþø®G«éh®æj:+v‰¨Šwè†Ù>Wš±êÞiÛaxPjØm¶ŸÿÃ-»+ƒùdš_