1. Patchew
  2. Xen
  3. View series

[PATCH v2] xen/sched: validate RTDS putinfo period and budget

Oleksii Moisieiev posted 1 patch 1 week, 1 day ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/c629e66ebf05d620423babf1e4e98866c1f75357.1774452210.git.oleksii._5Fmoisieiev@epam.com
There is a newer version of this series
xen/common/sched/rt.c | 37 ++++++++++++++++++++++++-------------
1 file changed, 24 insertions(+), 13 deletions(-)
[PATCH v2] xen/sched: validate RTDS putinfo period and budget
Posted by Oleksii Moisieiev 1 week, 1 day ago 
The RTDS domain-wide XEN_DOMCTL_SCHEDOP_putinfo path only checks for
zero values before applying period and budget to all vCPUs in the
domain.

This is weaker than the per-vCPU XEN_DOMCTL_SCHEDOP_putvcpuinfo path,
which already rejects values below the minimum, above the maximum, and
cases where budget exceeds period.

Use the same validation rules for putinfo as for putvcpuinfo, so
invalid domain-wide updates are rejected with -EINVAL instead of being
applied inconsistently.

Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
---

Changes in v2:
- introduce rt_validate_params helper function to check period and budget

 xen/common/sched/rt.c | 37 ++++++++++++++++++++++++-------------
 1 file changed, 24 insertions(+), 13 deletions(-)

diff --git a/xen/common/sched/rt.c b/xen/common/sched/rt.c
index 7b1f64a779..645b091de7 100644
--- a/xen/common/sched/rt.c
+++ b/xen/common/sched/rt.c
@@ -1362,6 +1362,20 @@ out:
     unit_schedule_unlock_irq(lock, unit);
 }
 
+static int
+rt_validate_params(uint32_t period_us, uint32_t budget_us,
+                   s_time_t *period, s_time_t *budget)
+{
+    *period = MICROSECS(period_us);
+    *budget = MICROSECS(budget_us);
+
+    if ( *period > RTDS_MAX_PERIOD || *budget < RTDS_MIN_BUDGET ||
+         *budget > *period || *period < RTDS_MIN_PERIOD )
+        return -EINVAL;
+
+    return 0;
+}
+
 /*
  * set/get each unit info of each domain
  */
