Re: [PULL 03/16] net: validate that ids are well formed

[Paolo Bonzini <pbonzini@redhat.com>]

On 12/03/21 14:26, Jason Wang wrote:
> 
> On 2021/3/12 4:44 下午, Paolo Bonzini wrote:
>> On 12/03/21 07:16, Jason Wang wrote:
>>> From: Paolo Bonzini <pbonzini@redhat.com>
>>>
>>> When a network or network device is created from the command line or 
>>> HMP,
>>> QemuOpts ensures that the id passes the id_wellformed check. However,
>>> QMP skips this:
>>>
>>>     $ qemu-system-x86_64 -qmp stdio -S -nic user,id=123/456
>>>     qemu-system-x86_64: -nic user,id=123/456: Parameter id expects an 
>>> identifier
>>>     Identifiers consist of letters, digits, -, ., _, starting with a 
>>> letter.
>>>
>>>     $ qemu-system-x86_64 -qmp stdio -S
>>>     {"execute":"qmp_capabilities"}
>>>     {"return": {}}
>>>     {"execute":"netdev_add", "arguments": {"type": "user", "id": 
>>> "123/456"}}
>>>     {"return": {}}
>>>
>>> After:
>>>
>>>     $ qemu-system-x86_64 -qmp stdio -S
>>>     {"execute":"qmp_capabilities"}
>>>     {"return": {}}
>>>     {"execute":"netdev_add", "arguments": {"type": "user", "id": 
>>> "123/456"}}
>>>     {"error": {"class": "GenericError", "desc": "Parameter "id" 
>>> expects an identifier"}}
>>>
>>> Validity checks should be performed always at the bottom of the call 
>>> chain,
>>> because QMP skips all the steps above.  Do this for the network 
>>> subsystem.
>>>
>>> Cc: Jason Wang <jasowang@redhat.com>
>>> Reviewed-by: Eric Blake <eblake@redhat.com>
>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>>> ---
>>>   net/net.c | 12 ++++++++++++
>>>   1 file changed, 12 insertions(+)
>>>
>>> diff --git a/net/net.c b/net/net.c
>>> index 9c784da..d36729f 100644
>>> --- a/net/net.c
>>> +++ b/net/net.c
>>> @@ -44,6 +44,7 @@
>>>   #include "qemu/cutils.h"
>>>   #include "qemu/config-file.h"
>>>   #include "qemu/ctype.h"
>>> +#include "qemu/id.h"
>>>   #include "qemu/iov.h"
>>>   #include "qemu/qemu-print.h"
>>>   #include "qemu/main-loop.h"
>>> @@ -1011,6 +1012,17 @@ static int net_client_init1(const Netdev 
>>> *netdev, bool is_netdev, Error **errp)
>>>           }
>>>       }
>>>   +    /*
>>> +     * The id for -net has already been checked by QemuOpts and
>>> +     * could be automatically generated, in which case it is not
>>> +     * well-formed by design.  HMP and QMP only call us with
>>> +     * is_netdev == true.
>>> +     */
>>> +    if (is_netdev && !id_wellformed(netdev->id)) {
>>> +        error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "id", "an 
>>> identifier");
>>> +        return -1;
>>> +    }
>>> +
>>>       nc = qemu_find_netdev(netdev->id);
>>>       if (nc) {
>>>           error_setg(errp, "Duplicate ID '%s'", netdev->id);
>>>
>>
>> Sorry, I sent v2 yesterday.  This patch passed the tests at the time 
>> it was submitted, but now fails (because it does not work with -nic).
> 
> I don't see that. But I add a fixup in the pull request:
> 
> https://lists.gnu.org/archive/html/qemu-devel/2021-03/msg04237.html
> 
> If it doesn't make sense, I will drop this and send a new pull request 
> next week.

Hmm no, it didn't reach the list.  Let me resend.

Paolo