mailarchive of the pengutronix oss-tools mailing list
 help / color / mirror / Atom feed
* [OSS-Tools] [PATCH 1/5] configure: pass -fno-strict-aliasing to GCC
@ 2023-05-31 15:10 Ahmad Fatoum
  2023-05-31 15:10 ` [OSS-Tools] [PATCH 2/5] libdt: fix issues of external function without prototype Ahmad Fatoum
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Ahmad Fatoum @ 2023-05-31 15:10 UTC (permalink / raw)
  To: oss-tools

Like the Linux kernel, barebox is built with -fno-strict-aliasing, and
code is written to take advantage of that. As dt-utils imports code from
barebox, we may import code that assumes aliasing to be non-strict, so
we better compile it with the same rules.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 configure.ac | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index 00c80105e467..be8967eb0809 100644
--- a/configure.ac
+++ b/configure.ac
@@ -37,7 +37,8 @@ my_CFLAGS="-Wall \
 -Wmissing-declarations -Wmissing-prototypes \
 -Wnested-externs -Wsign-compare -Wchar-subscripts \
 -Wstrict-prototypes -Wshadow \
--Wformat-security -Wtype-limits"
+-Wformat-security -Wtype-limits \
+-fno-strict-aliasing"
 AC_SUBST([my_CFLAGS])
 
 PKG_CHECK_MODULES(UDEV, [libudev])
-- 
2.39.2




^ permalink raw reply	[flat|nested] 6+ messages in thread

* [OSS-Tools] [PATCH 2/5] libdt: fix issues of external function without prototype
  2023-05-31 15:10 [OSS-Tools] [PATCH 1/5] configure: pass -fno-strict-aliasing to GCC Ahmad Fatoum
@ 2023-05-31 15:10 ` Ahmad Fatoum
  2023-05-31 15:10 ` [OSS-Tools] [PATCH 3/5] libdt: use memcpy instead of strncpy Ahmad Fatoum
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Ahmad Fatoum @ 2023-05-31 15:10 UTC (permalink / raw)
  To: oss-tools

When increasing warning level, we see that of_find_device_by_node_path
lacks a prototype despite having external linkage and being exported
in the symbols file. On the other hand, scan_proc_dir has external
linkage, but is only ever needed internally, so let's give it internal
linkage.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 src/crc32.c | 1 +
 src/dt/dt.h | 1 +
 src/libdt.c | 2 +-
 3 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/crc32.c b/src/crc32.c
index 8d4dddcf6129..8273d3465f6f 100644
--- a/src/crc32.c
+++ b/src/crc32.c
@@ -8,6 +8,7 @@
  * For conditions of distribution and use, see copyright notice in zlib.h
  */
 
+#include <dt/common.h>
 #include <stdint.h>
 
 /* ========================================================================
diff --git a/src/dt/dt.h b/src/dt/dt.h
index 4ae24ba8bf7a..6ce95d91da87 100644
--- a/src/dt/dt.h
+++ b/src/dt/dt.h
@@ -121,6 +121,7 @@ extern struct device_node *of_find_node_by_type(struct device_node *from,
 	const char *type);
 extern struct device_node *of_find_compatible_node(struct device_node *from,
 	const char *type, const char *compat);
+extern struct udev_device *of_find_device_by_node_path(const char *of_full_path);
 extern const struct of_device_id *of_match_node(
 	const struct of_device_id *matches, const struct device_node *node);
 extern struct device_node *of_find_matching_node_and_match(
diff --git a/src/libdt.c b/src/libdt.c
index 59e76d336d8d..a833c582dfbf 100644
--- a/src/libdt.c
+++ b/src/libdt.c
@@ -1938,7 +1938,7 @@ int of_device_disable_path(const char *path)
 	return of_device_disable(node);
 }
 
-int scan_proc_dir(struct device_node *node, const char *path)
+static int scan_proc_dir(struct device_node *node, const char *path)
 {
 	DIR *dir;
 	struct dirent *dirent;
-- 
2.39.2




^ permalink raw reply	[flat|nested] 6+ messages in thread

* [OSS-Tools] [PATCH 3/5] libdt: use memcpy instead of strncpy
  2023-05-31 15:10 [OSS-Tools] [PATCH 1/5] configure: pass -fno-strict-aliasing to GCC Ahmad Fatoum
  2023-05-31 15:10 ` [OSS-Tools] [PATCH 2/5] libdt: fix issues of external function without prototype Ahmad Fatoum
