[U-Boot] [PATCH v2] bootm: Avoid 256-byte overflow in fixup_silent_linux()

Doug Anderson dianders at chromium.org
Tue Jan 10 23:51:15 CET 2012


Dear Wolfgang Denk,

On Tue, Jan 10, 2012 at 2:28 PM, Wolfgang Denk <wd at denx.de> wrote:
>> This makes fixup_silent_linux() use malloc() to allocate its
>> working space, meaning that our maximum kernel command line
>> should only be limited by malloc().  Previously it was silently
>> overflowing the stack.
> ...
>>  static void fixup_silent_linux(void)
>>  {
>> -     char buf[256], *start, *end;
>
> Are you sure that the kernel's buffer is long enough?

The kernel's buffer might be big enough, depending on the
architecture.  For ARM (which is what I'm on), I see that the command
line is 1024 bytes.

Here's where I'm looking:
<http://git.kernel.org/?p=linux/kernel/git/next/linux-next.git;a=blob;f=arch/arm/include/asm/setup.h;h=23ebc0c82a3975ae5c455dd39598e93ab33922e7;hb=refs/heads/master#l19>


> I think your patch is likely to break all these architectures?

I'm not sure how my patch would break these architectures.  My patch
removes a buffer overrun--it doesn't actually increase any particular
board's command line length.  I need this because my board uses a
command line that is ~300 bytes--under the kernel limit but currently
over u-boot's.

I agree completely that this patch doesn't remove all limits on Linux
command line length.  However, it does allow you to use the full Linux
command line length on kernels that have a #define with something
greater than 256.

In addition, a buffer overrun is a particularly gnarly failure case
(opens you up to all sorts of security attacks if someone can figure
out how to manipulate the command line), so is generally good to fix.

If you'd rather, I'd be happy to rework the patch to change the
hardcoded 256 to a CONFIG_COMMAND_LINE_SIZE, then add overflow
checking to the function.  That would allow my use case and also
prevent future buffer overruns.


-Doug


More information about the U-Boot mailing list