[PATCH] lxc: Fix segfault when lxc.network does not start with 'type'

Julio Faracco posted 1 patch 2 weeks ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20200206021205.15424-1-jcfaracco@gmail.com
src/lxc/lxc_native.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)

[PATCH] lxc: Fix segfault when lxc.network does not start with 'type'

Posted by Julio Faracco 2 weeks ago
To configure network settings using config file, legacy LXC settings
require starting them with 'lxc.network.type' entry. If someone
accidentally starts with 'lxc.network.name', libvirt will crash with
segfault. This patch checks if this case is happening.

Sample invalid settings:
lxc.network.link = eth0
lxc.network.type = phys
lxc.network.name = eth1
lxc.network.ipv4 = 192.168.122.2/24
lxc.network.ipv4.gateway = 192.168.122.1

Now, libvirt only see error without segmentation fault.

Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
---
 src/lxc/lxc_native.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c
index 59f3dd4fee..5462b74b85 100644
--- a/src/lxc/lxc_native.c
+++ b/src/lxc/lxc_native.c
@@ -717,7 +717,11 @@ lxcNetworkGetParseDataByIndexLegacy(lxcNetworkParseDataArray *networks,
     }
 
     /* Return last element added like a stack. */
-    return networks->parseData[ndata - 1];
+    if (networks->ndata > 0)
+        return networks->parseData[ndata - 1];
+
+    /* Not able to retrive an element */
+    return NULL;
 }
 
 
-- 
2.20.1


Re: [PATCH] lxc: Fix segfault when lxc.network does not start with 'type'

Posted by Michal Privoznik 2 weeks ago
On 2/6/20 3:12 AM, Julio Faracco wrote:
> To configure network settings using config file, legacy LXC settings
> require starting them with 'lxc.network.type' entry. If someone
> accidentally starts with 'lxc.network.name', libvirt will crash with
> segfault. This patch checks if this case is happening.
> 
> Sample invalid settings:
> lxc.network.link = eth0
> lxc.network.type = phys
> lxc.network.name = eth1
> lxc.network.ipv4 = 192.168.122.2/24
> lxc.network.ipv4.gateway = 192.168.122.1
> 
> Now, libvirt only see error without segmentation fault.
> 
> Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
> ---
>   src/lxc/lxc_native.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c
> index 59f3dd4fee..5462b74b85 100644
> --- a/src/lxc/lxc_native.c
> +++ b/src/lxc/lxc_native.c
> @@ -717,7 +717,11 @@ lxcNetworkGetParseDataByIndexLegacy(lxcNetworkParseDataArray *networks,
>       }
>   
>       /* Return last element added like a stack. */
> -    return networks->parseData[ndata - 1];
> +    if (networks->ndata > 0)
> +        return networks->parseData[ndata - 1];

This doesn't feel right. The same variable should be used in both lines. 
I mean either go with:

   if (ndata > 0)
     return networks->parseData[ndata - 1];

or:

   if (networks->ndata > 0)
     return networks->parseData[networks->ndata - 1];


The first one looks better to me. I can fix it before pushing, if you agree.

Michal

Re: [PATCH] lxc: Fix segfault when lxc.network does not start with 'type'

Posted by Julio Faracco 2 weeks ago
Hi Michal,

The first one looks cleaner.
You can go ahead.
Thanks again!

Em qui., 6 de fev. de 2020 às 05:51, Michal Privoznik
<mprivozn@redhat.com> escreveu:
>
> On 2/6/20 3:12 AM, Julio Faracco wrote:
> > To configure network settings using config file, legacy LXC settings
> > require starting them with 'lxc.network.type' entry. If someone
> > accidentally starts with 'lxc.network.name', libvirt will crash with
> > segfault. This patch checks if this case is happening.
> >
> > Sample invalid settings:
> > lxc.network.link = eth0
> > lxc.network.type = phys
> > lxc.network.name = eth1
> > lxc.network.ipv4 = 192.168.122.2/24
> > lxc.network.ipv4.gateway = 192.168.122.1
> >
> > Now, libvirt only see error without segmentation fault.
> >
> > Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
> > ---
> >   src/lxc/lxc_native.c | 6 +++++-
> >   1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c
> > index 59f3dd4fee..5462b74b85 100644
> > --- a/src/lxc/lxc_native.c
> > +++ b/src/lxc/lxc_native.c
> > @@ -717,7 +717,11 @@ lxcNetworkGetParseDataByIndexLegacy(lxcNetworkParseDataArray *networks,
> >       }
> >
> >       /* Return last element added like a stack. */
> > -    return networks->parseData[ndata - 1];
> > +    if (networks->ndata > 0)
> > +        return networks->parseData[ndata - 1];
>
> This doesn't feel right. The same variable should be used in both lines.
> I mean either go with:
>
>    if (ndata > 0)
>      return networks->parseData[ndata - 1];
>
> or:
>
>    if (networks->ndata > 0)
>      return networks->parseData[networks->ndata - 1];
>
>
> The first one looks better to me. I can fix it before pushing, if you agree.
>
> Michal
>