On Thu, Jun 04, 2020 at 08:44:05PM +0200, Michal Privoznik wrote:
> There are recommendations and limitations to the name of the
> config blobs we need to follow [1].
>
> Firstly, we don't want users to change any value only add new
> blobs. This means, that the name must have "opt/" prefix and at
> the same time must not begin with "opt/ovmf" nor "opt/org.qemu"
> as these are reserved for OVMF or QEMU respectively.
>
> Secondly, there is a limit (FW_CFG_MAX_FILE_PATH in qemu.git) of
> 56 characters for filename.
Ewww, that is horrible. I'm have inclined to say we should leave the
limit unchecked in libvirt, and file a BZ against QEMU. It should be
using g_strdup_printf() with filenames and not allocating on the
stack. We already see peoiple exceeding the 100 charater limit of UNIX
sockets, so a 56 character limit is going to be trivially exceeded
without even trying hard.
> 1: docs/specs/fw_cfg.txt from qemu.git
>
> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
> ---
> src/qemu/qemu_validate.c | 40 ++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 40 insertions(+)
>
> diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
> index 584d1375b8..56a7ebfd7f 100644
> --- a/src/qemu/qemu_validate.c
> +++ b/src/qemu/qemu_validate.c
> @@ -762,6 +762,41 @@ qemuValidateDefGetVcpuHotplugGranularity(const virDomainDef *def)
> }
>
>
> +#define QEMU_FW_CFG_MAX_FILE_PATH 55
> +static int
> +qemuValidateDomainDefSysinfo(const virSysinfoDef *def,
> + virQEMUCapsPtr qemuCaps G_GNUC_UNUSED)
> +{
> + size_t i;
> +
> + for (i = 0; i < def->nfw_cfgs; i++) {
> + const virSysinfoFWCfgDef *f = &def->fw_cfgs[i];
> +
> + if (!STRPREFIX(f->name, "opt/")) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("Invalid firmware name"));
> + return -1;
> + }
> +
> + if (STRPREFIX(f->name, "opt/ovmf/") ||
> + STRPREFIX(f->name, "opt/org.qemu/")) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("That firmware name is reserved"));
> + return -1;
> + }
> +
> + if (f->file &&
> + strlen(f->file) > QEMU_FW_CFG_MAX_FILE_PATH) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("firmware file too long"));
> + return -1;
> + }
> + }
> +
> + return 0;
> +}
> +
> +
> int
> qemuValidateDomainDef(const virDomainDef *def,
> void *opaque)
> @@ -978,6 +1013,11 @@ qemuValidateDomainDef(const virDomainDef *def,
> }
> }
>
> + for (i = 0; i < def->nsysinfo; i++) {
> + if (qemuValidateDomainDefSysinfo(def->sysinfo[i], qemuCaps) < 0)
> + return -1;
> + }
> +
> return 0;
> }
>
> --
> 2.26.2
>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
