[PATCH 0/2] stdio: fix stdio_deregister_dev()
When setting or copying the name field of a stdio device we must ensure that the target field is NUL terminated.
Avoid truncation in stdio_deregister_dev().
Heinrich Schuchardt (2): dm: serial: fix serial_post_probe() stdio: fix stdio_deregister_dev()
common/stdio.c | 6 +++--- drivers/serial/serial-uclass.c | 2 +- include/stdio_dev.h | 3 ++- 3 files changed, 6 insertions(+), 5 deletions(-)
The size of the name of a udevice is not limited.
When setting the fixed sized name field of a stdio device we must ensure that the target string is NUL terminated to avoid buffer overflows.
Fixes: 57d92753d4ca ("dm: Add a uclass for serial devices") Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com --- drivers/serial/serial-uclass.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c index 5e2e7dfbcb..4a2da7a331 100644 --- a/drivers/serial/serial-uclass.c +++ b/drivers/serial/serial-uclass.c @@ -520,7 +520,7 @@ static int serial_post_probe(struct udevice *dev) return 0; memset(&sdev, '\0', sizeof(sdev));
- strncpy(sdev.name, dev->name, sizeof(sdev.name)); + strlcpy(sdev.name, dev->name, sizeof(sdev.name)); sdev.flags = DEV_FLAGS_OUTPUT | DEV_FLAGS_INPUT | DEV_FLAGS_DM; sdev.priv = dev; sdev.putc = serial_stub_putc;
On Thu, 28 Sept 2023 at 18:47, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
The size of the name of a udevice is not limited.
When setting the fixed sized name field of a stdio device we must ensure that the target string is NUL terminated to avoid buffer overflows.
Fixes: 57d92753d4ca ("dm: Add a uclass for serial devices") Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
drivers/serial/serial-uclass.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Simon Glass sjg@chromium.org
On Fri, Sep 29, 2023 at 02:47:16AM +0200, Heinrich Schuchardt wrote:
The size of the name of a udevice is not limited.
When setting the fixed sized name field of a stdio device we must ensure that the target string is NUL terminated to avoid buffer overflows.
Fixes: 57d92753d4ca ("dm: Add a uclass for serial devices") Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!
When copying the name of a stdio device we must ensure that it is NUL terminated before passing it to strcmp() to avoid a buffer overrun.
Truncating the name field leads to failure to deregister a stdio device. When copying we must ensure that the name field sizes match.
Addresses-Coverity-ID: 350462 String not null terminated Fixes: 5294e97832a6 ("stdio: extend "name" to 32 symbols") Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com --- common/stdio.c | 6 +++--- include/stdio_dev.h | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/common/stdio.c b/common/stdio.c index 010bf576af..e3354f092d 100644 --- a/common/stdio.c +++ b/common/stdio.c @@ -259,7 +259,7 @@ int stdio_register(struct stdio_dev *dev) int stdio_deregister_dev(struct stdio_dev *dev, int force) { struct list_head *pos; - char temp_names[3][16]; + char temp_names[3][STDIO_NAME_LEN]; int i;
/* get stdio devices (ListRemoveItem changes the dev list) */ @@ -272,8 +272,8 @@ int stdio_deregister_dev(struct stdio_dev *dev, int force) /* Device is assigned -> report error */ return -EBUSY; } - memcpy(&temp_names[i][0], stdio_devices[i]->name, - sizeof(temp_names[i])); + strlcpy(&temp_names[i][0], stdio_devices[i]->name, + sizeof(temp_names[i])); }
list_del(&dev->list); diff --git a/include/stdio_dev.h b/include/stdio_dev.h index 7f18102052..4e3c4708f8 100644 --- a/include/stdio_dev.h +++ b/include/stdio_dev.h @@ -17,6 +17,7 @@ #define DEV_FLAGS_INPUT 0x00000001 /* Device can be used as input console */ #define DEV_FLAGS_OUTPUT 0x00000002 /* Device can be used as output console */ #define DEV_FLAGS_DM 0x00000004 /* Device priv is a struct udevice * */ +#define STDIO_NAME_LEN 32
int stdio_file_to_flags(const int file);
@@ -24,7 +25,7 @@ int stdio_file_to_flags(const int file); struct stdio_dev { int flags; /* Device flags: input/output/system */ int ext; /* Supported extensions */ - char name[32]; /* Device name */ + char name[STDIO_NAME_LEN]; /* Device name */
/* GENERAL functions */
On Thu, 28 Sept 2023 at 18:47, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
When copying the name of a stdio device we must ensure that it is NUL terminated before passing it to strcmp() to avoid a buffer overrun.
Truncating the name field leads to failure to deregister a stdio device. When copying we must ensure that the name field sizes match.
Addresses-Coverity-ID: 350462 String not null terminated Fixes: 5294e97832a6 ("stdio: extend "name" to 32 symbols") Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
common/stdio.c | 6 +++--- include/stdio_dev.h | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
On Fri, Sep 29, 2023 at 02:47:17AM +0200, Heinrich Schuchardt wrote:
When copying the name of a stdio device we must ensure that it is NUL terminated before passing it to strcmp() to avoid a buffer overrun.
Truncating the name field leads to failure to deregister a stdio device. When copying we must ensure that the name field sizes match.
Addresses-Coverity-ID: 350462 String not null terminated Fixes: 5294e97832a6 ("stdio: extend "name" to 32 symbols") Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!
participants (3)
-
Heinrich Schuchardt -
Simon Glass -
Tom Rini