Skip to contentSkip to conversation

Fix issues detected by static analyzer

PATCH Applied Iker Pedrosa 2 participants 6 messages Jul 29, 2024 – Jul 31, 2024

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

v2 cover Fix issues detected by static analyzer

This patch series contain a set of fixes for several issues detected by a static analyzer tool. They are related to wrong pointers management and string termination.

v2 includes a number of fixes to issues highlighted by Kent Gibson <[email protected]>.

Iker Pedrosa (4):
bindings: python: gpiod: avoid use after free
lib: line-info strings termination
lib: chip-info strings termination
tools: free to avoid leak

 bindings/python/gpiod/ext/chip.c | 6 ++++--
 lib/chip-info.c                  | 4 ++--
 lib/line-info.c                  | 4 ++--
 tools/gpioinfo.c                 | 4 +++-
 4 files changed, 11 insertions(+), 7 deletions(-)
Signature · 1 line
2.45.2
#1 lore raw

PATCH v2 1/4 bindings: python: gpiod: avoid use after free

req_cfg variable is freed and then used, which would generate an error. Avoid this problem by freeing when the variable will no longer be used.

---
 bindings/python/gpiod/ext/chip.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
bindings/python/gpiod/ext/chip.c +4 -2
index 28cf504..e8eaad8 100644
@@ -274,14 +274,16 @@ static PyObject *chip_request_lines(chip_object *self, PyObject *args)
 	Py_BEGIN_ALLOW_THREADS;
 	request = gpiod_chip_request_lines(self->chip, req_cfg, line_cfg);
 	Py_END_ALLOW_THREADS;
-	gpiod_request_config_free(req_cfg);
-	if (!request)
+	if (!request) {
+		gpiod_request_config_free(req_cfg);
 		return Py_gpiod_SetErrFromErrno();
+	}
 
 	req_obj = Py_gpiod_MakeRequestObject(request,
 			gpiod_request_config_get_event_buffer_size(req_cfg));
 	if (!req_obj)
 		gpiod_line_request_release(request);
+	gpiod_request_config_free(req_cfg);
 
 	return req_obj;
 }
Signature · 1 line
2.45.2
#2 lore raw

PATCH v2 2/4 lib: line-info strings termination

In reply to Iker Pedrosa

strncpy() truncates the destination buffer if it isn't large enough to hold the copy. Thus, let's increase the size of the destination strings to add the NULL character at the end.

---
 lib/line-info.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
lib/line-info.c +2 -2
index 9f53b04..a7c6241 100644
@@ -10,9 +10,9 @@
 
 struct gpiod_line_info {
 	unsigned int offset;
-	char name[GPIO_MAX_NAME_SIZE];
+	char name[GPIO_MAX_NAME_SIZE+1];
 	bool used;
-	char consumer[GPIO_MAX_NAME_SIZE];
+	char consumer[GPIO_MAX_NAME_SIZE+1];
 	enum gpiod_line_direction direction;
 	bool active_low;
 	enum gpiod_line_bias bias;
Signature · 1 line
2.45.2
#3 lore raw

PATCH v2 3/4 lib: chip-info strings termination

In reply to Iker Pedrosa

strncpy() truncates the destination buffer if it isn't large enough to hold the copy. Thus, let's increase the size of the destination strings to add the NULL character at the end.

---
 lib/chip-info.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
lib/chip-info.c +2 -2
index 87fd9e7..478cd62 100644
@@ -10,8 +10,8 @@
 
 struct gpiod_chip_info {
 	size_t num_lines;
-	char name[32];
-	char label[32];
+	char name[GPIO_MAX_NAME_SIZE+1];
+	char label[GPIO_MAX_NAME_SIZE+1];
 };
 
 GPIOD_API void gpiod_chip_info_free(struct gpiod_chip_info *info)
Signature · 1 line
2.45.2
#4 lore raw

PATCH v2 4/4 tools: free to avoid leak

In reply to Iker Pedrosa

info variable is allocated, but never freed when the loop continues. Free it so that it isn't leaked.

---
 tools/gpioinfo.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
tools/gpioinfo.c +3 -1
index 44d1c8c..d5e4751 100644
@@ -195,8 +195,10 @@ static void list_lines(struct line_resolver *resolver, struct gpiod_chip *chip,
 				   offset, gpiod_chip_info_get_name(chip_info));
 
 		if (resolver->num_lines &&
-		    !resolve_line(resolver, info, chip_num))
+		    !resolve_line(resolver, info, chip_num)) {
+			gpiod_line_info_free(info);
 			continue;
+		}
 
 		if (resolver->num_lines) {
 			printf("%s %u", gpiod_chip_info_get_name(chip_info),
Signature · 1 line
2.45.2
#5 lore raw
In reply to Iker Pedrosa

From: Bartosz Golaszewski <[email protected]>

Quoted 8 lines · Iker Pedrosa

This patch series contain a set of fixes for several issues detected by a static analyzer tool. They are related to wrong pointers management and string termination.

v2 includes a number of fixes to issues highlighted by Kent Gibson <[email protected]>.

[...]

Applied, thanks!

Please use imperative mode next time in commit messages. I tweaked them myself this time.

[1/4] bindings: python: gpiod: avoid use after free

      commit: c497e29ca1f88963c525351e60af23ed896a2b8c
[2/4] lib: line-info strings termination
      commit: 5533f277aa28b36f8ed4bbfac8e22e011ed68858
[3/4] lib: chip-info strings termination
      commit: d71f5ad24663624a17d335462908ae5a8f69bcf7
[4/4] tools: free to avoid leak
      commit: e7b40978801a6148cee75b10b9b775eba1b70a64

Best regards,

Signature · 1 line
Bartosz Golaszewski <[email protected]>