On Fri, May 22, 2015 at 15:52:25 +0800, Wang Yufei wrote: > From: Zhang Bo <oscar.zhangbo@xxxxxxxxxx> > > When we change system clock to years ago, a certain CPU may use up 100% cputime. > The reason is that in function virEventPollCalculateTimeout(), we assign the > unsigned long long result to an INT variable, > *timeout = then - now; // timeout is INT, and then/now are long long > if (*timeout < 0) > *timeout = 0; > there's a chance that variable @then minus variable @now may be a very large number > that overflows INT value expression, then *timeout will be negative and be assigned to 0. > Next the 'poll' in function virEventPollRunOnce() will get into an 'endless' while loop there. > thus, the cpu that virEventPollRunOnce() thread runs on will go up to 100%. > > Although as we discussed before in https://www.redhat.com/archives/libvir-list/2015-May/msg00400.html > it should be prohibited to set-time while other applications are running, but it does > seems to have no harm to make the codes more robust. > > Signed-off-by: Wang Yufei <james.wangyufei@xxxxxxxxxx> > Signed-off-by: Zhang Bo <oscar.zhangbo@xxxxxxxxxx> > --- > src/util/vireventpoll.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/src/util/vireventpoll.c b/src/util/vireventpoll.c > index ffda206..9c9e7af 100644 > --- a/src/util/vireventpoll.c > +++ b/src/util/vireventpoll.c > @@ -357,9 +357,12 @@ static int virEventPollCalculateTimeout(int *timeout) > return -1; > > EVENT_DEBUG("Schedule timeout then=%llu now=%llu", then, now); > - *timeout = then - now; > - if (*timeout < 0) > + if (then <= now) { > *timeout = 0; > + } else { > + *timeout = (int) (then - now); This still won't fix the overflow issue since the same implicit typecast would be done without the explicit one. You probably should clamp the value to INT_MAX if you want to be safe. > + *timeout = (*timeout > 0) ? (*timeout) : (*timeout)*(-1); This would denote that timeout overflowed, hence you did not fix it at first. > + } > } else { > *timeout = -1; > } I'm not discussing the previous comments done by DanPB though. Peter
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list