| Message ID | 1477330907-13733-6-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/Application/AndroidFastboot/AndroidBootImg.c | 3 ++- > EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c | 4 ++-- > 2 files changed, 4 insertions(+), 3 deletions(-) > > diff --git a/EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c b/EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c > index bbca90fc08a2..f3e770bcc980 100644 > --- a/EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c > +++ b/EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c > @@ -84,7 +84,8 @@ ParseAndroidBootImg ( > + ALIGN_VALUE (Header->KernelSize, Header->PageSize)); > } > > - AsciiStrnCpy (KernelArgs, Header->KernelArgs, BOOTIMG_KERNEL_ARGS_SIZE); > + AsciiStrnCpyS (KernelArgs, BOOTIMG_KERNEL_ARGS_SIZE, Header->KernelArgs, > + BOOTIMG_KERNEL_ARGS_SIZE); > > return EFI_SUCCESS; > } This loses the zero padding, but I guess that's okay. Is fine otherwise. > diff --git a/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c b/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c > index 9ddc34f57cf4..960218b25241 100644 > --- a/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c > +++ b/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c > @@ -127,7 +127,7 @@ HandleDownload ( > if (mDataBuffer == NULL) { > SEND_LITERAL ("FAILNot enough memory"); > } else { > - AsciiStrnCpy (Response + 4, NumBytesString, 8); > + AsciiStrnCpyS (Response + 4, mNumDataBytes, NumBytesString, 8); > mTransport->Send (sizeof(Response), Response, &mFatalSendErrorEvent); > > mState = ExpectDataState; I don't think this is right. Here we're trying to copy no more than 8 characters from NumBytesString into Response, after the "DATA" prefix. mNumDataBytes is the decimal value of NumBytesString, and it's unrelated to this formatting. What we could do is AsciiStrnCpyS (Response + 4, sizeof Response - 4, NumBytesString, 8) in order to remain "surgical". However, that's not right again, because AsciiStrnCpyS() *always* NUL-terminates, and here we only have CHAR8 Response[12] = "DATA"; i.e., no room for values above 0x0FFF_FFFF. Another issue is that the zero padding of AsciiStrnCpy() would be lost, and we actually send the zero padding to the wire. So, the real fix is, in my opinion: * resize Response to 4 + 8 + 1 == 13 bytes, * format it like this: ZeroMem (Response, sizeof Response); AsciiSPrint (Response, sizeof Response, "DATA%x", (UINT32)mNumDataBytes); * send it like this: mTransport->Send (sizeof Response - 1, Response, &mFatalSendErrorEvent); > @@ -257,7 +257,7 @@ AcceptCmd ( > } > > // Commands aren't null-terminated. Let's get a null-terminated version. > - AsciiStrnCpy (Command, Data, Size); > + AsciiStrnCpyS (Command, sizeof Command, Data, Size); > Command[Size] = '\0'; > > // Parse command > This looks good, but the explicit NUL-termination can be dropped, as AsciiStrnCpyS() enforces that internally. Thanks Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
diff --git a/EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c b/EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c index bbca90fc08a2..f3e770bcc980 100644 --- a/EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c +++ b/EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c @@ -84,7 +84,8 @@ ParseAndroidBootImg ( + ALIGN_VALUE (Header->KernelSize, Header->PageSize)); } - AsciiStrnCpy (KernelArgs, Header->KernelArgs, BOOTIMG_KERNEL_ARGS_SIZE); + AsciiStrnCpyS (KernelArgs, BOOTIMG_KERNEL_ARGS_SIZE, Header->KernelArgs, + BOOTIMG_KERNEL_ARGS_SIZE); return EFI_SUCCESS; } diff --git a/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c b/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c index 9ddc34f57cf4..960218b25241 100644 --- a/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c +++ b/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c @@ -127,7 +127,7 @@ HandleDownload ( if (mDataBuffer == NULL) { SEND_LITERAL ("FAILNot enough memory"); } else { - AsciiStrnCpy (Response + 4, NumBytesString, 8); + AsciiStrnCpyS (Response + 4, mNumDataBytes, NumBytesString, 8); mTransport->Send (sizeof(Response), Response, &mFatalSendErrorEvent); mState = ExpectDataState; @@ -257,7 +257,7 @@ AcceptCmd ( } // Commands aren't null-terminated. Let's get a null-terminated version. - AsciiStrnCpy (Command, Data, Size); + AsciiStrnCpyS (Command, sizeof Command, Data, Size); Command[Size] = '\0'; // Parse command
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/Application/AndroidFastboot/AndroidBootImg.c | 3 ++- EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) -- 2.7.4 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel