* [PATCH] kexec ppc64: fix misaligned cmdline @ 2007-06-04 7:23 Michael Neuling 2007-06-04 9:22 ` Milton Miller 2007-06-04 23:49 ` David Gibson 0 siblings, 2 replies; 10+ messages in thread From: Michael Neuling @ 2007-06-04 7:23 UTC (permalink / raw) To: horms; +Cc: kexec, miltonm, linuxppc-dev If the cmdline changes between boots, we can get misalignment of the bootargs entry, which in turn corrupts our device tree blob and hence kills our kexec boot. Specifically, if the cmdline length was >= 8 before and the new cmdline length is < 8, we can get corruption. Signed-off-by: Michael Neuling <mikey@neuling.org> --- kexec/arch/ppc64/fs2dt.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) Index: kexec-tools-testing/kexec/arch/ppc64/fs2dt.c =================================================================== --- kexec-tools-testing.orig/kexec/arch/ppc64/fs2dt.c +++ kexec-tools-testing/kexec/arch/ppc64/fs2dt.c @@ -197,6 +197,7 @@ static void putprops(char *fn, struct di struct dirent *dp; int i = 0, fd, len; struct stat statbuf; + int dt_realigned = 0; for (i = 0; i < numlist; i++) { dp = nlist[i]; @@ -243,8 +244,10 @@ static void putprops(char *fn, struct di *dt++ = len; *dt++ = propnum(fn); - if ((len >= 8) && ((unsigned long)dt & 0x4)) + if ((len >= 8) && ((unsigned long)dt & 0x4)){ dt++; + dt_realigned = 1; + } fd = open(pathname, O_RDONLY); if (fd == -1) @@ -283,6 +286,8 @@ static void putprops(char *fn, struct di strcat(local_cmdline, " "); cmd_len = strlen(local_cmdline); cmd_len = cmd_len + 1; + if (dt_realigned && cmd_len < 8) + dt--; memcpy(dt, local_cmdline,cmd_len); len = cmd_len; *dt_len = cmd_len; ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] kexec ppc64: fix misaligned cmdline 2007-06-04 7:23 [PATCH] kexec ppc64: fix misaligned cmdline Michael Neuling @ 2007-06-04 9:22 ` Milton Miller 2007-06-04 9:42 ` Michael Neuling 2007-06-04 23:49 ` David Gibson 1 sibling, 1 reply; 10+ messages in thread From: Milton Miller @ 2007-06-04 9:22 UTC (permalink / raw) To: Michael Neuling; +Cc: linuxppc-dev, horms, kexec On Jun 4, 2007, at 2:23 AM, Michael Neuling wrote: > If the cmdline changes between boots, we can get misalignment of the > bootargs entry, which in turn corrupts our device tree blob and hence > kills our kexec boot. ... > - if ((len >= 8) && ((unsigned long)dt & 0x4)) > + if ((len >= 8) && ((unsigned long)dt & 0x4)){ > dt++; > + dt_realigned = 1; > + } > > fd = open(pathname, O_RDONLY); > if (fd == -1) > @@ -283,6 +286,8 @@ static void putprops(char *fn, struct di > strcat(local_cmdline, " "); > cmd_len = strlen(local_cmdline); > cmd_len = cmd_len + 1; > + if (dt_realigned && cmd_len < 8) > + dt--; > While this appears to fix the stated problem, did you explore my suggestion of deleting and creating the property like we do for the initrd base and size? Deleting and recreating should also handle the case of no boot-args in the original /chosen sub-tree (when the bootloader didn't (need to) supply a command line). milton ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] kexec ppc64: fix misaligned cmdline 2007-06-04 9:22 ` Milton Miller @ 2007-06-04 9:42 ` Michael Neuling 2007-06-07 1:19 ` Michael Neuling 0 siblings, 1 reply; 10+ messages in thread From: Michael Neuling @ 2007-06-04 9:42 UTC (permalink / raw) To: Milton Miller; +Cc: linuxppc-dev, horms, kexec > While this appears to fix the stated problem, did you explore my > suggestion of deleting and creating the property like we do for > the initrd base and size? Yeah, but it didn't seem "hapazard" enough for kexec. :-) > Deleting and recreating should also handle the case of no boot-args in > the original /chosen sub-tree (when the bootloader didn't (need to) > supply a command line). Yeah, we're screwed when the first boot has no bootargs. Let me get the policy right. We blow away the old command line except for the "root=blah" item. Correct? Mikey ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] kexec ppc64: fix misaligned cmdline 2007-06-04 9:42 ` Michael Neuling @ 2007-06-07 1:19 ` Michael Neuling 2007-06-07 16:19 ` Geoff Levand 2007-06-19 5:06 ` Horms 0 siblings, 2 replies; 10+ messages in thread From: Michael Neuling @ 2007-06-07 1:19 UTC (permalink / raw) To: Milton Miller, linuxppc-dev, horms, kexec, Santhosh Rao If the cmdline changes between boots, we can get misalignment of the bootargs entry, which in turn corrupts our device tree blob and hence kills our kexec boot. Also, if there was no /chosen/bootargs previously, we'd never add a new one. This ignores the bootargs while parsing the tree and inserts it later. Signed-off-by: Michael Neuling <mikey@neuling.org> --- This hopefully address issues raised by Milton. kexec/arch/ppc64/fs2dt.c | 95 +++++++++++++++++++++++++++++++---------------- 1 file changed, 63 insertions(+), 32 deletions(-) Index: kexec-tools-testing/kexec/arch/ppc64/fs2dt.c =================================================================== --- kexec-tools-testing.orig/kexec/arch/ppc64/fs2dt.c +++ kexec-tools-testing/kexec/arch/ppc64/fs2dt.c @@ -18,6 +18,7 @@ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. */ +#define _GNU_SOURCE #include <sys/types.h> #include <sys/stat.h> @@ -233,6 +234,12 @@ static void putprops(char *fn, struct di !reuse_initrd) continue; + /* This property will be created later in putnode() So + * ignore it now. + */ + if (!strcmp(dp->d_name, "bootargs")) + continue; + if (! S_ISREG(statbuf.st_mode)) continue; @@ -257,38 +264,6 @@ static void putprops(char *fn, struct di checkprop(fn, dt, len); - /* Get the cmdline from the device-tree and modify it */ - if (!strcmp(dp->d_name, "bootargs")) { - int cmd_len; - char temp_cmdline[COMMAND_LINE_SIZE] = { "" }; - char *param = NULL; - cmd_len = strlen(local_cmdline); - if (cmd_len != 0) { - param = strstr(local_cmdline, "crashkernel="); - if (param) - crash_param = 1; - param = strstr(local_cmdline, "root="); - } - if (!param) { - char *old_param; - memcpy(temp_cmdline, dt, len); - param = strstr(temp_cmdline, "root="); - if (param) { - old_param = strtok(param, " "); - if (cmd_len != 0) - strcat(local_cmdline, " "); - strcat(local_cmdline, old_param); - } - } - strcat(local_cmdline, " "); - cmd_len = strlen(local_cmdline); - cmd_len = cmd_len + 1; - memcpy(dt, local_cmdline,cmd_len); - len = cmd_len; - *dt_len = cmd_len; - fprintf(stderr, "Modified cmdline:%s\n", local_cmdline); - } - dt += (len + 3)/4; if (!strcmp(dp->d_name, "reg") && usablemem_rgns.size) add_usable_mem_property(fd, len); @@ -385,6 +360,62 @@ static void putnode(void) reserve(initrd_base, initrd_size); } + /* Add cmdline to the second kernel. Check to see if the new + * cmdline has a root=. If not, use the old root= cmdline. */ + if (!strcmp(basename,"/chosen/")) { + size_t cmd_len = 0; + char *param = NULL; + + cmd_len = strlen(local_cmdline); + if (cmd_len != 0) { + param = strstr(local_cmdline, "crashkernel="); + if (param) + crash_param = 1; + /* does the new cmdline have a root= ? ... */ + param = strstr(local_cmdline, "root="); + } + + /* ... if not, grab root= from the old command line */ + if (!param) { + char filename[MAXPATH]; + FILE *fp; + char *last_cmdline = NULL; + char *old_param; + + strcpy(filename, pathname); + strcat(filename, "bootargs"); + fp = fopen(filename, "r"); + if (fp) { + if (getline(&last_cmdline, &cmd_len, fp) == -1) + die("unable to read %s\n", filename); + + param = strstr(last_cmdline, "root="); + if (param) { + old_param = strtok(param, " "); + if (cmd_len != 0) + strcat(local_cmdline, " "); + strcat(local_cmdline, old_param); + } + } + if (last_cmdline) + free(last_cmdline); + } + strcat(local_cmdline, " "); + cmd_len = strlen(local_cmdline); + cmd_len = cmd_len + 1; + + /* add new bootargs */ + *dt++ = 3; + *dt++ = cmd_len; + *dt++ = propnum("bootargs"); + if ((cmd_len >= 8) && ((unsigned long)dt & 0x4)) + dt++; + memcpy(dt, local_cmdline,cmd_len); + dt += (cmd_len + 3)/4; + + fprintf(stderr, "Modified cmdline:%s\n", local_cmdline); + } + for (i=0; i < numlist; i++) { dp = namelist[i]; strcpy(dn, dp->d_name); ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] kexec ppc64: fix misaligned cmdline 2007-06-07 1:19 ` Michael Neuling @ 2007-06-07 16:19 ` Geoff Levand 2007-06-19 5:06 ` Horms 1 sibling, 0 replies; 10+ messages in thread From: Geoff Levand @ 2007-06-07 16:19 UTC (permalink / raw) To: Michael Neuling; +Cc: dwmw2, kexec, Milton Miller, linuxppc-dev, horms Michael Neuling wrote: > If the cmdline changes between boots, we can get misalignment of the > bootargs entry, which in turn corrupts our device tree blob and hence > kills our kexec boot. Also, if there was no /chosen/bootargs > previously, we'd never add a new one. > > This ignores the bootargs while parsing the tree and inserts it later. > > Signed-off-by: Michael Neuling <mikey@neuling.org> Hi. Acked-by: Geoff Levand <geoffrey.levand@am.sony.com> I tested this with PS3 and it works as expected. I added it to my ps3-kexec-tools.git. With this patch mainline kexec will work with the PS3. I still have a work-in-progress patch started by David Woodhouse to support building a 32 bit kexec executable for PPC64 platforms. I don't have any definite plan to work on it though. -Geoff ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] kexec ppc64: fix misaligned cmdline 2007-06-07 1:19 ` Michael Neuling 2007-06-07 16:19 ` Geoff Levand @ 2007-06-19 5:06 ` Horms 1 sibling, 0 replies; 10+ messages in thread From: Horms @ 2007-06-19 5:06 UTC (permalink / raw) To: Michael Neuling; +Cc: linuxppc-dev, kexec, Milton Miller On Thu, Jun 07, 2007 at 11:19:22AM +1000, Michael Neuling wrote: > If the cmdline changes between boots, we can get misalignment of the > bootargs entry, which in turn corrupts our device tree blob and hence > kills our kexec boot. Also, if there was no /chosen/bootargs > previously, we'd never add a new one. > > This ignores the bootargs while parsing the tree and inserts it later. > > Signed-off-by: Michael Neuling <mikey@neuling.org> Thanks, applied. -- Horms H: http://www.vergenet.net/~horms/ W: http://www.valinux.co.jp/en/ ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] kexec ppc64: fix misaligned cmdline 2007-06-04 7:23 [PATCH] kexec ppc64: fix misaligned cmdline Michael Neuling 2007-06-04 9:22 ` Milton Miller @ 2007-06-04 23:49 ` David Gibson 2007-06-04 23:56 ` Michael Neuling 1 sibling, 1 reply; 10+ messages in thread From: David Gibson @ 2007-06-04 23:49 UTC (permalink / raw) To: Michael Neuling; +Cc: linuxppc-dev, horms, kexec, miltonm On Mon, Jun 04, 2007 at 05:23:45PM +1000, Miichael Neuling wrote: > If the cmdline changes between boots, we can get misalignment of the > bootargs entry, which in turn corrupts our device tree blob and hence > kills our kexec boot. > > Specifically, if the cmdline length was >= 8 before and the new cmdline > length is < 8, we can get corruption. Hrm. Have you considered using dtc for this conversion, rather than this somewhat dubious looking fs2dt? -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] kexec ppc64: fix misaligned cmdline 2007-06-04 23:49 ` David Gibson @ 2007-06-04 23:56 ` Michael Neuling 2007-06-05 0:16 ` David Gibson 0 siblings, 1 reply; 10+ messages in thread From: Michael Neuling @ 2007-06-04 23:56 UTC (permalink / raw) To: David Gibson; +Cc: linuxppc-dev, horms, kexec, miltonm > > If the cmdline changes between boots, we can get misalignment of the > > bootargs entry, which in turn corrupts our device tree blob and hence > > kills our kexec boot. > > > > Specifically, if the cmdline length was >= 8 before and the new cmdline > > length is < 8, we can get corruption. > > Hrm. Have you considered using dtc for this conversion, rather than > this somewhat dubious looking fs2dt? Numerous times. fs2dt needs to be converted to use the device tree library, but that will take a lot more work than a 5 minute patch. Mikey ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] kexec ppc64: fix misaligned cmdline 2007-06-04 23:56 ` Michael Neuling @ 2007-06-05 0:16 ` David Gibson 2007-06-05 0:58 ` Michael Neuling 0 siblings, 1 reply; 10+ messages in thread From: David Gibson @ 2007-06-05 0:16 UTC (permalink / raw) To: Michael Neuling; +Cc: linuxppc-dev, horms, kexec, miltonm On Tue, Jun 05, 2007 at 09:56:37AM +1000, Miichael Neuling wrote: > > > If the cmdline changes between boots, we can get misalignment of the > > > bootargs entry, which in turn corrupts our device tree blob and hence > > > kills our kexec boot. > > > > > > Specifically, if the cmdline length was >= 8 before and the new cmdline > > > length is < 8, we can get corruption. > > > > Hrm. Have you considered using dtc for this conversion, rather than > > this somewhat dubious looking fs2dt? > > Numerous times. > > fs2dt needs to be converted to use the device tree library, but that > will take a lot more work than a 5 minute patch. Well, sure.. but do you need fs2dt at all. dtc has an "fs" input mode.. -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] kexec ppc64: fix misaligned cmdline 2007-06-05 0:16 ` David Gibson @ 2007-06-05 0:58 ` Michael Neuling 0 siblings, 0 replies; 10+ messages in thread From: Michael Neuling @ 2007-06-05 0:58 UTC (permalink / raw) To: David Gibson; +Cc: linuxppc-dev, horms, kexec, miltonm > On Tue, Jun 05, 2007 at 09:56:37AM +1000, Miichael Neuling wrote: > > > > If the cmdline changes between boots, we can get misalignment of the > > > > bootargs entry, which in turn corrupts our device tree blob and hence > > > > kills our kexec boot. > > > > > > > > Specifically, if the cmdline length was >= 8 before and the new cmdline > > > > length is < 8, we can get corruption. > > > > > > Hrm. Have you considered using dtc for this conversion, rather than > > > this somewhat dubious looking fs2dt? > > > > Numerous times. > > > > fs2dt needs to be converted to use the device tree library, but that > > will take a lot more work than a 5 minute patch. > > > Well, sure.. but do you need fs2dt at all. dtc has an "fs" input > mode.. As discussed offline, you can do this with the --devicetreeblob kexec option. The issue is the special cases like having to modify the kernel cmdline options or initrd locations. Mikey ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2007-06-19 5:06 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2007-06-04 7:23 [PATCH] kexec ppc64: fix misaligned cmdline Michael Neuling 2007-06-04 9:22 ` Milton Miller 2007-06-04 9:42 ` Michael Neuling 2007-06-07 1:19 ` Michael Neuling 2007-06-07 16:19 ` Geoff Levand 2007-06-19 5:06 ` Horms 2007-06-04 23:49 ` David Gibson 2007-06-04 23:56 ` Michael Neuling 2007-06-05 0:16 ` David Gibson 2007-06-05 0:58 ` Michael Neuling
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).