@@ -1388,17 +1402,17 @@ rt_dom_cntl(
         op->u.rtds.budget = RTDS_DEFAULT_BUDGET / MICROSECS(1);
         break;
     case XEN_DOMCTL_SCHEDOP_putinfo:
-        if ( op->u.rtds.period == 0 || op->u.rtds.budget == 0 )
-        {
-            rc = -EINVAL;
+        rc = rt_validate_params(op->u.rtds.period, op->u.rtds.budget,
+                                &period, &budget);
+        if ( rc )
             break;
-        }
+
         spin_lock_irqsave(&prv->lock, flags);
         for_each_sched_unit ( d, unit )
         {
             svc = rt_unit(unit);
-            svc->period = MICROSECS(op->u.rtds.period); /* transfer to nanosec */
-            svc->budget = MICROSECS(op->u.rtds.budget);
+            svc->period = period;
+            svc->budget = budget;
         }
         spin_unlock_irqrestore(&prv->lock, flags);
         break;
@@ -1440,14 +1454,11 @@ rt_dom_cntl(
             }
             else
             {
-                period = MICROSECS(local_sched.u.rtds.period);
-                budget = MICROSECS(local_sched.u.rtds.budget);
-                if ( period > RTDS_MAX_PERIOD || budget < RTDS_MIN_BUDGET ||
-                     budget > period || period < RTDS_MIN_PERIOD )
-                {
-                    rc = -EINVAL;
+                rc = rt_validate_params(local_sched.u.rtds.period,
+                                        local_sched.u.rtds.budget,
+                                        &period, &budget);
+                if ( rc )
                     break;
-                }
 
                 spin_lock_irqsave(&prv->lock, flags);
                 svc = rt_unit(d->vcpu[local_sched.vcpuid]->sched_unit);
-- 
2.43.0

base-commit: a7bf8ff218ca05eb3674fdfd2817f6cff471e96a
branch: amoi_rtds_SCHEDOP_putinfov2
Re: [PATCH v2] xen/sched: validate RTDS putinfo period and budget
Posted by Andrew Cooper 1 week, 1 day ago 
On 25/03/2026 3:24 pm, Oleksii Moisieiev wrote:
> The RTDS domain-wide XEN_DOMCTL_SCHEDOP_putinfo path only checks for
> zero values before applying period and budget to all vCPUs in the
> domain.
>
> This is weaker than the per-vCPU XEN_DOMCTL_SCHEDOP_putvcpuinfo path,
> which already rejects values below the minimum, above the maximum, and
> cases where budget exceeds period.
>
> Use the same validation rules for putinfo as for putvcpuinfo, so
> invalid domain-wide updates are rejected with -EINVAL instead of being
> applied inconsistently.
>
> Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
> ---
>
> Changes in v2:
> - introduce rt_validate_params helper function to check period and budget
>
>  xen/common/sched/rt.c | 37 ++++++++++++++++++++++++-------------
>  1 file changed, 24 insertions(+), 13 deletions(-)
>
> diff --git a/xen/common/sched/rt.c b/xen/common/sched/rt.c
> index 7b1f64a779..645b091de7 100644
> --- a/xen/common/sched/rt.c
> +++ b/xen/common/sched/rt.c
> @@ -1362,6 +1362,20 @@ out:
>      unit_schedule_unlock_irq(lock, unit);
>  }
>  
> +static int
> +rt_validate_params(uint32_t period_us, uint32_t budget_us,
> +                   s_time_t *period, s_time_t *budget)
> +{
> +    *period = MICROSECS(period_us);
> +    *budget = MICROSECS(budget_us);
> +
> +    if ( *period > RTDS_MAX_PERIOD || *budget < RTDS_MIN_BUDGET ||
> +         *budget > *period || *period < RTDS_MIN_PERIOD )
> +        return -EINVAL;
> +
> +    return 0;
> +}

Code written like this is horrible; both to read, and in terms of
generated code.  Because of potential aliasing, that's 7 distinct memory
accesses because the values cannot be cached in registers.

You'll get far better code generation by writing it more like:

{
    s_time_t p = MICROSECS(period_us);
    s_time_t b = MICROSECS(budget_us);

    if ( p > RTDS_MAX_PERIOD || ... )
        return -EINVAL;

    *period = p;
    *budget = b;

    return 0;
}

See https://godbolt.org/z/W63TY8qTW

But it would also be better still if you passed op->u.rtds into this
function rather than {period,budget}_us separately.

~Andrew
Re: [PATCH v2] xen/sched: validate RTDS putinfo period and budget
Posted by Oleksii Moisieiev 6 days, 12 hours ago 
On 25/03/2026 17:45, Andrew Cooper wrote:
> On 25/03/2026 3:24 pm, Oleksii Moisieiev wrote:
>> The RTDS domain-wide XEN_DOMCTL_SCHEDOP_putinfo path only checks for
>> zero values before applying period and budget to all vCPUs in the
>> domain.
>>
>> This is weaker than the per-vCPU XEN_DOMCTL_SCHEDOP_putvcpuinfo path,
>> which already rejects values below the minimum, above the maximum, and
>> cases where budget exceeds period.
>>
>> Use the same validation rules for putinfo as for putvcpuinfo, so
>> invalid domain-wide updates are rejected with -EINVAL instead of being
>> applied inconsistently.
>>
>> Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
>> ---
>>
>> Changes in v2:
>> - introduce rt_validate_params helper function to check period and budget
>>
>>   xen/common/sched/rt.c | 37 ++++++++++++++++++++++++-------------
>>   1 file changed, 24 insertions(+), 13 deletions(-)
>>
>> diff --git a/xen/common/sched/rt.c b/xen/common/sched/rt.c
>> index 7b1f64a779..645b091de7 100644
>> --- a/xen/common/sched/rt.c
>> +++ b/xen/common/sched/rt.c
>> @@ -1362,6 +1362,20 @@ out:
>>       unit_schedule_unlock_irq(lock, unit);
>>   }
>>   
>> +static int
>> +rt_validate_params(uint32_t period_us, uint32_t budget_us,
>> +                   s_time_t *period, s_time_t *budget)
>> +{
>> +    *period = MICROSECS(period_us);
>> +    *budget = MICROSECS(budget_us);
>> +
>> +    if ( *period > RTDS_MAX_PERIOD || *budget < RTDS_MIN_BUDGET ||
>> +         *budget > *period || *period < RTDS_MIN_PERIOD )
>> +        return -EINVAL;
>> +
>> +    return 0;
>> +}
> Code written like this is horrible; both to read, and in terms of
> generated code.  Because of potential aliasing, that's 7 distinct memory
> accesses because the values cannot be cached in registers.
>
> You'll get far better code generation by writing it more like:
>
> {
>      s_time_t p = MICROSECS(period_us);
>      s_time_t b = MICROSECS(budget_us);
>
>      if ( p > RTDS_MAX_PERIOD || ... )
>          return -EINVAL;
>
>      *period = p;
>      *budget = b;
>
>      return 0;
> }
>
> See https://godbolt.org/z/W63TY8qTW
>
> But it would also be better still if you passed op->u.rtds into this
> function rather than {period,budget}_us separately.
>
> ~Andrew


Hi Andrew,

Thank you for pointing this out.

Will fix and make v3

--

Oleksii.

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.