Pino Toscano
2014-Aug-11 16:35 UTC
[Libguestfs] [PATCH] python: fix possible free on uninit memory with OStringList optargs
When using optional arguments of type OStringList, the code free'ing the member in the optargs_s struct corresponding to that optional argument would just check for a non-PyNone PyObject for that argument. If before that optional argument there are other arguments which can cause an earlier error return from that binding function, the free'ing code will then act on garbage values. Enhance the check by also checking whether the optargs struct has the bitmask with the element for that argument, meaning that the corresponding struct member was initialized. --- generator/python.ml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/generator/python.ml b/generator/python.ml index 72bc8a0..a763104 100644 --- a/generator/python.ml +++ b/generator/python.ml @@ -511,7 +511,9 @@ put_table (char * const * const argv) function | OBool _ | OInt _ | OInt64 _ | OString _ -> () | OStringList n -> - pr " if (py_%s != Py_None)\n" n; + let uc_n = String.uppercase n in + pr " if (py_%s != Py_None && (optargs_s.bitmask |= %s_%s_BITMASK) != 0)\n" + n c_optarg_prefix uc_n; pr " free ((char **) optargs_s.%s);\n" n ) optargs; -- 1.9.3
Pino Toscano
2014-Aug-12 08:18 UTC
Re: [Libguestfs] [PATCH] python: fix possible free on uninit memory with OStringList optargs
On Monday 11 August 2014 18:35:51 Pino Toscano wrote:> When using optional arguments of type OStringList, the code free'ing > the member in the optargs_s struct corresponding to that optional > argument would just check for a non-PyNone PyObject for that argument. > If before that optional argument there are other arguments which can > cause an earlier error return from that binding function, the > free'ing code will then act on garbage values. > > Enhance the check by also checking whether the optargs struct has the > bitmask with the element for that argument, meaning that the > corresponding struct member was initialized. > --- > generator/python.ml | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/generator/python.ml b/generator/python.ml > index 72bc8a0..a763104 100644 > --- a/generator/python.ml > +++ b/generator/python.ml > @@ -511,7 +511,9 @@ put_table (char * const * const argv) > function > > | OBool _ | OInt _ | OInt64 _ | OString _ -> () > | OStringList n -> > > - pr " if (py_%s != Py_None)\n" n; > + let uc_n = String.uppercase n in > + pr " if (py_%s != Py_None && (optargs_s.bitmask |= %s_%s_BITMASK) != 0)\n" > + n c_optarg_prefix uc_n; > pr " free ((char **) optargs_s.%s);\n" n > ) optargs;Apologies for the wrong patch, I just realized I apparently committed and send an old/wrong version. I'll follow-up with a correct patch. -- Pino Toscano
Pino Toscano
2014-Aug-12 08:30 UTC
[Libguestfs] [PATCH] python: fix possible free on uninit memory with OStringList optargs
When using optional arguments of type OStringList, the code free'ing the member in the optargs_s struct corresponding to that optional argument would just check for a non-PyNone PyObject for that argument. If before that optional argument there are other arguments which can cause an earlier error return from that binding function, the free'ing code will then act on garbage values. Enhance the check by also checking whether the optargs struct has the bitmask with the element for that argument, meaning that the corresponding struct member was initialized. --- generator/python.ml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/generator/python.ml b/generator/python.ml index 72bc8a0..07e87d2 100644 --- a/generator/python.ml +++ b/generator/python.ml @@ -511,7 +511,9 @@ put_table (char * const * const argv) function | OBool _ | OInt _ | OInt64 _ | OString _ -> () | OStringList n -> - pr " if (py_%s != Py_None)\n" n; + let uc_n = String.uppercase n in + pr " if (py_%s != Py_None && (optargs_s.bitmask & %s_%s_BITMASK) != 0)\n" + n c_optarg_prefix uc_n; pr " free ((char **) optargs_s.%s);\n" n ) optargs; -- 1.9.3
Richard W.M. Jones
2014-Aug-12 09:54 UTC
Re: [Libguestfs] [PATCH] python: fix possible free on uninit memory with OStringList optargs
On Tue, Aug 12, 2014 at 10:30:02AM +0200, Pino Toscano wrote:> When using optional arguments of type OStringList, the code free'ing > the member in the optargs_s struct corresponding to that optional > argument would just check for a non-PyNone PyObject for that argument. > If before that optional argument there are other arguments which can > cause an earlier error return from that binding function, the free'ing > code will then act on garbage values. > > Enhance the check by also checking whether the optargs struct has the > bitmask with the element for that argument, meaning that the > corresponding struct member was initialized. > --- > generator/python.ml | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/generator/python.ml b/generator/python.ml > index 72bc8a0..07e87d2 100644 > --- a/generator/python.ml > +++ b/generator/python.ml > @@ -511,7 +511,9 @@ put_table (char * const * const argv) > function > | OBool _ | OInt _ | OInt64 _ | OString _ -> () > | OStringList n -> > - pr " if (py_%s != Py_None)\n" n; > + let uc_n = String.uppercase n in > + pr " if (py_%s != Py_None && (optargs_s.bitmask & %s_%s_BITMASK) != 0)\n" > + n c_optarg_prefix uc_n; > pr " free ((char **) optargs_s.%s);\n" n > ) optargs; >ACK. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://people.redhat.com/~rjones/virt-df/
Maybe Matching Threads
- [PATCH 3/3] python: Allow bindings to be compiled with different version of libguestfs (RHBZ#1262983).
- [PATCH 0/7] Add tar compress, numericowner, excludes flags.
- [PATCH v2] python: add simple wrappers for PyObject<->string functions
- [PATCH 4/4] php: fix memory leak in OStringList optargs
- [PATCH 1/4] php: fix invalid memory access with OptString