diff mbox

[U-Boot] fixing wrong termination of string

Message ID 2026242272ea4f6bb96adbd87ca61532@EINTMBXC.einet.ad.eivd.ch
State Changes Requested
Delegated to: Simon Glass
Headers show

Commit Message

Rossier Daniel July 19, 2013, 6:31 p.m. UTC 
Hi,

I discovered a small bug in lib/vsprintf.c which leads to an "Access violation(2)" when I tried to tftp a file, in QEMU.
If CONFIG_SYS_VSNPRINTF is set, the str pointer is incremented even if str reached the end of string (str == end) because of ADDCH.
This leads to a wrong length of string and causes the problem.
Here is the patch:

Regards
Daniel



HEIG-VD
Haute Ecole d'Ingénierie et de Gestion du Canton de Vaud
School of Business and Engineering Vaud
Dr Daniel Rossier
Professeur HES
Département TIC
Institut REDS (Reconfigurable Embedded Digital Systems)
Tél. :

0041 24 55 762 69

Mobile :

0041 79 29 254 58

daniel.rossier@heig-vd.ch<mailto:daniel.rossier@heig-vd.ch>
Route de Cheseaux 1
CH-1401 Yverdon-les-Bains
http://www.heig-vd.ch<http://www.heig-vd.ch/>
[Annonce HEIG-VD]<http://go.heig-vd.ch/bannerEmail/>

Comments

Simon Glass July 21, 2013, 8:07 p.m. UTC | #1 
On Fri, Jul 19, 2013 at 12:31 PM, Rossier Daniel
<Daniel.Rossier@heig-vd.ch>wrote:

> Hi,
>
> I discovered a small bug in lib/vsprintf.c which leads to an "Access
> violation(2)" when I tried to tftp a file, in QEMU.
> If CONFIG_SYS_VSNPRINTF is set, the str pointer is incremented even if str
> reached the end of string (str == end) because of ADDCH.
> This leads to a wrong length of string and causes the problem.
> Here is the patch:
>
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index 82e5c13..2ba8126 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -747,8 +747,9 @@ repeat:
>  #ifdef CONFIG_SYS_VSNPRINTF
>         if (size > 0) {
> -               ADDCH(str, '\0');
> -               if (str > end)
> +               if (str < end)
> +                       *str = '\0';
> +               else
>                         end[-1] = '\0';
>

This is good - but can I suggest going a little further, maybe:

+               if (str < end)
> +                       *str = '\0';
> +               else if (end > buf)
>                         end[-1] = '\0';
>

since I think it is actually valid to call this function with a size of 0,
perhaps to find out the length that would be produced.

Regards,
Simon
diff mbox

Patch

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 82e5c13..2ba8126 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -747,8 +747,9 @@  repeat:
 #ifdef CONFIG_SYS_VSNPRINTF
        if (size > 0) {
-               ADDCH(str, '\0');
-               if (str > end)
+               if (str < end)
+                       *str = '\0';
+               else
                        end[-1] = '\0';
        }
#else