[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC PATCH v6 3/5] softmmu/vl: Allow -fw_cfg 'blob_id' option to set
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [RFC PATCH v6 3/5] softmmu/vl: Allow -fw_cfg 'blob_id' option to set any file pathname |
Date: |
Thu, 28 May 2020 19:03:18 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0 |
On 5/20/20 12:45 AM, Laszlo Ersek wrote:
> On 05/19/20 20:20, Philippe Mathieu-Daudé wrote:
>> This is to silent:
>>
>> $ qemu-system-x86_64 \
>> -object tls-cipher-suites,id=ciphersuite0,priority=@SYSTEM \
>> -fw_cfg name=etc/edk2/https/ciphers,blob_id=ciphersuite0
>> qemu-system-x86_64: -fw_cfg
>> name=etc/edk2/https/ciphers,blob_id=ciphersuite0: warning: externally
>> provided fw_cfg item names should be prefixed with "opt/"
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>> softmmu/vl.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/softmmu/vl.c b/softmmu/vl.c
>> index f76c53ad2e..3b77dcc00d 100644
>> --- a/softmmu/vl.c
>> +++ b/softmmu/vl.c
>> @@ -2052,7 +2052,7 @@ static int parse_fw_cfg(void *opaque, QemuOpts *opts,
>> Error **errp)
>> FW_CFG_MAX_FILE_PATH - 1);
>> return -1;
>> }
>> - if (strncmp(name, "opt/", 4) != 0) {
>> + if (!nonempty_str(blob_id) && strncmp(name, "opt/", 4) != 0) {
>> warn_report("externally provided fw_cfg item names "
>> "should be prefixed with \"opt/\"");
>> }
>>
>
> Hmmm, difficult question! Is "ciphersuite0" now externally provided or not?
>
> Because, ciphersuite0 is populated internally to QEMU alright (and so we
> can think it's internal), but its *association with the name* is external.
>
> What if we keep the same "-object" switch, but use a different (bogus)
> "name" with "-fw_cfg"? IMO that kind of invalidates "-object" too.
>
> Should the fw_cfg generator interface dictate the fw_cfg filename too?
> Because that would eliminate this problem. Put differently, we now have
> a possibility to label the "ciphersuite0" object in the fw_cfg file
> directory any way we want -- but is that freedom *useful* for anything?
Good point. Not now.
>
> I guess we might want multiple "tls-cipher-suites" objects one day, so
> hard-coding the fw_cfg names on that level could cause conflicts.
I haven't thought of that. If we set a limit today, we can relax it
tomorrow. That won't break anything.
> On the
> other hand, I wouldn't like "blob_id" to generally circumvent the "etc/"
> namespace protection.
Eh, that was too easy.
>
> I'm leaning towards agreeing with this patch, but I'd appreciate some
> convincing arguments.
OK. I'll think about it. Maybe keep this as single-RFC patch & repost
the rest so review can go on until we settle on this.
>
> Thanks
> Laszlo
>
- [PATCH v6 2/5] softmmu/vl: Let -fw_cfg option take a 'blob_id' argument, (continued)
- [PATCH v6 2/5] softmmu/vl: Let -fw_cfg option take a 'blob_id' argument, Philippe Mathieu-Daudé, 2020/05/19
- [PATCH v6 1/5] hw/nvram/fw_cfg: Add the FW_CFG_DATA_GENERATOR interface, Philippe Mathieu-Daudé, 2020/05/19
- [RFC PATCH v6 3/5] softmmu/vl: Allow -fw_cfg 'blob_id' option to set any file pathname, Philippe Mathieu-Daudé, 2020/05/19
- [PATCH v6 5/5] crypto/tls-cipher-suites: Product fw_cfg consumable blob, Philippe Mathieu-Daudé, 2020/05/19
- [PATCH v6 4/5] crypto: Add tls-cipher-suites object, Philippe Mathieu-Daudé, 2020/05/19
- Re: [PATCH v6 0/5] fw_cfg: Add FW_CFG_DATA_GENERATOR; crypto: Add tls-cipher-suites, Philippe Mathieu-Daudé, 2020/05/27