Re: [Xen-devel] [PATCH v3 06/12] livepatch: Do not enforce ELF_LIVEPATCH_FUNC section presence

[Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>]

On 9/16/19 11:59 AM, Pawel Wieczorkiewicz wrote:
> With default implementation the ELF_LIVEPATCH_FUNC section containing
> all functions to be replaced or added must be part of the hotpatch
> payload, otherwise the payload is rejected (with -EINVAL).
>
> However, with the extended hooks implementation, a hotpatch may be
> constructed of only hooks to perform certain actions without any code
> to be added or replaced.
> Therefore, do not always expect the functions section and allow it to
> be missing, provided there is at least one section containing hooks
> present. The functions section, when present in a payload, must be a
> single, non-empty section.
>
> Check also all extended hooks sections if they are a single, non-empty
> sections each.
>
> At least one of the functions or hooks section must be present in a
> valid payload.
snip
> +    /*
> +     * The patching sections are optional, but at least one
> +     * must be present. Otherwise, there is nothing to do.
> +     * All the existing sections must not be empty and must
> +     * be present at most once.
> +     */
> +    for ( i = 0; i < ARRAY_SIZE(names); i++ )
> +    {
> +        const struct livepatch_elf_sec *sec;
> +
> +        sec = livepatch_elf_sec_by_name(elf, names[i]);
> +        if ( !sec )
> +        {
> +            dprintk(XENLOG_DEBUG, LIVEPATCH "%s: %s is missing\n",
> +                    elf->name, names[i]);
> +            continue; /* This section is optional */
> +        }
> +
> +        if ( !sec->sec->sh_size )
> +        {
> +            printk(XENLOG_ERR LIVEPATCH "%s: %s is empty\n",
> +                   elf->name, names[i]);
> +            return -EINVAL;
> +        }
> +
> +        if ( test_and_set_bit(i, found) )
> +        {
> +            printk(XENLOG_ERR LIVEPATCH "%s: %s was seen more than once\n",
> +                    elf->name, names[i]);
This indentation is wrong.

> +            return -EINVAL;
> +        }
> +    }
> +
> +    /* Checking if at least one section is present. */
> +    if ( bitmap_empty(found, ARRAY_SIZE(names)) )
> +    {
> +        printk(XENLOG_ERR LIVEPATCH "%s: Nothing to patch. Aborting...\n",
> +               elf->name);
> +        return -EINVAL;
> +    }
> +
> +    return 0;
> +}
> +
LGTM,

Reviewed-by: Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>

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