Re: [PATCH] conf: Check for bandwidth limits during parsing

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

 



On Thu, Jul 04, 2024 at 13:02:46 +0200, Michal Privoznik wrote:
> The 'tc' program stores speeds in 64bit integers (unit is bytes
> per second) and sizes in uints (unit is bytes). We use different
> units: kilobytes per second and kibibytes and therefore we can
> parse values larger than 'tc' can handle. Reject those values
> right away.
> 
> And while at it, fix the schema which assumed speed values fit
> into uint.
> 
> Resolves: https://issues.redhat.com/browse/RHEL-45200
> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
> ---
>  src/conf/netdev_bandwidth_conf.c   | 17 +++++++++++++++++
>  src/conf/schemas/networkcommon.rng |  2 +-
>  2 files changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/src/conf/netdev_bandwidth_conf.c b/src/conf/netdev_bandwidth_conf.c
> index 9faa46a27f..f3f0b2209a 100644
> --- a/src/conf/netdev_bandwidth_conf.c
> +++ b/src/conf/netdev_bandwidth_conf.c
> @@ -24,6 +24,16 @@
>  
>  #define VIR_FROM_THIS VIR_FROM_NONE
>  
> +#define CHECK_LIMIT(val, limit, name) \
> +    do { \
> +        if ((val) > (limit)) { \
> +            virReportError(VIR_ERR_OVERFLOW, \
> +                           _("value '%1$llu' is too big for '%2$s' parameter, maximum is '%3$llu'"), \
> +                           val, name, (unsigned long long) limit); \
> +            return -1; \
> +        } \
> +    } while (0)
> +
>  static int
>  virNetDevBandwidthParseRate(xmlNodePtr node,
>                              virNetDevBandwidthRate *rate,
> @@ -50,6 +60,11 @@ virNetDevBandwidthParseRate(xmlNodePtr node,
>                                          &rate->floor)) < 0)
>          return -1;
>  
> +    CHECK_LIMIT(rate->average, 1ULL << 54, "average");
> +    CHECK_LIMIT(rate->peak, 1ULL << 54, "peak");
> +    CHECK_LIMIT(rate->burst, UINT_MAX >> 10, "burst");
> +    CHECK_LIMIT(rate->floor, 1ULL << 54, "floor");

This is making the parser stricter than it used to be, which could
result into failures when parsing existing libvirt-stored configs.

'virNetDevBandwidthParseRate()' is called from
'virNetDevBandwidthParse()' which in turn is used e.g. in
'virDomainNetDefParseXML()' thus in worst case could lead into losing
defined VMs.

As of such I don't think this validation can be in the parser.



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux