netdev - Re: virtio_net: MAC address releated breakage if there is no MAC area in config

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 02 Apr 2009 10:06:25 -0600
From:	Alex Williamson <alex.williamson@...com>
To:	Christian Borntraeger <borntraeger@...ibm.com>
Cc:	Rusty Russell <rusty@...tcorp.com.au>, netdev@...r.kernel.org,
	kvm@...r.kernel.org
Subject: Re: virtio_net: MAC address releated breakage if there is no MAC
 area in config

On Thu, 2009-04-02 at 13:33 +0200, Christian Borntraeger wrote:
> I read this as the mac config field is optional (similar to all the optional
> fields we added in virtio_blk later).
> 
> I see two options:
> 1. Change our sample userspace to always allocate the config (like lguest and
> qemu)
> 2. Change the kernel code to not write into the config unless a specific feature
> bit is set. (e.g. VIRTIO_NET_F_SETMAC)
> 
> 
> Opinions?

Hi Christian,

Sorry for the breakage.  My interpretation of the virtio-net config
space was that the mac field was always present and the host had
programmed a valid value when the F_MAC feature is available.  However,
from the history of the flag, it seems like you're interpretation is
likely correct.  Setting the config value from the randomly generated
mac was largely opportunistic since there's no userspace that doesn't
provide a mac by default.  So perhaps we can drop that and gate the
set_mac_address entry point as shown below.  How does this look?
Thanks,

Alex


virtio_net: Set the mac config only when VIRITO_NET_F_MAC

VIRTIO_NET_F_MAC indicates the presence of the mac field in config
space, not the validity of the value it contains.  Allow the mac to be
changed at runtime, but only push the change into config space with the
VIRTIO_NET_F_MAC feature present.

Signed-off-by: Alex Williamson <alex.williamson@...com>
--

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index a6f1e19..9c82a39 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -575,8 +575,9 @@ static int virtnet_set_mac_address(struct net_device *dev, void *p)
 	if (ret)
 		return ret;
 
-	vdev->config->set(vdev, offsetof(struct virtio_net_config, mac),
-			  dev->dev_addr, dev->addr_len);
+	if (virtio_has_feature(vdev, VIRTIO_NET_F_MAC))
+		vdev->config->set(vdev, offsetof(struct virtio_net_config, mac),
+		                  dev->dev_addr, dev->addr_len);
 
 	return 0;
 }
@@ -876,11 +877,8 @@ static int virtnet_probe(struct virtio_device *vdev)
 		vdev->config->get(vdev,
 				  offsetof(struct virtio_net_config, mac),
 				  dev->dev_addr, dev->addr_len);
-	} else {
+	} else
 		random_ether_addr(dev->dev_addr);
-		vdev->config->set(vdev, offsetof(struct virtio_net_config, mac),
-				  dev->dev_addr, dev->addr_len);
-	}
 
 	/* Set up our device-specific information */
 	vi = netdev_priv(dev);


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html