Xen devel - Dec 2013 - [PATCH] libxenctrl: Fix xc_interface_close() crash if it gets NULL as an argument

xc_interface_close() crashes if it gets NULL as an argument. However,
it just calls xc_interface_close_common() which is called by many
others functions. It means that they are also vulnerable. So fix above
mentioned issue by adding NULL check in xc_interface_close_common().
This way we fix similar issue in other functions which calls
xc_interface_close_common() too.

Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
---
 tools/libxc/xc_private.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/libxc/xc_private.c b/tools/libxc/xc_private.c
index 524862e..838fd21 100644
--- a/tools/libxc/xc_private.c
+++ b/tools/libxc/xc_private.c
@@ -198,6 +198,9 @@ static int xc_interface_close_common(xc_interface *xch)
 {
     int rc = 0;
 
+    if (!xch)
+	return 0;
+
     xc__hypercall_buffer_cache_release(xch);
 
     xtl_logger_destroy(xch->dombuild_logger_tofree);
-- 
1.7.10.4

On Mon, 2013-12-02 at 20:13 +0100, Daniel Kiper wrote:
> xc_interface_close() crashes if it gets NULL as an argument. However,
> it just calls xc_interface_close_common() which is called by many
> others functions. It means that they are also vulnerable. So fix above
> mentioned issue by adding NULL check in xc_interface_close_common().
> This way we fix similar issue in other functions which calls
> xc_interface_close_common() too.
> 
> Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
I suppose this makes sense in the same way that free(NULL) does:
        Acked-by: Ian Campbell <ian.campbell@citrix.com>

Did you observe this with a particular caller?

On Tue, Dec 03, 2013 at 09:47:35AM +0000, Ian Campbell
wrote:
> On Mon, 2013-12-02 at 20:13 +0100, Daniel Kiper wrote:
> > xc_interface_close() crashes if it gets NULL as an argument. However,
> > it just calls xc_interface_close_common() which is called by many
> > others functions. It means that they are also vulnerable. So fix above
> > mentioned issue by adding NULL check in xc_interface_close_common().
> > This way we fix similar issue in other functions which calls
> > xc_interface_close_common() too.
> >
> > Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
>
> I suppose this makes sense in the same way that free(NULL) does:
>         Acked-by: Ian Campbell <ian.campbell@citrix.com>
Thanks.
> Did you observe this with a particular caller?
Yes, I found this issue by chance in kaxec-tools. It is fixed
right now but I agree that this type of functions should
behave with NULL like free(NULL).

Daniel

On Tue, 2013-12-03 at 12:58 +0100, Daniel Kiper wrote:
> On Tue, Dec 03, 2013 at 09:47:35AM +0000, Ian Campbell wrote:
> > On Mon, 2013-12-02 at 20:13 +0100, Daniel Kiper wrote:
> > > xc_interface_close() crashes if it gets NULL as an argument.
However,
> > > it just calls xc_interface_close_common() which is called by many
> > > others functions. It means that they are also vulnerable. So fix
above
> > > mentioned issue by adding NULL check in
xc_interface_close_common().
> > > This way we fix similar issue in other functions which calls
> > > xc_interface_close_common() too.
> > >
> > > Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
> >
> > I suppose this makes sense in the same way that free(NULL) does:
> >         Acked-by: Ian Campbell <ian.campbell@citrix.com>
> 
> Thanks.
And now applied.
> > Did you observe this with a particular caller?
> 
> Yes, I found this issue by chance in kaxec-tools. It is fixed
Great, thanks.
> right now but I agree that this type of functions should
> behave with NULL like free(NULL).
> 
> Daniel