[libgpiod] bindings: python: fix segfault when calling Line.request()

[libgpiod] bindings: python: fix segfault when calling Line.request()

[Joel Savitz]

When Line.request() is called without the required 'consumer=value'
argument, the module attempts access an empty dictionary object
resulting in a segfault. This patch avoids such access when the
dictionary is empty and maintains the current design where the
LineBulk object is responsible for validation of arguments.

Signed-off-by: Joel Savitz <jsavitz@redhat.com>
---
 bindings/python/gpiodmodule.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/bindings/python/gpiodmodule.c b/bindings/python/gpiodmodule.c
index 2f6ef51..ae7e1cc 100644
--- a/bindings/python/gpiodmodule.c
+++ b/bindings/python/gpiodmodule.c
@@ -434,8 +434,12 @@ static PyObject *gpiod_Line_request(gpiod_LineObject *self,
 	gpiod_LineBulkObject *bulk_obj;
 	int rv;
 
-	def_val = PyDict_GetItemString(kwds, "default_val");
-	def_vals = PyDict_GetItemString(kwds, "default_vals");
+	if (PyDict_Size(kwds) > 0) {
+		def_val = PyDict_GetItemString(kwds, "default_val");
+		def_vals = PyDict_GetItemString(kwds, "default_vals");
+	} else {
+		def_val = def_vals = NULL;
+	}
 
 	if (def_val && def_vals) {
 		PyErr_SetString(PyExc_TypeError,
-- 
2.23.0

Re: [libgpiod] bindings: python: fix segfault when calling Line.request()

[Bartosz Golaszewski]

wt., 3 gru 2019 o 20:24 Joel Savitz <jsavitz@redhat.com> napisał(a):
>
> When Line.request() is called without the required 'consumer=value'
> argument, the module attempts access an empty dictionary object
> resulting in a segfault. This patch avoids such access when the
> dictionary is empty and maintains the current design where the
> LineBulk object is responsible for validation of arguments.
>
> Signed-off-by: Joel Savitz <jsavitz@redhat.com>
> ---
>  bindings/python/gpiodmodule.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/bindings/python/gpiodmodule.c b/bindings/python/gpiodmodule.c
> index 2f6ef51..ae7e1cc 100644
> --- a/bindings/python/gpiodmodule.c
> +++ b/bindings/python/gpiodmodule.c
> @@ -434,8 +434,12 @@ static PyObject *gpiod_Line_request(gpiod_LineObject *self,
>         gpiod_LineBulkObject *bulk_obj;
>         int rv;
>
> -       def_val = PyDict_GetItemString(kwds, "default_val");
> -       def_vals = PyDict_GetItemString(kwds, "default_vals");
> +       if (PyDict_Size(kwds) > 0) {
> +               def_val = PyDict_GetItemString(kwds, "default_val");
> +               def_vals = PyDict_GetItemString(kwds, "default_vals");
> +       } else {
> +               def_val = def_vals = NULL;
> +       }
>
>         if (def_val && def_vals) {
>                 PyErr_SetString(PyExc_TypeError,
> --
> 2.23.0
>

Applied to master and 1.4.x stable.

Thanks,
Bartosz

Re: [libgpiod] bindings: python: fix segfault when calling Line.request()

[Bartosz Golaszewski]

wt., 3 gru 2019 o 20:24 Joel Savitz <jsavitz@redhat.com> napisał(a):
>
> When Line.request() is called without the required 'consumer=value'
> argument, the module attempts access an empty dictionary object
> resulting in a segfault. This patch avoids such access when the
> dictionary is empty and maintains the current design where the
> LineBulk object is responsible for validation of arguments.
>
> Signed-off-by: Joel Savitz <jsavitz@redhat.com>
> ---
>  bindings/python/gpiodmodule.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/bindings/python/gpiodmodule.c b/bindings/python/gpiodmodule.c
> index 2f6ef51..ae7e1cc 100644
> --- a/bindings/python/gpiodmodule.c
> +++ b/bindings/python/gpiodmodule.c
> @@ -434,8 +434,12 @@ static PyObject *gpiod_Line_request(gpiod_LineObject *self,
>         gpiod_LineBulkObject *bulk_obj;
>         int rv;
>
> -       def_val = PyDict_GetItemString(kwds, "default_val");
> -       def_vals = PyDict_GetItemString(kwds, "default_vals");
> +       if (PyDict_Size(kwds) > 0) {
> +               def_val = PyDict_GetItemString(kwds, "default_val");
> +               def_vals = PyDict_GetItemString(kwds, "default_vals");
> +       } else {
> +               def_val = def_vals = NULL;
> +       }
>
>         if (def_val && def_vals) {
>                 PyErr_SetString(PyExc_TypeError,
> --
> 2.23.0
>

Thanks a lot for bringing this to my attention - I wasn't aware that
the kwds dictionary will be NULL if no named arguments were passed to
the function call. Let me know if you see any such bug anywhere else.
I've looked through the code but this seems to be the only place where
I ever access the dictionary directly.

Best regards,
Bartosz