Re: [Xen-devel] [PATCH v2 5/6] tools/dm_depriv: Add first cut RLIMITs

[Ian Jackson <ian.jackson@xxxxxxxxxx>]

George Dunlap writes ("[PATCH v2 5/6] tools/dm_depriv: Add first cut RLIMITs"):
> Limit the ability of a potentially compromised QEMU to consume system
> resources.  Key limits:
...
> +static struct {
> +    int resource;
> +    rlim_t limit;
> +} rlimits[] = {
> +    {
> +        .resource = RLIMIT_FSIZE,
> +        /* Big enough for log files, not big enough for a DoS */
> +        .limit = 256*1024,
> +    },
> +    {
> +        .resource = RLIMIT_NPROC,
> +        .limit = 0
> +    },
> +    {
> +        .resource = RLIMIT_CORE,
> +        .limit = 0
> +    },
> +    {
> +        .resource = RLIMIT_MSGQUEUE,
> +        .limit = 0
> +    },
> +    {
> +        .resource = RLIMIT_LOCKS,
> +        .limit = 0
> +    },
> +    {
> +        .resource = RLIMIT_MEMLOCK,
> +        .limit = 0

This table would be much nicer in table format, with each entry as a
row.  If the entries are too long to wrap then options include:
 * Use an intermediate struct so as to avoid having to use
    named initialisers
 * Introduce a macro to define each table row (I think
    this is better).

> +    /* Set various "easy" rlimits */
> +    for (i=0; rlimits[i].resource != -1; i++) {

It would be more idiomatic to walk a pointer along the array, rather
than writing rlimits[i] all the time.

> +        if (rc < 0) {
> +            char *msg = GCSPRINTF("libxl: Setting rlimit %d to %lld failed 
> with error %d\n",
> +                                  rlimits[i].resource,

If you cared very much about the error handling, you could produce
names rather than numbers for the rlimit values by wrapping the struct
rlimit up in a struct of our own with a `const char *name' field, and
having a macro to generate the table entries.

But it's probably not worth it.

Again, use `r' for syscall returns.  And, again, don't print numerical
errno values.  use LOGE.

...
> +    if [[ "$input" =~ 
> ^$limit_string[[:space:]]*([^[:space:]]+)[[:space:]]*([^[:space:]]+)[[:space:]]*[^[:space:]]+
>  ]] ; then
> +     if [[ "${BASH_REMATCH[1]}" != $tgt ||

Cor.  I think I would have reached for Perl by now.  I guess tastes
differ.

I guess you have tested that this does all pass and fail
appropriately ?


About the output format from this checker script.  As it is, osstest
will have to grow an additional parser to parse this.  I don't mind
that if it's subunit v1 (or some other plsusible standard).

But if it's going to be bespoke it would be much more convenient if
the output was parseable by the same parser as the oiutput from
depriv-fd-checker.

Thanks,
Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel