From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:54364) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ULaZE-00051z-5c for qemu-devel@nongnu.org; Fri, 29 Mar 2013 10:46:09 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ULaZB-0005sk-ME for qemu-devel@nongnu.org; Fri, 29 Mar 2013 10:46:08 -0400 MIME-Version: 1.0 In-Reply-To: <1364517522.31713.59.camel@liguang.fnst.cn.fujitsu.com> References: <1364364631-24665-1-git-send-email-lig.fnst@cn.fujitsu.com> <20130328094237.GE24910@stefanha-thinkpad.redhat.com> <1364517522.31713.59.camel@liguang.fnst.cn.fujitsu.com> Date: Fri, 29 Mar 2013 15:46:04 +0100 Message-ID: From: Stefan Hajnoczi Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [Qemu-trivial] [PATCH] hw/i386/pc: reject to boot a wrong header magic kernel List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: li guang Cc: qemu-trivial , qemu-devel On Fri, Mar 29, 2013 at 1:38 AM, li guang wrote: > =E5=9C=A8 2013-03-28=E5=9B=9B=E7=9A=84 10:42 +0100=EF=BC=8CStefan Hajnocz= i=E5=86=99=E9=81=93=EF=BC=9A >> On Wed, Mar 27, 2013 at 02:10:31PM +0800, liguang wrote: >> > if head magic is missing or wrong unexpectedly, we'd >> > better to reject booting. >> > e.g. >> > I make a mistake to boot a vmlinuz for MIPS(which >> > I think it's for x86) like this: >> > qemu-system-x86_64 -kernel vmlinuz -initrd demord >> > then qemu report: >> > "qemu: linux kernel too old to load a ram disk" >> > that's misleading. >> > >> > Signed-off-by: liguang >> > --- >> > hw/i386/pc.c | 4 +++- >> > 1 files changed, 3 insertions(+), 1 deletions(-) >> > >> > diff --git a/hw/i386/pc.c b/hw/i386/pc.c >> > index b1e06fa..2b78dfc 100644 >> > --- a/hw/i386/pc.c >> > +++ b/hw/i386/pc.c >> > @@ -683,8 +683,10 @@ static void load_linux(void *fw_cfg, >> > if (load_multiboot(fw_cfg, f, kernel_filename, initrd_filenam= e, >> > kernel_cmdline, kernel_size, header)) { >> > return; >> > + } else { >> > + fprintf(stderr, "please assure specicified kernel is for = x86!\n"); >> > + exit(1); >> >> load_multiboot() can fail for other reasons so this error messing is >> misleading. Giving QEMU a non-x86 kernel is just one scenario where >> this may fail. > > according to my check of load_mutiboot function, > mostly it will return 0 if it's not multboot, > or 1 it's a multiboot, so print this message, > or can I just print "wrong kernel image!" ? Yes, load_multiboot() fails if the image is not a valid multiboot image. An error message like "not a valid multiboot image" is good. >> >> > } >> > - protocol =3D 0; >> > } >> >> Why did you drop protocol =3D 0? > > I think we only want either normal or multi boot linux kernel, > I can't see meaning let other case go on. > so, here, if a normal kernel, OK, go on, > if multiboot, OK, go on, > others, NO, end up. > so, "protocol =3D 0" is meaningless here. I looked in more detail now and here is why we cannot drop this line of cod= e: uint16_t protocol; Note that the variable is not initialized. [...] if (ldl_p(header+0x202) =3D=3D 0x53726448) protocol =3D lduw_p(header+0x206); else { /* This looks like a multiboot kernel. If it is, let's stop treating it like a Linux kernel. */ if (load_multiboot(fw_cfg, f, kernel_filename, initrd_filename, kernel_cmdline, kernel_size, header)) return; protocol =3D 0; Set it to 0 here so it's initialized } if (protocol < 0x200 || !(header[0x211] & 0x01)) { Use variable here - so we *must* initialize it before use. Stefan