[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH for 4.5 v4 1/4] xen: add real time scheduler rtds

On 09/24/2014 02:25 PM, Meng Xu wrote:
2014-09-24 9:14 GMT-04:00 Dario Faggioli <dario.faggioli@xxxxxxxxxx>:
On mer, 2014-09-24 at 13:09 +0100, Jan Beulich wrote:
On 21.09.14 at 00:13, <mengxu@xxxxxxxxxxxxx> wrote:
*** CID 1240234:  Division or modulo by zero  (DIVIDE_BY_ZERO)
/xen/common/sched_rt.c: 327 in rt_update_deadline()
321             do
322                 svc->cur_deadline += svc->period;
323             while ( svc->cur_deadline <= now );
324         }
325         else
326         {
     CID 1240234:  Division or modulo by zero  (DIVIDE_BY_ZERO)
     In expression "(now - svc->cur_deadline) / svc->period", division by expression 
"svc->period" which may be zero has undefined behavior.
327             long count = ((now - svc->cur_deadline) / svc->period) + 1;
328             svc->cur_deadline += count * svc->period;
329         }
331         svc->cur_budget = svc->budget;


     ASSERT(svc->period != 0);

a few lines up. However, the ASSERT() itself is currently invalid
because above code doesn't make sure zero wouldn't get stored
into that field. I.e. the ASSERT() currently (indirectly) verifies
valid caller input rather than valid hypervisor state.

Right, good point. Sorry I missed this while reviewing the series.

This needs
to be fixed. And I would have sent a patch right away if it wasn't
unclear to me whether op->u.rtds.budget should also be checked
against zero (under the assumption that all other values are valid
for these two fields).

I'd go for doing the sanity checking in rt_dom_cntl(), SCHEDOP_putinfo
branch, of course... Meng, do you agree? If yes, can you send a patch to
that effect?
Sure! We need do sanity check for this, although the toolstack does
not allow zero value for period and budget of a vcpu. When I do the
sanity check, I will return EINVAL if period or budget is zero.

Yes, sorry I missed this -- the hypervisor always needs to do all of its own checking (which is why I initially said that it was redundant to do checking in libxl).


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.