ls /dev causes a page fault

[apprentice]

When reading the contents of /dev in any way the kernel crashes. I attached a (poor quality) image of the stack crawl to this ticket.

[apprentice]

stack crawl

[Adek336]

What does listdev say ?

[apprentice]

->listdev

device Multimedia controller (Multimedia audio controller) [4|1|0]

vendor 1106: VIA Technologies, Inc. device 3059: VT8233/A/8235/8237 AC97 Audio Controller

device Mass storage controller (IDE interface) [1|1|8a]

vendor 1106: VIA Technologies, Inc. device 0571: VT82C586A/B/VT82C686/A/B/VT823x/A/C PIPC Bus Master IDE

device Bridge (ISA bridge) [6|1|0]

vendor 1106: VIA Technologies, Inc. device 3177: VT8235 ISA Bridge

device Serial bus controller (USB Controller, EHCI) [c|3|20]

vendor 1106: VIA Technologies, Inc. device 3104: USB 2.0

device Serial bus controller (USB Controller, UHCI) [c|3|0]

vendor 1106: VIA Technologies, Inc. device 3038: VT82xxxxx UHCI USB 1.1 Controller

device Serial bus controller (USB Controller, UHCI) [c|3|0]

vendor 1106: VIA Technologies, Inc. device 3038: VT82xxxxx UHCI USB 1.1 Controller

device Serial bus controller (USB Controller, UHCI) [c|3|0]

vendor 1106: VIA Technologies, Inc. device 3038: VT82xxxxx UHCI USB 1.1 Controller

device Network controller (Ethernet controller) [2|0|0]

vendor 10b7: 3Com Corporation device 9001: 3c900 10Mbps Combo [Boomerang]

device Display controller (VGA compatible controller, VGA controller) [3|0|0]

vendor 10de: nVidia Corporation device 0322: NV34 [GeForce FX 5200]

device Bridge (PCI bridge, Normal decode) [6|4|0]

vendor 1106: VIA Technologies, Inc. device b099: VT8366/A/7 [Apollo KT266/A/333 AGP]

device Bridge (Host bridge) [6|0|0]

vendor 1106: VIA Technologies, Inc. device 3099: VT8366/A/7 [Apollo KT266/A/333]

---

Seems that according to listdev the device is loaded and working. If I have time tomorrow I'll remove the card from the mainboard and try to read the contents of the /dev directory again. Can anyone tell me that when the hardware is found but the device driver fails to initialize itself, does it still create the device node. It shouldn't because the device isn't working anyway (Duh!). Maybe it just thinks the device is live and tries to use some structure uninitialized resulting into a null pointer exception that crashes the system. Just poking thin ice, since I don't really know what the kernel does when one tries to access the /dev filesystem...

[umccullough]

Actually, listdev just queries the pci bus to get that info. It doesn't suggest that a driver has been loaded for the devices yet.

[Adek336]

Could you attach the syslog, please? Does ls /dev/net, ls /dev/audio, ls /dev/disk work? Does dd if=/dev/zero of=/dev/null work?

If you start up with the Safe Mode Console (press space at boot time, choose Safe Mode IIRC), can you do ls /dev?

[apprentice]

I Attached the syslog, but haven't had the time to test any commands. Somebody suggested that the bug might be in Haiku's fbsd layer and said that my 3com boomerang nic that doesn't initialize the hardware properly might be related(I posted a separate ticket on this but don't remember the ticket number and this is a duplicate syslog) to the kernel crash.

[apprentice]

I booted into safe mode as suggested and disabled the bios part also. ls /dev returned the contents of the /dev directory and I could cd to every directory without a hitch, so far so good. For a final stress test I ran 'find .' under /dev. Kernel crashed again and the stack trace looked the same as the earlier (beefdead -- read error). None of the commands worked without safe mode. Maybe the problem is in my mainboard's bios (msi kt3 ultra2).

[anevilyak]

The BIOS shouldn't actually be involved when looking at /dev, it's most probably a buggy driver or something along those lines. When you ran 'find .', was that in safe mode or normal? What might be interesting is the list of kernel images when you're not in safe mode (listimage on the kernel's team ID will give you this).

[apprentice]

It crashed in safe mode when running find. I will check the listimage later. I haven't yet tried the dd command in safe mode either. It is also interesting that I can cd into any directory that /dev contains, I just can't read the contents of the directory. ls /dev/s[TAB] for example crashes the kernel immediately. Also I can mount my linux partitions from the gui just fine.

[apprentice]

I just booted into haiku in normal mode. dd if=/dev/zero of=/dev/null worked! ls /dev/zero and ls /dev/null took ages to complete but they worked also. Again ls /dev crashed. It must be some specific device file in /dev that cannot be stat()d. Can someone tell me what devices are under /dev when haiku boots normally, but not in safe mode, since in safe mode ls /dev works. I could then try to ls every file there and maybe find what device node is buggy.

[Adek336]

ls /dev is the same under safe and normal mode

~> ls /dev
audio  console  dprintf  graphics  keyboard  misc  null  ptmx    tt   urandom
bus    disk     dvb      input     midi      net   pt    random  tty  zero

but some are files (as opposed to directories):

console, dprintf, keyboard, null, ptmx, random, tty, random, zero

perhaps you can ls or stat them. If that works, these are more files for ls / stat

/dev/pt/p0
/dev/misc/test/1
/dev/misc/poke
/dev/graphics/vesa

Perhaps deleting /boot/beos/add-ons/kernel/drivers/bin/3com would help?

[apprentice]

Thanks, I'll try that

[apprentice]

Getting nowhere...

I did a separate ls on every file/directory you listed and they all worked. The ls /dev/dir returned the directory contents of every directory. So the /dev works but I just have to know exactly what device file I'm using since ls /dev always crashes.

BTW, ran into another bug (maybe this is a known issue already):

When running in safe mode and continuosly booting and crashing the system the filesystem got corrupted. The process using all of the cpu time was the syslog daemon. so I tried to cd to /var/log, but couldn't. ls shows that /var/log exists. but it isn't really there since I can't cd to it (I thought that journaling filesystems don't have these kind of problems). Well I've heard of stories of Linux crashing due to power failure, after which XFS and ReiserFS filesystems were so corrupted that they couldn't be recovered and mkfs was the only solution. So is there a fsck.befs anywhere?

I also deleted the 3com driver, but it didn't help. I just don't know how to debug this problem any further. I've tried everything, but it just keeps crashing.

[Adek336]

Thanks for the testing!

If you'd care for a little coding:

fix_dirent() is static in src/system/kernel/fs/vfs.cpp and invoked only from dir_read(), so, out of the top of my head, one would like to add a sanity check like

in dir_read:
+                       if ( ((unsigned)vnode) && 0xffff0000 == 0xbeaf0000)
+                               panic("dir_read: vnode is beafdead");
			error = fix_dirent(vnode, buffer, ioContext, &length);

but a stack corruption might be happening after entering fix_dirent so another thing useful'd be

in dir_read:
+                       dprintf("dir_read: enter fix_dirent, ioContext=%p, "
+                               "vnode=%p, cookie=%p, buffer=%p, bufferSize=0x%x, _count=%p, i=0x%x\n",
+                                   ioContext, vnode, cookie, buffer, (unsigned)bufferSize, _count, i);
+                       if ( ((unsigned)vnode) && 0xffff0000 == 0xbeaf0000)
+                               panic("dir_read: vnode is beafdead");
			error = fix_dirent(vnode, buffer, ioContext, &length);
+                       dprintf("dir_read: leave fix_dirent\n");

[apprentice]

syslog-with-sanity-check

[apprentice]

I applied your sanity check and added the syslog. This is strange. I booted into safe mode and run 'find /dev' and it started to flood my screen with the device files, but it didn't crash... it went into an infinite loop like there was a symlink from somewhere under /dev/tt back to /dev. 'cd /dev'; 'find .' again crashed in safe mode.

[apprentice]

I forgot to mention the most important thing. The stack trace is exactly same as before, so when entering into fix_dirent() it seems that the vnode is valid because it passes the sanity check...

[Adek336]

Actually the sanity check is a most basic one and very weak. The interesting part are the last messages written to the log; however, these don't have the chance to be written back to the disk, as the kernel crashes. The interesting part of the log is what you get when you type "syslog|tail 5" into the debugger.

Also, after you crash, can you continue through by entering "continue", or by entering "suspend" and then "continue" into the debugger? Perhaps by doing so a couple of times? Does that make the panic message change?

[apprentice]

Yes and no. suspend/continue restores the system to a semi-usable state. I can use it like nothing happened as long as I don't touch the crashed "ls" thread. Kernel debugger says that it terminated the ls thread, but it doesn't since running threads still shows the offending process. Shutting the OS cleanly is through 'shutdown -r' that throws me in kernel debugging land from where I can run reboot. The stack trace doesn't seem to be altered in anyway by suspend/continue.

[Adek336]

What does "sys|ta 5" tell you after a crash?

[apprentice]

sys | tail -5

[apprentice]

Added the sys|tail -5

[Adek336]

Thanks for the note!

It would be interesting to see how many lines of fix_dirent() are executed before the crash.

To avoid slowing down the system with the syslog being written to disk, it's better to use ktrace_printf.

It has to be enabled at build time: create a directory build/user_config_headers, copy the file build/config_headers/tracing_config.h to build/user_config_headers and change the line

#  define ENABLE_TRACING 0

to

#  define ENABLE_TRACING 1

. Then you can apply fix_dirent-line-numbers.diff (against src/system/kernel/fs/vfs.cpp) and build.

Let's also use tracing here:

in dir_read:
-                       dprintf("dir_read: enter fix_dirent, ioContext=%p, "
+                       ktrace_printf("dir_read: enter fix_dirent, ioContext=%p, "
                               "vnode=%p, cookie=%p, buffer=%p, bufferSize=0x%x, _count=%p, i=0x%x\n",
                                   ioContext, vnode, cookie, buffer, (unsigned)bufferSize, _count, i);
                       if ( ((unsigned)vnode) && 0xffff0000 == 0xbeaf0000)
                               panic("dir_read: vnode is beafdead");
			error = fix_dirent(vnode, buffer, ioContext, &length);
-                       dprintf("dir_read: leave fix_dirent\n");
+                       ktrace_printf("dir_read: leave fix_dirent\n");

Now with those changes, typing "trac 1" in the debugger gives the first 30 messages and typing "trac" gives the next sets of messages, "trac 0" gives the last 30 messages.

Could you try this changes and see what are the last trace messages? That may shed some light on the problem, thanks for the feedback!

[Adek336]

line numbers tracing in fix_dirent()

[apprentice]

Thanks to you. You have devoted a lot of time in debugging a bug that no one else has been able to reproduce on their hardware.

I'm currently a bit short of time so it might take some time to apply the diff and test it, but rest assured I'll do it as soon as I get a chance.

[apprentice]

I applied the patch and recompiled. Hopefully the image is clear enough.

[apprentice]

Does the trac output give any useful information on the crash whatsoever?

[Adek336]

trace file names; sanity check against buffer overflow

[Adek336]

The trac output gives a pretty clear view of what instructions cause the crash and suggest that "parent" is not a valid pointer at that point; also we see, that the value of parent which is fed into fix_dirent() seems reasonable (in the trac, vnode=0x...). That is a lot of valuable information! For once, it allows me to suspect what the direct cause of the crash is, a buffer overflow during user_memcpy() of d_name; if that is the case, with the new diff you will see a panic "d_reclen>sizeof(buffer)" which you can easily continue through; that would be quite a progress :-)

The new diff also traces the names of files, so we will know the name of the /dev/* file which goes right before the offending file.

The diff is against the trunk version of vfs.cpp, so to revert to that version before applying the patch you may do

svn revert src/system/kernel/fs/vfs.cpp

[apprentice]

Yup! It seems you nailed the bug. I'll run 'trac 0' later to get more informative ouput.

[Adek336]

That's great !

Probably related to #1840.

[Adek336]

Also note that #489 mentions both a 3com ethernet adapter and ls /dev/net problem.

[Adek336]

trace vnodes created in devfs. print name of offending direntry

[Adek336]

Replying to apprentice:

Yup! It seems you nailed the bug.

Not quite, we secured fix_dirent() against a stack overflow but the devfs is still erroneously passing an overgrown dir entry.

Could you send in the last trace messages ("trac 0") and all trace messages with the string devfs_create_vnode (all pages of "trac 1 300 -1 #devfs_create_vnode") after applying devfs_create_vnode.diff.

[apprentice]

I added the trac 0. I'll send the trace messages of the created device nodes as soon as possible. BTW, just out of curiosity, why can the system continue to run after a buffer overflow in kernel space. Doesn't it overwrite the adjacent memory locations or is this secured in the memcpy() implementation?

[Adek336]

The last two patches avoid doing a memcpy when it would overflow the buffer.

Without them, you still can suspend and continue, because AFAIK the overflow damages the kernel stack allocated for the the ls /dev team. Because fix_dirent() dereferences a pointer from the stack right after memcpy is called and crashes, it doesn't have the opportunity to do much harm to other parts of the kernel which it would if it tried to, for example, update vnode lists with corrupt data from the stack.

[apprentice]

So here is all the output from the latest diff. I hope it helps!

[Adek336]

We're lucky!, because the triangles in devfs_create_vnode3.jpg rule out a bug in devfs. They probably come from a device driver. Also note that the triangle is ascii \314, just like in #1840.

[Adek336]

panic_at_vnode_name.diff: should panic at boot time when the corrupt file name is fed into devfs_create_vnode(); additional debug messages.

Particularly interesting are

bt

,

trac 1 300 -1 #republish

which could point at a faulty driver and

trac 0 90

.

[Adek336]

Also to avoid recompiling a lot and reinstalling to the partition, you may do

jam kernel_x86

and copy the file generated/objects/haiku/x86/release/system/kernel/kernel_x86 to /boot/beos/system/, and after that reboot.

[apprentice]

It's the 3com nic after all! The kernel shouldn't even create a device node for it since the driver initialization fails. So I guess the fbsd layer should be looked at very careful.

[Adek336]

Could you see if deleting /boot/beos/system/add-ons/kernel/drivers/dev/net/3com helps ?

[apprentice]

Yes. That does the trick. After deleting the driver and rebooting everything works. Except for the nic of course :-)

[apprentice]

Strange. I have deleted the 3com driver before and it didn't help. And I guess it doesn't get loaded in safe mode and it crashed in safe mode when running find in /dev.

[Adek336]

Are you still able to crash safe mode?

[apprentice]

No. It doesn't crash in safe mode either.

[apprentice]

This bug would have been easy to debug just by removing the nic but I was too short on time to devote enough effort on this. The driver is exactly the same as in FreeBSD and it works there. I wonder what makes it defunct in Haiku.

[Adek336]

Yea, even more interestingly the driver works with my 3com 556b adapter.

Please apply fbsd_compat.diff against a trunk version of the sources, file system modules no longer emit debug messages, the freebsd compat layer now emit in various moments the contents of gDeviceNameList[0] to find out where it gets corrupt. Please also restore the 3com driver.

It should panic at boot, there should be only a handful of trace messages, these are of interest.

[apprentice]

I'll apply your diff as soon as trac starts working again. I have been trying to download the diff for several hours now, but can't download anything. This is a known problem and has a ticket... Trac complains something about IOError: too many open files

[Adek336]

Temporarily ​http://pastebin.com/m166d140c .

[apprentice]

So here are the trace messages after applying your diff.

[Adek336]

Thanks !

I think the problem is as follows. gDeviceNameList[0] is a char pointer that points at a char array inside of ifp: ifp->device_name. ifp is freed in xl_detach after a failure in xl_attach. Thus, gDeviceNameList[0] points at unallocated memory.

We can verify this by looking at the traced contents after applying fbsd_compat.2.diff, backtrace not necessary.

[apprentice]

trac from the latest diff.

[Adek336]

Ok, thanks, so it was the problem I mentioned, I'll invent a fix sometime soon.

[apprentice]

Thanks, acknowledged.

[Adek336]

reproduce.diff: exposes the bug in libfreebsd_network through the 3com driver (even if no 3com adapter is present).

[Adek336]

proposed patch patch.1.diff

• fix_dirent(): assert that the buffer is big enough for the file name; fix an off-by-1 error which would lead to string truncation;

• if_free(): remove entries from gDevices and gDeviceNameList referenced by the ifp that is being freed.

if_index will be uninitialized, if if_initname has not been called, for example as a result of driver failure, so the patch checks for if_index initialization.

Avoid holes in the middle of gDeviceNameList as compat_open relies on that, relocate the last entry of gDeviceNameList in the place of the deleted entry.

_fbsd_uninit_driver will delete the devices which it finds in gDevices. I'm not sure if this is a problem, but if_free now removes the entry from gDevices so probably the device will never get deleted after calling if_free; on the other hand, if the driver doesn't call if_initname, no entry in gDevices is created so the device probably can't be deleted neither.

[apprentice]

Here's the trace output from reproduce.diff

[apprentice]

The patch did the trick. ls /dev doesn't crash anymore and ls /dev/net is empty as it should be. I think this ticket can be closed now.

bye, and thanks for all the fish.

[Adek336]

Thanks for the note !

reproduce.diff is for the general public: it exposes the libfreebsd_network bug on any computer by simulating a failure in xl_attach which happens to only a few people, like you. Haiku with reproduce.diff crashes on the bug regardless if a 3com adapter is present and may be useful for local debugging.

patch.2.diff: "int clear" -> "u_short clear" as that is the type of struct ifnet::if_index.

[Adek336]

Replying to Adek336:

_fbsd_uninit_driver will delete the devices which it finds in gDevices. I'm not sure if this is a problem, but if_free now removes the entry from gDevices so probably the device will never get deleted after calling if_free; on the other hand, if the driver doesn't call if_initname, no entry in gDevices is created so the device probably can't be deleted neither.

Using ld to wrap around malloc,calloc,realloc and free in the driver binary I can see that there are no memory leaks with the 3com+reproduce.diff, that is, after going through probe / try attach / detach-after-failure-in-attach.

That is a rough indication, that my patch doesn't infere with deleting of the devices.

[apprentice]

Should I test patch.2.diff? The former patch worked flawlessly.

[Adek336]

No need to, it's only polished in one place.

[axeld]

Now that has gone a long way :-) Thanks for your persistence and the patch, Adrian! There are some issues with it, though:

• The changes in vfs.cpp are unrelated, and shouldn't be part of the same patch.
• I'm not particularly sure if it's a good idea to change the index of an existing driver, although that pretty much looks like the only solution here. But since the initialization works sequentially, this case should actually never happen - if it does, it probably makes sense to panic.
• Coding style issues: no space before '=' in 'clean=...', use NULL for pointers, not 0.
• 'clear' should be defined inside the if-block, as it's not used outside of it.

[Adek336]

It wouldn't be possible without Apprentice's patience, thanks a lot ! :-)

The detach function calls if_free. Is it possible that detach is called on a driver/device which is not the last on the list? Or is detach only called in reverse sequential order.

[Adek336]

Patches no. 3, if_free() can wipe out only the last entry, hopefully if it breaks for someone it won't do much harm.

[axeld]

Good question - it's probably wrong now. Maybe panicking isn't really a good idea; if this would be a USB driver, you cannot know in which order the devices are removed, anyway.

Maybe it's a better idea to review how the interface index is used, and if it could be changed on-the-fly safely (including potential threading problems), or if that's not possible, to rethink that interface a bit.

If you would like to look into that, great, but if not, just tell, no problem at all :-)

[Adek336]

• the list returned by publish_devices, gDeviceNameList can have entries relocated and isn't indexed by if_index anymore;
  
  
• gDevices list is indexed by if_index and won't have entries relocated, but can have empty holes;
  
  
• like in FreeBSD, if_index is allocated in if_alloc instead of if_initname;
  
  
• only compat_open seems to be relevant to change;
  
  
• works both with my via_rhine and the pseudo-3com driver with reproducible.diff applied;
  
  
• perhaps the looping code should be refactored?
  
  
• in if.c:123, "name" is checked against NULL. Perhaps it should be checked against ""? Or does the device_manager check for such a filename?
  
  

[Adek336]

Also note ​http://www.alvestrand.no/objectid/1.3.6.1.2.1.2.2.1.1.html and ​http://lists.freebsd.org/pipermail/freebsd-net/2004-April/003750.html which suggest that the if_index should not be reused.

[Adek336]

If we decide to reuse if_indices we should at least leave a comment in the code saying that they are supposed to be not reused but are reused.

[Adek336]

Replying to Adek336:

• in if.c:123, "name" is checked against NULL. Perhaps it should be checked against ""? Or does the device_manager check for such a filename?
  
  

vfs.cpp seems to be checking against "". devfs.cpp does not check against "" and shouldn't because it looks like a "" has a special meaning during boot.
So if_initname should check, just like vfs.cpp does.

if.c:
void if_initname(...)
{
int i;

if (name == NULL || name[0] == '\0')
    panic("interface goes unnamed");

also fix typo ("unamed").

[Adek336]

FreeBSD tries to get the lowest free if_index ​http://fxr.watson.org/fxr/source/net/if.c#L399 , NetBSD ​http://fxr.watson.org/fxr/source/net/if.c?v=NETBSD#L437 and Linux ​http://fxr.watson.org/fxr/source/net/core/dev.c?v=linux-2.6#L3608 first try to find an if_index bigger than the last allocated one and only fallback to reusing old if_indices.

I say, let's just make if_alloc try to find the lowest available if_index; that way one doesn't have to bother about resizing gDevices and gDeviceNameList. And also leave a comment about this behaviour in if_alloc.

I am not really sure if if_indices should be system unique or driver unique, it seems that FreeBSD and Haiku go for driver unique whereas NetBSD and Linux for system unique.

[Adek336]

On a related note, in src/libs/compat/freebsd_network/device.c in compat_write()

        //if_printf(ifp, "compat_write(%lld, %p, [%lu])\n", position,
        //      buffer, *numBytes);

-       mb = m_getcl(0, MT_DATA, M_PKTHDR);
+       if (*numBytes > MHLEN)
+               mb = m_getcl(0, MT_DATA, M_PKTHDR);
+       else
+               mb = m_gethdr(0, MT_DATA);
+
        if (mb == NULL)
                return ENOBUFS;

This avoids allocating a mbuf with a cluster when a mbuf without a cluster suffices, this is how it's used in many places in fbsd_mbuf.c. Enjoy!

[Adek336]

This probably affects all 3c900 [vendor 10b7, device 9001] users, so it's worth commiting before the alpha.

[axeld]

Thanks for the patch, applied in hrev29593! Please take a closer look to our coding style, though.