Re: [OpenBIOS] [PATCH 2/2] SPARC64: Remove limit on kernel command line length caused by using a static buffer

[Mark Cave-Ayland]

On 13/04/13 10:07, Blue Swirl wrote:
> > Instead use strdup() so that we can dynamically allocate the string ourselves. As
>
> Actually there's no call to strdup()...
> > reported by Rob Landley.
> > Signed-off-by: Mark Cave-Ayland
> >
> > openbios-devel/arch/sparc64/openbios.c |   11 ++++-------
> >   1 file changed, 4 insertions(+), 7 deletions(-)
> >
> > diff --git a/openbios-devel/arch/sparc64/openbios.c b/openbios-devel/arch/sparc64/openbios.c
> > index 3b372b9..440e505 100644
> > --- a/openbios-devel/arch/sparc64/openbios.c
> > +++ b/openbios-devel/arch/sparc64/openbios.c
> > @@ -42,9 +42,6 @@
> >   #define NVRAM_OB_START   (0)
> >   #define NVRAM_OB_SIZE    ((0x1fd0 - NVRAM_OB_START)&  ~15)
> >
> > -#define OBIO_CMDLINE_MAX 256
> > -static char obio_cmdline[OBIO_CMDLINE_MAX];
> >
> > static uint8_t idprom[NVRAM_IDPROM_SIZE];
> >
> > struct hwdef {
> >
> > @@ -370,6 +367,7 @@ static uint8_t qemu_uuid[16];
> >
> > void arch_nvram_get(char *data)
> >   {
> > char *obio_cmdline = { '\0' };
>
> Isn't this allocated in the stack? So ...
> >   uint32_t size = 0;
> >   const struct cpudef *cpu;
> >   char buf[256];
> >
> > @@ -401,12 +399,11 @@ void arch_nvram_get(char *data)
> >           kernel_image = fw_cfg_read_i64(FW_CFG_KERNEL_ADDR);
> >   size = fw_cfg_read_i32(FW_CFG_CMDLINE_SIZE);
> > if (size>  OBIO_CMDLINE_MAX - 1)
> >    size = OBIO_CMDLINE_MAX - 1;
> > if (size) {
> >   obio_cmdline = (char *)malloc(size + 1);
> >     fw_cfg_read(FW_CFG_CMDLINE_DATA, obio_cmdline, size);
> > }
> > obio_cmdline[size] = '\0';
> >   obio_cmdline[size] = '\0';
> > }
>
> ... I'd add an 'else' case here with something like obio_cmdline = strdup("").
> >   qemu_cmdline = (uint64_t)obio_cmdline;
> >   cmdline_size = size;
> >   boot_device = fw_cfg_read_i16(FW_CFG_BOOT_DEVICE);

Thanks for the feedback - I've just posted a revised v2 patch to the 
list based upon your comments.

ATB,

Mark.