Bug 124409 - fsck(8) requires exact entry for mountpoints when executing / bug by design in getfsfile(3) in .../lib/libc/gen/fstab.c
Summary: fsck(8) requires exact entry for mountpoints when executing / bug by design i...
Status: Open
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: Unspecified
Hardware: Any Any
: Normal Affects Only Me
Assignee: freebsd-bugs (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-06-09 09:30 UTC by Enji Cooper
Modified: 2017-12-31 22:29 UTC (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Enji Cooper freebsd_committer freebsd_triage 2008-06-09 09:30:02 UTC
Originally thinking that I encountered a minor usage bug, where the following fails:

optimus# fsck /usr/
fsck: Could not determine filesystem type

Even though the following succeeds:

optimus# fsck /usr
** /dev/ad10s1e (NO WRITE)
** Last Mounted on /usr
** Phase 1 - Check Blocks and Sizes
** Phase 2 - Check Pathnames
** Phase 3 - Check Connectivity
** Phase 4 - Check Reference Counts
** Phase 5 - Check Cyl groups
70050 files, 1216169 used, 2713069 free (24693 frags, 336047 blocks, 0.6% fragmentation)

I started digging a bit deeper, only to discover that the problem lies with the fact that there was a corner case not covered in getfsfile(3)...

If specified value for name [the only argument that gets passed into getfsfile(3)] has slash in it and its corresponding entry does not, or vice versa, the function will return NULL as it searches for an exact match to the fstab(5) entry.

This isn't incredibly critical from what I can tell, as it only affects fsck, however, if someone screwed up some scripts which call fsck or fstab, this would become a possible high-risk issue as it would require in-person support to fix fsck issues on boot.

mount(1) for whatever reason isn't affected by this problem (I tried multiple cases to ensure that this was the case).

Also, the manpage for getfsfile(3) should be updated such that implementers are aware that using name == "none" will return the first entry, which may not be the node of interest in fstab. This is another bug-gish thing by design...

I'm going to discuss this on hackers@ a bit as I need to make sure that I'm not going to restrict the search space for the problem, such that functionality gets broken...

Fix: 

Coming soon pending discussion on hackers@
How-To-Repeat: fsck // # should be the same as `fsck /' =)
Comment 1 Garrett Cooper 2008-06-10 02:46:49 UTC
On Mon, Jun 9, 2008 at 5:55 PM, Giorgos Keramidas
<keramida@ceid.upatras.gr> wrote:
> On Tue, 10 Jun 2008 03:53:41 +0300, Giorgos Keramidas <keramida@ceid.upatras.gr> wrote:
>> On Mon, 9 Jun 2008 01:34:19 -0700, "Garrett Cooper" <yanefbsd@gmail.com> wrote:
>>> Hi hackers,
>>>       I have a question, pending a bug found in getfsfile(3) [1].
>>>       Is there any possibility where a mountpoint be any value other
>>> than a directory, a symlink, or "none", i.e. a flat file?
>>> Thanks,
>>> -Garrett
>>>
>>> References:
>>> http://www.freebsd.org/cgi/query-pr.cgi?pr=bin/124409 (not fully in PR
>>> database yet).
>>
>> It looks like could be 'fixed' by using realpath() on its argument.
>> Then this should work fine:
>>
>>     # fsck /usr/
>
> I meant to write "It looks like getfsfile() could be 'fixed'...", but
> the keyboard daemon ate a word there.

Right. I'm just awaiting a quorum decision on hackers@ because I don't
want to break existing functionality over what I think is correct.
-Garrett
Comment 2 Giorgos Keramidas freebsd_committer freebsd_triage 2008-06-22 04:07:37 UTC
On Mon, 16 Jun 2008 20:28:08 -0700, "Garrett Cooper" <yanefbsd@gmail.com> wrote:
> On Tue, Jun 10, 2008 at 2:27 AM, Garrett Cooper <yaneurabeya@gmail.com> wrote:
>> Ok.... it appears I wasn't intelligent enough to post this in the right
>> place last night. Comments please?
>>
>> Hi hackers,
>>
>>      I have a question, pending a bug found in getfsfile(3) [1].
>>
>>      Is there any possibility where a mountpoint be any value other
>>
>> than a directory, a symlink, or "none", i.e. a flat file?
>>
>> Thanks,
>>
>> -Garrett
>>
>> References:
>>
>> http://www.freebsd.org/cgi/query-pr.cgi?pr=bin/124409 (not fully in PR
>>
>> database yet).
>
> Going once, going twice...

Hi Garrett :-)

When I wrote the original comment in that PR I had something like this
in mind:

%%%
*** src.7789d2851415/lib/libc/gen/fstab.c	Sun Jun 22 05:57:09 2008
--- /home/build/src/lib/libc/gen/fstab.c	Sun Jun 22 05:56:48 2008
*************** struct fstab *
*** 236,245 ****
  getfsfile(name)
  	const char *name;
  {
  	if (setfsent())
! 		while (fstabscan())
! 			if (!strcmp(_fs_fstab.fs_file, name))
  				return(&_fs_fstab);
  	return((struct fstab *)NULL);
  }
  
--- 236,256 ----
  getfsfile(name)
  	const char *name;
  {
+ 	char name_path[PATH_MAX];
+ 	char fstab_path[PATH_MAX];
+ 
+ 	if (realpath(name, name_path) == NULL)
+ 		return((struct fstab *)NULL);
  	if (setfsent())
! 		while (fstabscan()) {
! 			if (strcmp(_fs_fstab.fs_file, name) == 0 ||
! 			    strcmp(_fs_fstab.fs_file, name_path) == 0)
! 				return(&_fs_fstab);
! 			if (realpath(_fs_fstab.fs_file, fstab_path) == NULL)
! 				return((struct fstab *)NULL);
! 			if (strcmp(fstab_path, name_path) == 0)
  				return(&_fs_fstab);
+ 		}
  	return((struct fstab *)NULL);
  }
  
%%%

I tried to avoid unnecessary realpath(3) calls as much as possible, but
there is still the possibility for at least N+1 calls, where N is the
number of entries in fstab...

Can you test the above patch, and let me know if it looks ok, if you
have a better fix in the works, etc.?  It seems to pass the bug you
originally reported when I run:

    % env LD_PRELOAD=/home/build/obj/home/build/src/lib/libc/libc.so.7 \
        fsck ///cdrom///
    fsck: exec fsck_cd9660 for /dev/acd0 in /sbin:/usr/sbin: No such file or directory

The failure later is ok, because we don't have fsck_cd9660 for CD-ROMs.
Comment 3 Eitan Adler freebsd_committer freebsd_triage 2017-12-31 08:01:21 UTC
For bugs matching the following criteria:

Status: In Progress Changed: (is less than) 2014-06-01

Reset to default assignee and clear in-progress tags.

Mail being skipped