@ 2023-05-31 15:10 ` Ahmad Fatoum
  2023-05-31 15:10 ` [OSS-Tools] [PATCH 4/5] libdt: don't use old-style function definition Ahmad Fatoum
  2023-05-31 15:10 ` [OSS-Tools] [PATCH 5/5] barebox-state: fix use after free in error path Ahmad Fatoum
  3 siblings, 0 replies; 6+ messages in thread
From: Ahmad Fatoum @ 2023-05-31 15:10 UTC (permalink / raw)
  To: oss-tools

Despite the name, GCC objects at the strncpy use in safe_strncpy on
safety grounds.  While that seems to be a false positive, we could
just be using memcpy instead and side step this altogether.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 src/dt/common.h | 34 ++++++++++++++--------------------
 1 file changed, 14 insertions(+), 20 deletions(-)

diff --git a/src/dt/common.h b/src/dt/common.h
index c3c4f53fc216..69a264cfc1a9 100644
--- a/src/dt/common.h
+++ b/src/dt/common.h
@@ -36,6 +36,12 @@ typedef uint64_t u64;
 #undef offsetof
 #define offsetof(TYPE, MEMBER) ((size_t) &((TYPE *)0)->MEMBER)
 
+#define min(x, y) ({				\
+	typeof(x) _min1 = (x);			\
+	typeof(y) _min2 = (y);			\
+	(void) (&_min1 == &_min2);		\
+	_min1 < _min2 ? _min1 : _min2; })
+
 struct device_d;
 
 void pr_level_set(int level);
@@ -199,14 +205,6 @@ static inline size_t DT_strlcpy(char *dest, const char *src, size_t size)
 	return ret;
 }
 
-/* Like strncpy but make sure the resulting string is always 0 terminated. */
-static inline char * safe_strncpy(char *dst, const char *src, size_t size)
-{
-	if (!size) return dst;
-	dst[--size] = '\0';
-	return strncpy(dst, src, size);
-}
-
 static inline char *xstrdup(const char *s)
 {
 	char *p = strdup(s);
@@ -415,21 +413,23 @@ static inline int dev_set_name(struct device_d *dev, const char *fmt, ...)
 {
 	char newname[MAX_DRIVER_NAME];
 	va_list vargs;
-	int err;
+	int ret;
 
 	va_start(vargs, fmt);
-	err = vsnprintf(newname, MAX_DRIVER_NAME, fmt, vargs);
+	ret = vsnprintf(newname, MAX_DRIVER_NAME, fmt, vargs);
 	va_end(vargs);
 
+	if (WARN_ON(ret < 0))
+		return ret;
+
 	/*
 	 * Copy new name into dev structure, we do this after vsnprintf call in
 	 * case old device name was in one of vargs
 	 */
-	safe_strncpy(dev->name, newname, MAX_DRIVER_NAME);
+	memcpy(dev->name, newname, min(MAX_DRIVER_NAME - 1, ret));
+	dev->name[MAX_DRIVER_NAME - 1] = '\0';
 
-	WARN_ON(err < 0);
-
-	return err;
+	return 0;
 }
 
 struct driver_d;
@@ -577,12 +577,6 @@ static inline __u32 ror32(__u32 word, unsigned int shift)
 	return (word >> shift) | (word << (32 - shift));
 }
 
-#define min(x, y) ({				\
-	typeof(x) _min1 = (x);			\
-	typeof(y) _min2 = (y);			\
-	(void) (&_min1 == &_min2);		\
-	_min1 < _min2 ? _min1 : _min2; })
-
 /*
  * Helper macros to use CONFIG_ options in C expressions. Note that
  * these only work with boolean and tristate options.
-- 
2.39.2




^ permalink raw reply	[flat|nested] 6+ messages in thread

* [OSS-Tools] [PATCH 4/5] libdt: don't use old-style function definition
  2023-05-31 15:10 [OSS-Tools] [PATCH 1/5] configure: pass -fno-strict-aliasing to GCC Ahmad Fatoum
  2023-05-31 15:10 ` [OSS-Tools] [PATCH 2/5] libdt: fix issues of external function without prototype Ahmad Fatoum
  2023-05-31 15:10 ` [OSS-Tools] [PATCH 3/5] libdt: use memcpy instead of strncpy Ahmad Fatoum
@ 2023-05-31 15:10 ` Ahmad Fatoum
  2023-05-31 15:10 ` [OSS-Tools] [PATCH 5/5] barebox-state: fix use after free in error path Ahmad Fatoum
  3 siblings, 0 replies; 6+ messages in thread
From: Ahmad Fatoum @ 2023-05-31 15:10 UTC (permalink / raw)
  To: oss-tools

Increasing the warning level has GCC warn us, so let's heed the warning.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 src/dt/common.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/dt/common.h b/src/dt/common.h
index 69a264cfc1a9..62776c0bb027 100644
--- a/src/dt/common.h
+++ b/src/dt/common.h
@@ -460,7 +460,7 @@ static inline int of_unregister_fixup(int (*fixup)(struct device_node *, void *)
 }
 
 #define __define_initcall(level,fn,id) \
-static void __attribute__ ((constructor)) __initcall_##id##_##fn() { \
+static void __attribute__ ((constructor)) __initcall_##id##_##fn(void) { \
 	fn(); \
 }
 
-- 
2.39.2




^ permalink raw reply	[flat|nested] 6+ messages in thread

* [OSS-Tools] [PATCH 5/5] barebox-state: fix use after free in error path
  2023-05-31 15:10 [OSS-Tools] [PATCH 1/5] configure: pass -fno-strict-aliasing to GCC Ahmad Fatoum
                   ` (2 preceding siblings ...)
  2023-05-31 15:10 ` [OSS-Tools] [PATCH 4/5] libdt: don't use old-style function definition Ahmad Fatoum
@ 2023-05-31 15:10 ` Ahmad Fatoum
  2023-06-02 12:45   ` Roland Hieber
  3 siblings, 1 reply; 6+ messages in thread
From: Ahmad Fatoum @ 2023-05-31 15:10 UTC (permalink / raw)
  To: oss-tools

blob_bin is freed a few lines above unconditionally, so freeing it
again in the error path will cause a double free.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 src/keystore-blob.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/src/keystore-blob.c b/src/keystore-blob.c
index ed6ecb4eaa25..8ec07f0a3d56 100644
--- a/src/keystore-blob.c
+++ b/src/keystore-blob.c
@@ -81,10 +81,8 @@ int keystore_get_secret(const char *name, const unsigned char **key, int *key_le
 
 	/* payload */
 	fd = open(blob_gen_payload, O_RDONLY);
-	if (fd < 0) {
-		free(blob_bin);
+	if (fd < 0)
 		return -errno;
-	}
 
 	payload = xzalloc(len);
 	len = read(fd, payload, len);
-- 
2.39.2




^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [OSS-Tools] [PATCH 5/5] barebox-state: fix use after free in error path
  2023-05-31 15:10 ` [OSS-Tools] [PATCH 5/5] barebox-state: fix use after free in error path Ahmad Fatoum
