Re: [libvirt] [PATCH] virnetdevtap: Don't crash on !ifname in virNetDevTapInterfaceStats

[Andrea Bolognani]

On Fri, 2018-07-13 at 11:24 +0200, Michal Privoznik wrote:
[...]
> +if (!ifname) {
> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +   _("Interface not found"));
> +return -1;
> +}

I would use something like "Interface name not provided" here,
as it would better describe the cause of the error.

[...]
>  if (ifa->ifa_addr->sa_family != AF_LINK)
>  continue;
>  
> -if (STREQ(ifa->ifa_name, ifname)) {
> +if (STREQ_NULLABLE(ifa->ifa_name, ifname)) {

Why is the fix different for the BSD implementation? I would
expect you to just add the same snippet as above to guard the
rest of the function from ifname being NULL...

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] virnetdevtap: Don't crash on !ifname in virNetDevTapInterfaceStats

[Michal Privoznik]

https://bugzilla.redhat.com/show_bug.cgi?id=1595184

Some domain  do not have a name (because they are
not TAP devices). Therefore, if
virNetDevTapInterfaceStats(net->ifname, ...) is called an instant
crash occurs. In Linux version of the function strlen() is called
over the name and in BSD version STREQ() is called.

Signed-off-by: Michal Privoznik 
---
 src/util/virnetdevtap.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c
index bd0710ad2e..2123ffe718 100644
--- a/src/util/virnetdevtap.c
+++ b/src/util/virnetdevtap.c
@@ -691,6 +691,12 @@ virNetDevTapInterfaceStats(const char *ifname,
 FILE *fp;
 char line[256], *colon;
 
+if (!ifname) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("Interface not found"));
+return -1;
+}
+
 fp = fopen("/proc/net/dev", "r");
 if (!fp) {
 virReportSystemError(errno, "%s",
@@ -781,7 +787,7 @@ virNetDevTapInterfaceStats(const char *ifname,
 if (ifa->ifa_addr->sa_family != AF_LINK)
 continue;
 
-if (STREQ(ifa->ifa_name, ifname)) {
+if (STREQ_NULLABLE(ifa->ifa_name, ifname)) {
 ifd = (struct if_data *)ifa->ifa_data;
 if (swapped) {
 stats->tx_bytes = ifd->ifi_ibytes;
-- 
2.16.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list