From nobody@FreeBSD.org Mon Jun 9 08:20:23 2008 Return-Path: Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 3189D1065673 for ; Mon, 9 Jun 2008 08:20:23 +0000 (UTC) (envelope-from nobody@FreeBSD.org) Received: from www.freebsd.org (www.freebsd.org [IPv6:2001:4f8:fff6::21]) by mx1.freebsd.org (Postfix) with ESMTP id 1B8EF8FC24 for ; Mon, 9 Jun 2008 08:20:23 +0000 (UTC) (envelope-from nobody@FreeBSD.org) Received: from www.freebsd.org (localhost [127.0.0.1]) by www.freebsd.org (8.14.2/8.14.2) with ESMTP id m598KN7F053953 for ; Mon, 9 Jun 2008 08:20:23 GMT (envelope-from nobody@www.freebsd.org) Received: (from nobody@localhost) by www.freebsd.org (8.14.2/8.14.1/Submit) id m598KMnm053952; Mon, 9 Jun 2008 08:20:22 GMT (envelope-from nobody) Message-Id: <200806090820.m598KMnm053952@www.freebsd.org> Date: Mon, 9 Jun 2008 08:20:22 GMT From: Garrett Cooper To: freebsd-gnats-submit@FreeBSD.org Subject: fsck(1) requires exact entry for mountpoints when executing / bug by design in getfsfile(3) in .../lib/libc/gen/fstab.c X-Send-Pr-Version: www-3.1 X-GNATS-Notify: >Number: 124409 >Category: bin >Synopsis: fsck(8) requires exact entry for mountpoints when executing / bug by design in getfsfile(3) in .../lib/libc/gen/fstab.c >Confidential: no >Severity: serious >Priority: medium >Responsible: freebsd-bugs >State: open >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Mon Jun 09 08:30:02 UTC 2008 >Closed-Date: >Last-Modified: Sun Jun 22 03:10:01 UTC 2008 >Originator: Garrett Cooper >Release: 8-CURRENT >Organization: n/a >Environment: FreeBSD optimus 8.0-CURRENT FreeBSD 8.0-CURRENT #0: Fri Jun 6 01:36:23 PDT 2008 gcooper@optimus:/usr/obj/usr/src/sys/OPTIMUS i386 >Description: 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... >How-To-Repeat: fsck // # should be the same as `fsck /' =) >Fix: Coming soon pending discussion on hackers@ >Release-Note: >Audit-Trail: From: "Garrett Cooper" To: bug-followup@freebsd.org Cc: Subject: Re: bin/124409: Bug by design in getfsfile(3) / needed sanity check for mountpoints Date: Mon, 9 Jun 2008 18:46:49 -0700 On Mon, Jun 9, 2008 at 5:55 PM, Giorgos Keramidas wrote: > On Tue, 10 Jun 2008 03:53:41 +0300, Giorgos Keramidas wrote: >> On Mon, 9 Jun 2008 01:34:19 -0700, "Garrett Cooper" 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 From: Giorgos Keramidas To: "Garrett Cooper" Cc: bug-followup@freebsd.org Subject: Re: bin/124409: fsck(8) requires exact entry for mountpoints when executing / bug by design in getfsfile(3) in .../lib/libc/gen/fstab.c Date: Sun, 22 Jun 2008 06:07:37 +0300 On Mon, 16 Jun 2008 20:28:08 -0700, "Garrett Cooper" wrote: > On Tue, Jun 10, 2008 at 2:27 AM, Garrett Cooper 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. >Unformatted: