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

Re: [Libvir] PATCH: 9/16: main internal storage driver



"Daniel P. Berrange" <berrange redhat com> wrote:
> +static int virStorageSize(virConnectPtr conn,
> +                          const char *unit,
> +                          const char *val,
> +                          unsigned long long *ret) {
> +    unsigned long long mult;
> +    char *end;
> +
> +    if (!unit) {
> +        mult = 1;
> +    } else {
> +        switch (unit[0]) {
> +        case 'k':
> +        case 'K':
> +            mult = 1024ull;
> +            break;
> +
> +        case 'm':
> +        case 'M':
> +            mult = 1024ull * 1024ull;
> +            break;
> +
> +        case 'g':
> +        case 'G':
> +            mult = 1024ull * 1024ull * 1024ull;
> +            break;
> +
> +        case 't':
> +        case 'T':
> +            mult = 1024ull * 1024ull * 1024ull * 1024ull;
> +            break;

Hi Dan,

>From our current perspective, 'T' looks big enough, but you might want
to add 'P', 'Y', and 'Z', if only so the code doesn't seem clueless
when reporting "unknown size units" :-)

> +        default:
> +            virStorageReportError(conn, VIR_ERR_XML_ERROR, "Unknown size units %s", unit);

Alternatively,

  s/Unknown/Invalid/

> +            return -1;
> +        }
> +    }
> +
> +    *ret = strtoull(val, &end, 10);
> +    if (end && *end) {
> +        virStorageReportError(conn, VIR_ERR_XML_ERROR, "malformed capacity element");
> +        return -1;
> +    }
> +    *ret *= mult;

If you change the guard to e.g.,

    if (*end || *ret > ULLONG_MAX / mult) {
        virStorageReportError(conn, VIR_ERR_XML_ERROR,
                              "malformed capacity element");

you avoid risk of overflow.


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