Re: [PATCH v2 4/8] qemu: Validate firmware blob configuration

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

 



On 6/9/20 11:52 AM, Daniel P. Berrangé wrote:
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.

Ah, I got it wrong when reading the documentation. The limit is the name of the entry, not file name associated. For instance the following works:

-fw_cfg name=opt/com.example/blah,file=/tmp/some_very_long_path_that_is_more_than_fifty_six_characters_long_to_see_what_happens_if_I_do_that

but if @name would be too long QEMU would fail:

qemu-system-x86_64: -fw_cfg name=opt/com.example/some_very_long_...,file=...: name too long (max. 55 char)

And this comes from kernel's implementation of qemu_fw_cfg. However, the name can be up to PAGE_SIZE long (minus 2 for terminating \n and NUL), according to sysfs documentation (Documentation/filesystems/sysfs.rst:241):

- The buffer will always be PAGE_SIZE bytes in length. On i386, this
  is 4096.

Given this new realization, I think I'll just remove the check and not fill bug. I don't think we need longer names, do we?

Michal




[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