Re: [PATCH] virMacAddrParse: Fix wrong termination character

[Eustance Wu <eustancewu@xxxxxxxxx>]

Thanks a lot. 

In libvirt of the latest code of the current master: When configuring the mac address of the DHCP host (c0:3b:04:21:15:35), if a space is added at the end (c0:3b:04:21:15:35\32 ), no error is thrown when starting the network.
I think an error should be thrown because such a mac (c0:3b:04:21:15:35\32) is not legal
And the mac address (c0:3b:04:21:15:35) does not take effect
The diagram is as follows:

> 
>          addr->addr[i] = (unsigned char) result;
> 
> -        if ((i == 5) && (*end_ptr <= ' '))
> +        if ((i == 5) && (*end_ptr == '\0'))

Can I mention a patch for this issue?
Will this affect other functions?

Peter Krempa <pkrempa@xxxxxxxxxx> 于2022年6月16日周四 14:52写道：

On Thu, Jun 16, 2022 at 14:09:06 +0800, Eustance Wu wrote:
> From ef22e53c9360ddb4bdff61a12013a2812fb7346a Mon Sep 17 00:00:00 2001
> From: longtao <longtao.wu@xxxxxxxxx>
> Date: Thu, 16 Jun 2022 14:08:14 +0800
> Subject: [PATCH] virMacAddrParse: Fix wrong termination character
>
> The judgment of the termination character should be the '\0' character, not
> a space.
> Using spaces to judge, content can be injected into mac. such as:
> "70:af:e7:1f:3f:89\32injected".
>
> Before this patch, the terminating character was a space ('\32'),not '\0'.

This sentence is inaccurate, it was any byte between 0 and ' '.

> So I can set the DHCP host mac like this "<host mac='c0:3b:04:21:15:35
>  injected' name='name129' ip='192.168.100.129'/>".

This format does not conform to our XML schema. The 'mac' attribute is
defined as:

  <define name="uniMacAddr">
    <data type="string">
      <param name="pattern">[a-fA-F0-9][02468aAcCeE](:[a-fA-F0-9]{2}){5}</param>
    </data>
  </define>

Thus 'space' or any other character is _not_ allowed in int including
the word 'injected'.

What are you actually trying to achieve? This is not clear from the
description in this bug.

Please describe also the end goal ...

> When running the network, no error is reported.
> But, when using this mac to create a virtual machine,  Will get
> "virNetSocketReadWire:1805 : End of file while reading data: Input/output

... rather than just an error you are seeing, since this doesn't seem to
be a plain bugfix.

> error" in the libvirtd log.
> ---
>  src/util/virmacaddr.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/util/virmacaddr.c b/src/util/virmacaddr.c
> index 6b22384cee..ba7c7e7076 100644
> --- a/src/util/virmacaddr.c
> +++ b/src/util/virmacaddr.c
> @@ -163,7 +163,7 @@ virMacAddrParse(const char* str, virMacAddr *addr)

This helper is used in many places, outside of the network driver, so
this will need careful assesment whether any other code depends on the
old behaviour.

>
>          addr->addr[i] = (unsigned char) result;
>
> -        if ((i == 5) && (*end_ptr <= ' '))
> +        if ((i == 5) && (*end_ptr == 0))

We prefer to use the character literal version of the nul byte ('\0').

>              return 0;
>          if (*end_ptr != ':')
>              break;
> --
> 2.32.0