NetBSD Problem Report #49208
From khorben@defora.org Wed Sep 17 00:00:20 2014
Return-Path: <khorben@defora.org>
Received: from mail.netbsd.org (mail.netbsd.org [149.20.53.66])
(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
(Client CN "mail.netbsd.org", Issuer "Postmaster NetBSD.org" (verified OK))
by mollari.NetBSD.org (Postfix) with ESMTPS id 2941FCE2F5
for <gnats-bugs@gnats.NetBSD.org>; Wed, 17 Sep 2014 00:00:20 +0000 (UTC)
Message-Id: <20140916235752.B166319A3D@sleak.defora.rom>
Date: Wed, 17 Sep 2014 01:57:52 +0200 (CEST)
From: Pierre Pronchery <khorben@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Subject: The first-stage FAT bootloader does not work
X-Send-Pr-Version: 3.95
>Number: 49208
>Category: port-i386
>Synopsis: bootxx_msdos fails to load the second-stage bootloader
>Confidential: no
>Severity: serious
>Priority: medium
>Responsible: port-i386-maintainer
>State: closed
>Class: sw-bug
>Submitter-Id: net
>Arrival-Date: Wed Sep 17 00:05:00 +0000 2014
>Closed-Date: Mon Oct 27 11:37:23 +0000 2014
>Last-Modified: Mon Oct 27 11:37:23 +0000 2014
>Originator: Pierre Pronchery
>Release: NetBSD 6.1_STABLE
>Organization:
The NetBSD Foundation
>Environment:
System: NetBSD sleak.defora.rom 6.1_STABLE NetBSD 6.1_STABLE (GENERIC) #1: Fri Aug 1 10:00:52 UTC 2014 edgebsd@altar.edgebsd.org:/home/edgebsd/amd64/obj/sys/arch/amd64/compile/GENERIC amd64
Architecture: x86_64
Machine: amd64
>Description:
My latest attempts at booting NetBSD from a USB memory stick formatted with
either the FAT16 or FAT32 filesystem all failed with the following error at
boot-time: "Can't open /boot".
I identified the error message to be coming from
src/sys/arch/i386/stand/bootxx/boot1.c:
62 const char *
63 boot1(uint32_t biosdev, uint64_t *sector)
64 {
[...]
109 /* if we fail here, so will fstat, so keep going */
110 if (fd == -1 || fstat(fd, &sb) == -1)
111 return "Can't open /boot\r\n";
Digging through the code, and after planting a few printf()'s^W^W^Wputstr()'s
in strategic places, I found the problem to originate from
src/sys/lib/libsa/dosfs.c:
470 static int
471 namede(DOS_FS *fs, const char *path, const struct direntry **dep)
472 {
[...]
483 while (*path) {
484 if (!(s = strchr(path, '/')))
485 s = strchr(path, 0);
486 if ((n = s - path) > 255)
487 return ENAMETOOLONG;
Where strchr("boot", 0) on line 485 would unexpectedly return NULL and the
following check underflow. A quick disassembly of strchr() from
$OBJDIR/sys/arch/i386/stand/bootxx/bootxx_msdos/lib/kern/libkern.a(strchr.o)
(as bootxx_msdos.map suggests) showed that the size-optimized i386 assembly
version of strchr() checks for the string termination character (thus
returning NULL) before checking that the NUL character is not what is actually
the subject of the search:
34 ENTRY(strchr)
[...]
41 1:
42 cmpb %cl, 0(%eax)
43 je 3f
44 cmpb $0, 0(%eax)
45 je 2f
Inverting the checks lines 42-43 and 44-45 solved the problem for me.
>How-To-Repeat:
Follow the instructions from installboot(8) to create a bootable USB memory
stick:
[partition 0 with type 11, offset 63, take everything]
# fdisk -u sd0
[be sure the first partition is active]
# fdisk -fa0 sd0
[remove and insert the USB stick again to be sure the disklabel matches]
# disklabel sd0
[you should see the "e" partition of type "MSDOS"]
# newfs_msdos -F 32 -r 16 sd0e
[it should say "MBR type: 11" just like we already selected]
# mkdir -p /mnt/usb
# mount /dev/sd0e /mnt/usb
# cp /usr/mdec/boot /mnt/usb/boot
# fetch -o /mnt/usb/netbsd.gz \
http://ftp.edgebsd.org/pub/EdgeBSD/EdgeBSD-7/amd64/binary/kernel/netbsd-INSTALL.gz
# umount /mnt/usb
# installboot -t raw /dev/rsd0e /usr/mdec/bootxx_msdos
It is also possible to use vnconfig(8) and test from qemu(1), which was very
useful during my tests:
$ dd if=/dev/zero of=msdos.img bs=512 count=200000 conv=sparse
$ sudo vnconfig -c vnd0 msdos.img
[perform all of the above, using vnd0 instead of sd0]
$ sudo vnconfig -u vnd0
$ qemu-system-x86_64 -net none -boot once=c,strict=on msdos.img
[this disables network boot to better see the messages from the bootloader]
>Fix:
This patch solved the issue for me. Let me know if I can commit it.
If so I will request pull-ups as appropriate.
commit 7fce87e97f920fae760e4fccd4db25c382784211
Author: Pierre Pronchery <khorben@EdgeBSD.org>
Date: Tue Sep 16 19:43:09 2014 +0200
Test for the character to match before testing for end of string
This fixes strchr(string, '\0') for the users of this file, like
bootxx_msdos which did not work without this fix.
diff --git a/common/lib/libc/arch/i386/string/small/strchr.S b/common/lib/libc/arch/i386/string/small/strchr.S
index 5b33347..bfa3461 100644
--- a/common/lib/libc/arch/i386/string/small/strchr.S
+++ b/common/lib/libc/arch/i386/string/small/strchr.S
@@ -39,10 +39,10 @@ ENTRY(strchr)
pushl %eax
pushl %edx
1:
- cmpb $0, 0(%eax)
- je 2f
cmpb %cl, 0(%eax)
je 3f
+ cmpb $0, 0(%eax)
+ je 2f
incl %eax
jmp 1b
2:
>Release-Note:
>Audit-Trail:
From: "Pierre Pronchery" <khorben@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/49208 CVS commit: src/common/lib/libc/arch/i386/string/small
Date: Mon, 22 Sep 2014 20:31:56 +0000
Module Name: src
Committed By: khorben
Date: Mon Sep 22 20:31:56 UTC 2014
Modified Files:
src/common/lib/libc/arch/i386/string/small: strchr.S
Log Message:
Look for the character to locate before checking for the NUL character
As documented in PR port-i386/49208, this fixes strchr(s, '\0'), as used by
the FAT first-stage bootloader on x86 (bootxx_msdos).
strchr(s, '\0') is otherwise equivalent to strlen(string), which would
probably look nicer in the original file, dosfs.c from libsa.
Confirmed working in qemu and on real hardware.
ok joerg@
XXX pull-up to netbsd-6 and netbsd-7
To generate a diff of this commit:
cvs rdiff -u -r1.2 -r1.3 src/common/lib/libc/arch/i386/string/small/strchr.S
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
State-Changed-From-To: open->pending-pullups
State-Changed-By: khorben@NetBSD.org
State-Changed-When: Fri, 26 Sep 2014 11:43:38 +0000
State-Changed-Why:
See pull-ups #1158 for netbsd-6 and #118 for netbsd-7
From: "SAITOH Masanobu" <msaitoh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/49208 CVS commit: [netbsd-7] src/common/lib/libc/arch/i386/string/small
Date: Mon, 29 Sep 2014 15:31:01 +0000
Module Name: src
Committed By: msaitoh
Date: Mon Sep 29 15:31:01 UTC 2014
Modified Files:
src/common/lib/libc/arch/i386/string/small [netbsd-7]: strchr.S
Log Message:
Pull up following revision(s) (requested by khorben in ticket #118):
common/lib/libc/arch/i386/string/small/strchr.S: revision 1.3
Look for the character to locate before checking for the NUL character
As documented in PR port-i386/49208, this fixes strchr(s, '\0'), as used by
the FAT first-stage bootloader on x86 (bootxx_msdos).
strchr(s, '\0') is otherwise equivalent to strlen(string), which would
probably look nicer in the original file, dosfs.c from libsa.
Confirmed working in qemu and on real hardware.
ok joerg@
XXX pull-up to netbsd-6 and netbsd-7
To generate a diff of this commit:
cvs rdiff -u -r1.2 -r1.2.4.1 \
src/common/lib/libc/arch/i386/string/small/strchr.S
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
From: "SAITOH Masanobu" <msaitoh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/49208 CVS commit: [netbsd-6] src/common/lib/libc/arch/i386/string/small
Date: Mon, 27 Oct 2014 10:51:16 +0000
Module Name: src
Committed By: msaitoh
Date: Mon Oct 27 10:51:16 UTC 2014
Modified Files:
src/common/lib/libc/arch/i386/string/small [netbsd-6]: strchr.S
Log Message:
Pull up following revision(s) (requested by khorben in ticket #1158):
common/lib/libc/arch/i386/string/small/strchr.S: revision 1.3
Look for the character to locate before checking for the NUL character
As documented in PR port-i386/49208, this fixes strchr(s, '\0'), as used by
the FAT first-stage bootloader on x86 (bootxx_msdos).
strchr(s, '\0') is otherwise equivalent to strlen(string), which would
probably look nicer in the original file, dosfs.c from libsa.
Confirmed working in qemu and on real hardware.
ok joerg@
XXX pull-up to netbsd-6 and netbsd-7
To generate a diff of this commit:
cvs rdiff -u -r1.1 -r1.1.4.1 \
src/common/lib/libc/arch/i386/string/small/strchr.S
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
State-Changed-From-To: pending-pullups->closed
State-Changed-By: msaitoh@NetBSD.org
State-Changed-When: Mon, 27 Oct 2014 11:37:23 +0000
State-Changed-Why:
Fixed and pulled up to netbsd-[67].
Thanks.
>Unformatted:
(Contact us)
$NetBSD: query-full-pr,v 1.39 2013/11/01 18:47:49 spz Exp $
$NetBSD: gnats_config.sh,v 1.8 2006/05/07 09:23:38 tsutsui Exp $
Copyright © 1994-2014
The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.