[U-Boot] [PATCH] cmd_mtdparts.c: prevent printbuffer overflows
The length of configured MTDPARTS_DEFAULT string could be greather than console printbuffer size. Check the lenght of the string before printing to prevent U-Boot crashes.
Signed-off-by: Anatolij Gustschin agust@denx.de --- common/cmd_mtdparts.c | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/common/cmd_mtdparts.c b/common/cmd_mtdparts.c index 230e96e..d4cb194 100644 --- a/common/cmd_mtdparts.c +++ b/common/cmd_mtdparts.c @@ -1254,6 +1254,14 @@ static void list_partitions(void) printf("\ndefaults:\n"); printf("mtdids : %s\n", mtdids_default ? mtdids_default : "none"); + + /* Check to prevent printbuffer overflows */ + if (mtdparts_default && strlen(mtdparts_default) > CONFIG_SYS_PBSIZE) { + puts("Warning: mtdparts too long," + " please increase CONFIG_SYS_PBSIZE\n"); + return; + } + printf("mtdparts: %s\n", mtdparts_default ? mtdparts_default : "none"); }
Hi Anatolij,
The length of configured MTDPARTS_DEFAULT string could be greather than console printbuffer size. Check the lenght of the string before printing to prevent U-Boot crashes.
Signed-off-by: Anatolij Gustschin agust@denx.de
common/cmd_mtdparts.c | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/common/cmd_mtdparts.c b/common/cmd_mtdparts.c index 230e96e..d4cb194 100644 --- a/common/cmd_mtdparts.c +++ b/common/cmd_mtdparts.c @@ -1254,6 +1254,14 @@ static void list_partitions(void) printf("\ndefaults:\n"); printf("mtdids : %s\n", mtdids_default ? mtdids_default : "none");
- /* Check to prevent printbuffer overflows */
- if (mtdparts_default && strlen(mtdparts_default) > CONFIG_SYS_PBSIZE) {
puts("Warning: mtdparts too long,"" please increase CONFIG_SYS_PBSIZE\n");return;- }
- printf("mtdparts: %s\n", mtdparts_default ? mtdparts_default : "none");
}
If I understand this correctly, then the real problem is the console code crashing without a warning, correct? If so, then please put such a warning in the correct place instead of fixing the caller sites.
Thanks Detlev
Dear Detlev Zundel,
In message m2ljekyyd5.fsf@ohwell.denx.de you wrote:
printf("mtdparts: %s\n", mtdparts_default ? mtdparts_default : "none"); }
If I understand this correctly, then the real problem is the console code crashing without a warning, correct? If so, then please put such a
True.
warning in the correct place instead of fixing the caller sites.
That's not exactly trivial, if you have a look at the code in "common/console.c" - the printf code has no way to know the size of the provided buffer.
OTOH it might make sense to use the (small, preconfigured) buffer only before relocation, and then, when devices become available, switch to a reasonably sized malloc()ed buffer - say 4 kB? But then - this just shifts the limits...
Best regards,
Wolfgang Denk
Dear Anatolij Gustschin,
In message 1266924140-14457-1-git-send-email-agust@denx.de you wrote:
The length of configured MTDPARTS_DEFAULT string could be greather than console printbuffer size. Check the lenght of the string before printing to prevent U-Boot crashes.
Signed-off-by: Anatolij Gustschin agust@denx.de
common/cmd_mtdparts.c | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/common/cmd_mtdparts.c b/common/cmd_mtdparts.c index 230e96e..d4cb194 100644 --- a/common/cmd_mtdparts.c +++ b/common/cmd_mtdparts.c @@ -1254,6 +1254,14 @@ static void list_partitions(void) printf("\ndefaults:\n"); printf("mtdids : %s\n", mtdids_default ? mtdids_default : "none");
- /* Check to prevent printbuffer overflows */
- if (mtdparts_default && strlen(mtdparts_default) > CONFIG_SYS_PBSIZE) {
puts("Warning: mtdparts too long,"" please increase CONFIG_SYS_PBSIZE\n");return;- }
Instead of adding essentially dead code that does not really help the end user, it would be better to avoid the potential problems. As log as the console code has not been improved, it may make sense to avoid printf() when you don't really need it.
I recommend to change this
printf("mtdparts: %s\n", mtdparts_default ? mtdparts_default : "none");
into something like
puts("mtdparts: "); puts(mtdparts_default ? mtdparts_default : "none");
Best regards,
Wolfgang Denk
Hello Wolfgang,
Wolfgang Denk wd@denx.de wrote:
- /* Check to prevent printbuffer overflows */
- if (mtdparts_default && strlen(mtdparts_default) > CONFIG_SYS_PBSIZE) {
puts("Warning: mtdparts too long,"" please increase CONFIG_SYS_PBSIZE\n");return;- }
Instead of adding essentially dead code that does not really help the end user, it would be better to avoid the potential problems. As log as the console code has not been improved, it may make sense to avoid printf() when you don't really need it.
This is indeed much better, thanks!
I recommend to change this
printf("mtdparts: %s\n", mtdparts_default ? mtdparts_default : "none");
into something like
puts("mtdparts: "); puts(mtdparts_default ? mtdparts_default : "none");
I'll fix it as suggested, thanks!
Best regards, Anatolij -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de
participants (3)
-
Anatolij Gustschin -
Detlev Zundel -
Wolfgang Denk