@ 2023-06-02 12:45   ` Roland Hieber
  0 siblings, 0 replies; 6+ messages in thread
From: Roland Hieber @ 2023-06-02 12:45 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: oss-tools

For the whole series:

Reviewed-by: Roland Hieber <rhi@pengutronix.de>

On Wed, May 31, 2023 at 05:10:15PM +0200, Ahmad Fatoum wrote:
> blob_bin is freed a few lines above unconditionally, so freeing it
> again in the error path will cause a double free.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  src/keystore-blob.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/src/keystore-blob.c b/src/keystore-blob.c
> index ed6ecb4eaa25..8ec07f0a3d56 100644
> --- a/src/keystore-blob.c
> +++ b/src/keystore-blob.c
> @@ -81,10 +81,8 @@ int keystore_get_secret(const char *name, const unsigned char **key, int *key_le
>  
>  	/* payload */
>  	fd = open(blob_gen_payload, O_RDONLY);
> -	if (fd < 0) {
> -		free(blob_bin);
> +	if (fd < 0)
>  		return -errno;
> -	}
>  
>  	payload = xzalloc(len);
>  	len = read(fd, payload, len);
> -- 
> 2.39.2
> 
> 
> 

-- 
Roland Hieber, Pengutronix e.K.          | r.hieber@pengutronix.de     |
Steuerwalder Str. 21                     | https://www.pengutronix.de/ |
31137 Hildesheim, Germany                | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686         | Fax:   +49-5121-206917-5555 |



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-06-02 12:45 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-31 15:10 [OSS-Tools] [PATCH 1/5] configure: pass -fno-strict-aliasing to GCC Ahmad Fatoum
2023-05-31 15:10 ` [OSS-Tools] [PATCH 2/5] libdt: fix issues of external function without prototype Ahmad Fatoum
2023-05-31 15:10 ` [OSS-Tools] [PATCH 3/5] libdt: use memcpy instead of strncpy Ahmad Fatoum
2023-05-31 15:10 ` [OSS-Tools] [PATCH 4/5] libdt: don't use old-style function definition Ahmad Fatoum
2023-05-31 15:10 ` [OSS-Tools] [PATCH 5/5] barebox-state: fix use after free in error path Ahmad Fatoum
2023-06-02 12:45   ` Roland Hieber

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox