[Libvir] [PATCH] Add the check of the format of MAC address on virsh attach-interface
Hi Even if specified MAC address is invalid, network interface is attached to the guest. And attached network interface cannot communicate. This patch checks the format of MAC address, and virsh become error when it is invalid. Signed-off-by: Masayuki Sunou <fj1826dm@aa.jp.fujitsu.com> Thanks, Masayuki Sunou. -------------------------------------------------------------------------------- Index: include/libvirt/virterror.h =================================================================== RCS file: /data/cvs/libvirt/include/libvirt/virterror.h,v retrieving revision 1.26 diff -u -p -r1.26 virterror.h --- include/libvirt/virterror.h 6 Jul 2007 14:56:15 -0000 1.26 +++ include/libvirt/virterror.h 12 Jul 2007 02:00:54 -0000 @@ -127,6 +127,7 @@ typedef enum { VIR_WAR_NO_NETWORK, /* failed to start network */ VIR_ERR_NO_DOMAIN, /* domain not found or unexpectedly disappeared */ VIR_ERR_NO_NETWORK, /* network not found */ + VIR_ERR_INVALID_MAC, /* invalid MAC adress */ } virErrorNumber; /** Index: src/virterror.c =================================================================== RCS file: /data/cvs/libvirt/src/virterror.c,v retrieving revision 1.28 diff -u -p -r1.28 virterror.c --- src/virterror.c 6 Jul 2007 14:56:15 -0000 1.28 +++ src/virterror.c 12 Jul 2007 02:00:54 -0000 @@ -646,6 +646,12 @@ __virErrorMsg(virErrorNumber error, cons else errmsg = _("Network not found: %s"); break; + case VIR_ERR_INVALID_MAC: + if (info == NULL) + errmsg = _("invalid MAC adress"); + else + errmsg = _("invalid MAC adress: %s"); + break; } return (errmsg); } Index: src/xml.c =================================================================== RCS file: /data/cvs/libvirt/src/xml.c,v retrieving revision 1.81 diff -u -p -r1.81 xml.c --- src/xml.c 11 Jul 2007 08:41:11 -0000 1.81 +++ src/xml.c 12 Jul 2007 02:00:55 -0000 @@ -934,8 +934,19 @@ virDomainParseXMLIfDesc(virConnectPtr co } virBufferAdd(buf, "(vif ", 5); - if (mac != NULL) + if (mac != NULL) { + unsigned int addr[12]; + int ret = sscanf((const char *)mac, "%01x%01x:%01x%01x:%01x%01x:%01x%01x:%01x%01x:%01x%01x", + (unsigned int*)&addr[0], (unsigned int*)&addr[1], (unsigned int*)&addr[2], + (unsigned int*)&addr[3], (unsigned int*)&addr[4], (unsigned int*)&addr[5], + (unsigned int*)&addr[6], (unsigned int*)&addr[7], (unsigned int*)&addr[8], + (unsigned int*)&addr[9], (unsigned int*)&addr[10], (unsigned int*)&addr[11]); + if (ret != 12 || strlen(mac) != 17) { + virXMLError(conn, VIR_ERR_INVALID_MAC, (const char *) mac, 0); + goto error; + } virBufferVSprintf(buf, "(mac '%s')", (const char *) mac); + } if (source != NULL) { if (typ == 0) virBufferVSprintf(buf, "(bridge '%s')", (const char *) source);
On Fri, Jul 13, 2007 at 02:24:01PM +0900, Masayuki Sunou wrote:
Hi
Even if specified MAC address is invalid, network interface is attached to the guest. And attached network interface cannot communicate.
This patch checks the format of MAC address, and virsh become error when it is invalid.
Signed-off-by: Masayuki Sunou <fj1826dm@aa.jp.fujitsu.com>
Hum, right, I don't expect to see non-ethernet MAC addresses at that point, and doing the checking earlier allows a better handling of the error. This makes sense so applied, but I fixed the code to avoid a couple of warnings from gcc. thanks ! Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
Hi Daniel
Hum, right, I don't expect to see non-ethernet MAC addresses at that point, and doing the checking earlier allows a better handling of the error. This makes sense so applied, but I fixed the code to avoid a couple of warnings from gcc.
I missed "warning" message from gcc. Thanks a lot for your fixing. Masayuki Sunou In message <20070713082727.GB30293@redhat.com> "Re: [Libvir] [PATCH] Add the check of the format of MAC address on virsh attach-interface" "Daniel Veillard <veillard@redhat.com>" wrote:
On Fri, Jul 13, 2007 at 02:24:01PM +0900, Masayuki Sunou wrote:
Hi
Even if specified MAC address is invalid, network interface is attached to the guest. And attached network interface cannot communicate.
This patch checks the format of MAC address, and virsh become error when it is invalid.
Signed-off-by: Masayuki Sunou <fj1826dm@aa.jp.fujitsu.com>
Hum, right, I don't expect to see non-ethernet MAC addresses at that point, and doing the checking earlier allows a better handling of the error. This makes sense so applied, but I fixed the code to avoid a couple of warnings from gcc.
thanks !
Daniel
-- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
On Fri, Jul 13, 2007 at 02:24:01PM +0900, Masayuki Sunou wrote:
Hi
Even if specified MAC address is invalid, network interface is attached to the guest. And attached network interface cannot communicate.
This patch checks the format of MAC address, and virsh become error when it is invalid.
It looks reasonable, but if we're going todo this, we should do it everywhere, not just in Xen's attach code - we, we should have a file mac.c for validating mac addresses & call if from the Xen, QEMU and Test drivers at all neccesssary places. I think in general now that we've got all drivers using the internal driver API, we need to make sure when we code these kind of fixes we make sure as much code can be shared between drivers as possible. Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
participants (3)
-
Daniel P. Berrange -
Daniel Veillard -
Masayuki Sunou