* [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