diff mbox series

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

Message ID 20191203192305.31722-1-jsavitz@redhat.com
State New
Headers show
Series [libgpiod] bindings: python: fix segfault when calling Line.request() | expand

Commit Message

Joel Savitz Dec. 3, 2019, 7:23 p.m. UTC
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(-)

Comments

Bartosz Golaszewski Dec. 11, 2019, 3:11 p.m. UTC | #1
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
Bartosz Golaszewski Dec. 12, 2019, 7:56 a.m. UTC | #2
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
diff mbox series

Patch

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,