Daniel Kiper
2013-Dec-03 20:28 UTC
[PATCH v2] xen/arch/x86: Fix early boot command line parsing
There is no reliable way to encode NUL character as a character so encode
it as a number. Read: http://sourceware.org/binutils/docs/as/Characters.html.
Octal and hex encoding do not work on at least one system (GNU assembler
version 2.22 (x86_64-linux-gnu) using BFD version (GNU Binutils for Debian)
2.22).
Without this fix e.g. no-real-mode option at the end of xen.gz command line
is not detected.
v2 - suggestions/fixes:
- add relevant comment to assembly file
(suggested by Andrew Cooper),
- leave encoding of other characters as is for readability
(suggested by Jan Beulich).
Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
---
xen/arch/x86/boot/cmdline.S | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/xen/arch/x86/boot/cmdline.S b/xen/arch/x86/boot/cmdline.S
index 05ffb94..b83975e 100644
--- a/xen/arch/x86/boot/cmdline.S
+++ b/xen/arch/x86/boot/cmdline.S
@@ -138,7 +138,15 @@
call .Lstrlen
add $4,%esp
xadd %eax,%ebx
- cmpb $''\0'',(%ebx)
+ /*
+ * There is no reliable way to encode NUL character as a character so
encode
+ * it as a number. Read:
http://sourceware.org/binutils/docs/as/Characters.html.
+ * Octal and hex encoding do not work on at least one system (GNU
assembler
+ * version 2.22 (x86_64-linux-gnu) using BFD version (GNU Binutils for
Debian) 2.22).
+ * Without this fix e.g. no-real-mode option at the end of xen.gz
command line
+ * is not detected.
+ */
+ cmpb $0,(%ebx)
je 3f
cmpb $'' '',(%ebx)
je 3f
--
1.7.10.4
Andrew Cooper
2013-Dec-03 20:32 UTC
Re: [PATCH v2] xen/arch/x86: Fix early boot command line parsing
On 03/12/13 20:28, Daniel Kiper wrote:> There is no reliable way to encode NUL character as a character so encode > it as a number. Read: http://sourceware.org/binutils/docs/as/Characters.html. > Octal and hex encoding do not work on at least one system (GNU assembler > version 2.22 (x86_64-linux-gnu) using BFD version (GNU Binutils for Debian) 2.22). > Without this fix e.g. no-real-mode option at the end of xen.gz command line > is not detected. > > v2 - suggestions/fixes: > - add relevant comment to assembly file > (suggested by Andrew Cooper), > - leave encoding of other characters as is for readability > (suggested by Jan Beulich). > > Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>I was thinking more along the lines of /* NULL check (as $''\0'' == 0x30 in GAS) */ Either way, at least it is obvious. Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>> --- > xen/arch/x86/boot/cmdline.S | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/xen/arch/x86/boot/cmdline.S b/xen/arch/x86/boot/cmdline.S > index 05ffb94..b83975e 100644 > --- a/xen/arch/x86/boot/cmdline.S > +++ b/xen/arch/x86/boot/cmdline.S > @@ -138,7 +138,15 @@ > call .Lstrlen > add $4,%esp > xadd %eax,%ebx > - cmpb $''\0'',(%ebx) > + /* > + * There is no reliable way to encode NUL character as a character so encode > + * it as a number. Read: http://sourceware.org/binutils/docs/as/Characters.html. > + * Octal and hex encoding do not work on at least one system (GNU assembler > + * version 2.22 (x86_64-linux-gnu) using BFD version (GNU Binutils for Debian) 2.22). > + * Without this fix e.g. no-real-mode option at the end of xen.gz command line > + * is not detected. > + */ > + cmpb $0,(%ebx) > je 3f > cmpb $'' '',(%ebx) > je 3f
Keir Fraser
2013-Dec-04 07:16 UTC
Re: [PATCH v2] xen/arch/x86: Fix early boot command line parsing
On 03/12/2013 20:32, "Andrew Cooper" <andrew.cooper3@citrix.com> wrote:> On 03/12/13 20:28, Daniel Kiper wrote: >> There is no reliable way to encode NUL character as a character so encode >> it as a number. Read: http://sourceware.org/binutils/docs/as/Characters.html. >> Octal and hex encoding do not work on at least one system (GNU assembler >> version 2.22 (x86_64-linux-gnu) using BFD version (GNU Binutils for Debian) >> 2.22). >> Without this fix e.g. no-real-mode option at the end of xen.gz command line >> is not detected. >> >> v2 - suggestions/fixes: >> - add relevant comment to assembly file >> (suggested by Andrew Cooper), >> - leave encoding of other characters as is for readability >> (suggested by Jan Beulich). >> >> Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com> > > I was thinking more along the lines of /* NULL check (as $''\0'' == 0x30 > in GAS) */ > > Either way, at least it is obvious. > > Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>Acked-by: Keir Fraser <keir@xen.org>>> --- >> xen/arch/x86/boot/cmdline.S | 10 +++++++++- >> 1 file changed, 9 insertions(+), 1 deletion(-) >> >> diff --git a/xen/arch/x86/boot/cmdline.S b/xen/arch/x86/boot/cmdline.S >> index 05ffb94..b83975e 100644 >> --- a/xen/arch/x86/boot/cmdline.S >> +++ b/xen/arch/x86/boot/cmdline.S >> @@ -138,7 +138,15 @@ >> call .Lstrlen >> add $4,%esp >> xadd %eax,%ebx >> - cmpb $''\0'',(%ebx) >> + /* >> + * There is no reliable way to encode NUL character as a character >> so encode >> + * it as a number. Read: >> http://sourceware.org/binutils/docs/as/Characters.html. >> + * Octal and hex encoding do not work on at least one system (GNU >> assembler >> + * version 2.22 (x86_64-linux-gnu) using BFD version (GNU Binutils >> for Debian) 2.22). >> + * Without this fix e.g. no-real-mode option at the end of xen.gz >> command line >> + * is not detected. >> + */ >> + cmpb $0,(%ebx) >> je 3f >> cmpb $'' '',(%ebx) >> je 3f > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel