| Message ID | 1477330907-13733-9-git-send-email-ard.biesheuvel@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 10/24/16 19:41, Ard Biesheuvel wrote: > Get rid of calls to unsafe string functions. These are deprecated and may > be removed in the future. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > EmbeddedPkg/Universal/MmcDxe/Diagnostics.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/EmbeddedPkg/Universal/MmcDxe/Diagnostics.c b/EmbeddedPkg/Universal/MmcDxe/Diagnostics.c > index 783e548d2aed..b489393e27b0 100644 > --- a/EmbeddedPkg/Universal/MmcDxe/Diagnostics.c > +++ b/EmbeddedPkg/Universal/MmcDxe/Diagnostics.c > @@ -44,7 +44,7 @@ DiagnosticLog ( > UINTN len = StrLen (Str); > if (len <= mLogRemainChar) { > mLogRemainChar -= len; > - StrCpy (mLogBuffer, Str); > + StrCpyS (mLogBuffer, mLogRemainChar, Str); > mLogBuffer += len; > return len; > } else { > I think this is incorrect: "mLogRemainChar" is decreased before printing. Also, the original controlling expression is broken: it will permit the body with (len == mLogRemainChar). The effect thereof, for the original code, is a buffer overflow, the terminating L'\0' will be written outside of the buffer. (See the allocation in DiagnosticInitLog().) This buffer overflow is actually fixed by the patch (by throwing away the last character and storing an L'\0' instead). However, that in turn invalidates the returned value (because not all "len" characters have been written). So, please move the decrement after the StrCpyS(), and replace the <= with <. Thanks Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
diff --git a/EmbeddedPkg/Universal/MmcDxe/Diagnostics.c b/EmbeddedPkg/Universal/MmcDxe/Diagnostics.c index 783e548d2aed..b489393e27b0 100644 --- a/EmbeddedPkg/Universal/MmcDxe/Diagnostics.c +++ b/EmbeddedPkg/Universal/MmcDxe/Diagnostics.c @@ -44,7 +44,7 @@ DiagnosticLog ( UINTN len = StrLen (Str); if (len <= mLogRemainChar) { mLogRemainChar -= len; - StrCpy (mLogBuffer, Str); + StrCpyS (mLogBuffer, mLogRemainChar, Str); mLogBuffer += len; return len; } else {
Get rid of calls to unsafe string functions. These are deprecated and may be removed in the future. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- EmbeddedPkg/Universal/MmcDxe/Diagnostics.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 2.7.4 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel