[BusyBox] Re: BusyBox 1.00-pre3 - SEGFAULT mount with /etc/filesystems (double fclose)

[Pete Flugstad]

I never saw any response to this, and I've reproduced it on the latest 
uClibc 0.9.21+ root_fs for X86 from uClibc
(http://www.uclibc.org/downloads/root_fs_i386.bz2).

It's 100% repeatable.  Linux kernel is 2.4.23-pre4 (no modules).
Hardware is a VIA Samuel 2 with VIA chipset (VT8601/VT82C686).

Pete Flugstad

PS: /bin/hostname in the dev system above doesn't take -F, which 
causes the call to it in inittab to fail.  Replacing provided 
/bin/hostname with one from Busybox fixes this. :-)

Pete Flugstad wrote:
> If you create a file /etc/filesystems and add a few lines to it, mount 
> is supposed to test these filesystem types first and if they fail, 
> then do the same with /proc/filesystems.
> 
> The code in mount_one looks like this:
> 
>      f = fopen("/etc/filesystems", "r");
>      if (f) {
>        while (fgets(buf, sizeof(buf), f)) {
>            .... process /etc/filesystems,
>            .... setting status to TRUE if one works
>        }
>        fclose(f);
>       }
> 
>       if ((!f || read_proc) && !status) {
>            f = bb_xfopen("/proc/filesystems", "r");
> 
>            while (fgets(buf, sizeof(buf), f) != NULL) {
>                .... process /proc/filesystems,
>                .... setting status to TRUE if one works
>            }
>        }
>        fclose(f);  <<<  THIS ONE
> 
> The problem is if /etc/filesystems exists, and the device is mounted 
> OK using a filesystem type from it, then the 2nd fclose is closing an 
> already closed file.  This is causing a SEGFAULT on my system here 
> (VIA C3 based).
> 
> Also, the return from opening /proc/filesystems is not checked.
> 
> I expect the 2nd block of code should probably look more like:
> 
>       if ((!f || read_proc) && !status) {
>            f = bb_xfopen("/proc/filesystems", "r");
>            if ( f ) {
>                while (fgets(buf, sizeof(buf), f) != NULL) {
>                    .... process /proc/filesystems,
>                    .... setting status to TRUE if one works
>                }
>                fclose(f);
>            }
>        }
> 
> Or something like that.  Wrapping the 2nd while loop with the if ( f )
> { ... } and moving the fclose within that block.
> 
> Hope this makes sense.
> 
> Pete Flugstad
> 
>