devfs behavior on nested paths is really unintuitive and pretty hard to tune. Suppose I have ZFS zvol under pool/vms/win2000 and I want to unhide node for it in the jail. Here's what it look like unhidden: % find /dev/zvol /dev/zvol /dev/zvol/pool /dev/zvol/pool/vms /dev/zvol/pool/vms/win2000 Now I hide everything and try to unhide desired node (first is ruleset, next `find /dev` output) (1) hide path zvol unhide /dev /dev/zvol (2) hide path zvol unhide path zvol/pool unhide /dev /dev/zvol (3) hide path zvol/pool/vms/win2000 unhide /dev (4) hide path zvol/* unhide /dev (5) hide path zvol unhide path pool unhide /dev /dev/zvol /dev/zvol/pool (6) hide path zvol unhide path pool unhide path vms unhide /dev /dev/zvol /dev/zvol/pool /dev/zvol/pool/vms (7) hide path zvol unhide path pool unhide path vms unhide path win2000 unhide /dev /dev/zvol /dev/zvol/pool /dev/zvol/pool/vms (8) hide path zvol unhide path pool unhide path vms unhide path *win2000 unhide /dev /dev/zvol /dev/zvol/pool /dev/zvol/pool/vms /dev/zvol/pool/vms/win2000 (9) hide path zvol unhide path pool unhide path vms/* unhide path *win2000 unhide /dev /dev/zvol /dev/zvol/pool This seems really broken. First, though man says that path is a glob, and it's used so in /etc/defaults/devfs.rules (add path 'fd/*' unhide) nested directories don't work at all (2,3,4). Specifying them separately works (5,6), but that's not quite good, as rules for different hierarchies may interfere with each other. Also, cases (7,8) is really strange, why do I need * here for a device node?
On 2008-04-17, Dmitry Marakasov wrote: > devfs behavior on nested paths is really unintuitive and pretty hard to tune. I agree that the behavior of rules is confusing with directories. > Suppose I have ZFS zvol under pool/vms/win2000 and I want to unhide node for it in the jail. > > Here's what it look like unhidden: > > % find /dev/zvol > /dev/zvol > /dev/zvol/pool > /dev/zvol/pool/vms > /dev/zvol/pool/vms/win2000 Following rules should do what you want: path zvol/* hide path zvol/pool/vms/win2000 unhide The problem is that for directories and symbolic links, rules match against single component name while for device files rules match against full device path (si_name). This may cause unwanted effects. For example, the rule "devfs fd hide" hides an entry /dev/label/fd/label. -- Jaakko
On 2010-12-08, Jaakko Heinonen wrote: > On 2008-04-17, Dmitry Marakasov wrote: > > devfs behavior on nested paths is really unintuitive and pretty hard to tune. > > The problem is that for directories and symbolic links, rules match > against single component name while for device files rules match against > full device path (si_name). This may cause unwanted effects. For > example, the rule "devfs fd hide" hides an entry /dev/label/fd/label. Here is an experimental patch to change rules to match against full path for directories and symbolic links: http://people.freebsd.org/~jh/patches/devfs-rule-fullpath.diff The problem with this change is that it breaks existing rulesets if someone relies on rules matching against single component of a path. -- Jaakko
Can't believe that we are still where we were more than two years ago... I think that we have to make this change even if it _might_ break some existing rulesets. Rationale: - current behavior is contrary to any documentation - current behavior is contrary to common sense - current behavior is very hard to describe and account for - I presume that very few people actually fully understand the current behavior - I presume that even fewer people made a conscious choice to depend or make use of its non-trivial features of the current behavior So, we should make the behavior of devfs pattern consistent with the documentation and the common sense. In addition to Jaakko's patch I propose that we pass FNM_PATHNAME to fnmatch(9), so that the matching is indeed consistent with glob(3) / shell glob-ing rules for filesystem paths. -- Andriy Gapon
Hi, I've go hit by this behavior of the devfs rule matching and I fully agree to the rationale of Andriy. Can we please agree on going ahead with this? Bye, Alexander. -- http://www.Leidinger.net Alexander @ Leidinger.net: PGP ID = B0063FE7 http://www.FreeBSD.org netchild @ FreeBSD.org : PGP ID = 72077137
Hi, On 2013-05-10, Alexander Leidinger wrote: > > I've go hit by this behavior of the devfs rule matching and I fully > agree to the rationale of Andriy. Can we please agree on going ahead > with this? I believe that Andriy is working to get the change committed. See the thread "big change to devfs rules path matching" on -current. If this is not the case, let me know and I will take care of the patch. -- Jaakko
The latest patch and discussion: http://people.freebsd.org/~jh/patches/devfs-rule-fullpath.3.diff http://lists.freebsd.org/pipermail/freebsd-current/2013-June/042482.html -- Jaakko
Responsible Changed From-To: freebsd-bugs->jh Take.
Author: avg Date: Fri Jul 26 14:25:58 2013 New Revision: 253677 URL: http://svnweb.freebsd.org/changeset/base/253677 Log: make path matching in devfs rules consistent and sane (and safer) Before this change path matching had the following features: - for device nodes the patterns were matched against full path - in the above case '/' in a path could be matched by a wildcard - for directories and links only the last component was matched So, for example, a pattern like 're*' could match the following entries: - re0 device - responder/u0 device - zvol/recpool directory Although it was possible to work around this behavior (once it was spotted and understood), it was very confusing and contrary to documentation. Now we always match a full path for all types of devfs entries (devices, directories, links) and a '/' has to be matched explicitly. This behavior follows the shell globbing rules. This change is originally developed by Jaakko Heinonen. Many thanks! PR: kern/122838 Submitted by: jh MFC after: 4 weeks Modified: head/UPDATING head/sys/fs/devfs/devfs_rule.c Modified: head/UPDATING ============================================================================== --- head/UPDATING Fri Jul 26 14:23:25 2013 (r253676) +++ head/UPDATING Fri Jul 26 14:25:58 2013 (r253677) @@ -64,6 +64,13 @@ NOTE TO PEOPLE WHO THINK THAT FreeBSD 10 even if neither the traced process nor the tracing process had write access to that file. +201306XX: + Behavior of devfs rules path matching has been changed. + Pattern is now always matched against fully qualified devfs + path and slash characters must be explicitly matched by + slashes in pattern (FNM_PATHNAME). Rulesets involving devfs + subdirectories must be reviewed. + 20130615: CVS has been removed from the base system. An exact copy of the code is available from the devel/cvs port. Modified: head/sys/fs/devfs/devfs_rule.c ============================================================================== --- head/sys/fs/devfs/devfs_rule.c Fri Jul 26 14:23:25 2013 (r253676) +++ head/sys/fs/devfs/devfs_rule.c Fri Jul 26 14:25:58 2013 (r253677) @@ -101,7 +101,7 @@ struct devfs_ruleset { static devfs_rid devfs_rid_input(devfs_rid rid, struct devfs_mount *dm); static void devfs_rule_applyde_recursive(struct devfs_krule *dk, - struct devfs_dirent *de); + struct devfs_mount *dm, struct devfs_dirent *de); static void devfs_rule_applydm(struct devfs_krule *dk, struct devfs_mount *dm); static int devfs_rule_autonumber(struct devfs_ruleset *ds, devfs_rnum *rnp); static struct devfs_krule *devfs_rule_byid(devfs_rid rid); @@ -109,13 +109,16 @@ static int devfs_rule_delete(struct dev static struct cdev *devfs_rule_getdev(struct devfs_dirent *de); static int devfs_rule_input(struct devfs_rule *dr, struct devfs_mount *dm); static int devfs_rule_insert(struct devfs_rule *dr); -static int devfs_rule_match(struct devfs_krule *dk, struct devfs_dirent *de); -static int devfs_rule_matchpath(struct devfs_krule *dk, +static int devfs_rule_match(struct devfs_krule *dk, struct devfs_mount *dm, + struct devfs_dirent *de); +static int devfs_rule_matchpath(struct devfs_krule *dk, struct devfs_mount *dm, struct devfs_dirent *de); -static void devfs_rule_run(struct devfs_krule *dk, struct devfs_dirent *de, unsigned depth); +static void devfs_rule_run(struct devfs_krule *dk, struct devfs_mount *dm, + struct devfs_dirent *de, unsigned depth); static void devfs_ruleset_applyde(struct devfs_ruleset *ds, - struct devfs_dirent *de, unsigned depth); + struct devfs_mount *dm, struct devfs_dirent *de, + unsigned depth); static void devfs_ruleset_applydm(struct devfs_ruleset *ds, struct devfs_mount *dm); static struct devfs_ruleset *devfs_ruleset_bynum(devfs_rsnum rsnum); @@ -146,7 +149,7 @@ devfs_rules_apply(struct devfs_mount *dm sx_slock(&sx_rules); ds = devfs_ruleset_bynum(dm->dm_ruleset); KASSERT(ds != NULL, ("mount-point has NULL ruleset")); - devfs_ruleset_applyde(ds, de, devfs_rule_depth); + devfs_ruleset_applyde(ds, dm, de, devfs_rule_depth); sx_sunlock(&sx_rules); } @@ -337,13 +340,14 @@ devfs_rid_input(devfs_rid rid, struct de * XXX: a linear search could be done through the cdev list instead. */ static void -devfs_rule_applyde_recursive(struct devfs_krule *dk, struct devfs_dirent *de) +devfs_rule_applyde_recursive(struct devfs_krule *dk, struct devfs_mount *dm, + struct devfs_dirent *de) { struct devfs_dirent *de2; TAILQ_FOREACH(de2, &de->de_dlist, de_list) - devfs_rule_applyde_recursive(dk, de2); - devfs_rule_run(dk, de, devfs_rule_depth); + devfs_rule_applyde_recursive(dk, dm, de2); + devfs_rule_run(dk, dm, de, devfs_rule_depth); } /* @@ -353,7 +357,7 @@ static void devfs_rule_applydm(struct devfs_krule *dk, struct devfs_mount *dm) { - devfs_rule_applyde_recursive(dk, dm->dm_rootdir); + devfs_rule_applyde_recursive(dk, dm, dm->dm_rootdir); } /* @@ -525,7 +529,8 @@ devfs_rule_insert(struct devfs_rule *dr) * de; 0, otherwise. */ static int -devfs_rule_match(struct devfs_krule *dk, struct devfs_dirent *de) +devfs_rule_match(struct devfs_krule *dk, struct devfs_mount *dm, + struct devfs_dirent *de) { struct devfs_rule *dr = &dk->dk_rule; struct cdev *dev; @@ -558,7 +563,7 @@ devfs_rule_match(struct devfs_krule *dk, dev_relthread(dev, ref); } if (dr->dr_icond & DRC_PATHPTRN) - if (!devfs_rule_matchpath(dk, de)) + if (!devfs_rule_matchpath(dk, dm, de)) return (0); return (1); @@ -568,35 +573,43 @@ devfs_rule_match(struct devfs_krule *dk, * Determine whether dk matches de on account of dr_pathptrn. */ static int -devfs_rule_matchpath(struct devfs_krule *dk, struct devfs_dirent *de) +devfs_rule_matchpath(struct devfs_krule *dk, struct devfs_mount *dm, + struct devfs_dirent *de) { struct devfs_rule *dr = &dk->dk_rule; - char *pname; struct cdev *dev; + int match; + char *pname, *specname; + specname = NULL; dev = devfs_rule_getdev(de); if (dev != NULL) pname = dev->si_name; else if (de->de_dirent->d_type == DT_LNK || - de->de_dirent->d_type == DT_DIR) - pname = de->de_dirent->d_name; - else + (de->de_dirent->d_type == DT_DIR && de != dm->dm_rootdir && + (de->de_flags & (DE_DOT | DE_DOTDOT)) == 0)) { + specname = malloc(SPECNAMELEN + 1, M_TEMP, M_WAITOK); + pname = devfs_fqpn(specname, dm, de, NULL); + } else return (0); - KASSERT(pname != NULL, ("devfs_rule_matchpath: NULL pname")); - return (fnmatch(dr->dr_pathptrn, pname, 0) == 0); + KASSERT(pname != NULL, ("devfs_rule_matchpath: NULL pname")); + match = fnmatch(dr->dr_pathptrn, pname, FNM_PATHNAME) == 0; + free(specname, M_TEMP); + return (match); } /* * Run dk on de. */ static void -devfs_rule_run(struct devfs_krule *dk, struct devfs_dirent *de, unsigned depth) +devfs_rule_run(struct devfs_krule *dk, struct devfs_mount *dm, + struct devfs_dirent *de, unsigned depth) { struct devfs_rule *dr = &dk->dk_rule; struct devfs_ruleset *ds; - if (!devfs_rule_match(dk, de)) + if (!devfs_rule_match(dk, dm, de)) return; if (dr->dr_iacts & DRA_BACTS) { if (dr->dr_bacts & DRB_HIDE) @@ -623,7 +636,7 @@ devfs_rule_run(struct devfs_krule *dk, s if (depth > 0) { ds = devfs_ruleset_bynum(dk->dk_rule.dr_incset); KASSERT(ds != NULL, ("DRA_INCSET but bad dr_incset")); - devfs_ruleset_applyde(ds, de, depth - 1); + devfs_ruleset_applyde(ds, dm, de, depth - 1); } } } @@ -632,12 +645,13 @@ devfs_rule_run(struct devfs_krule *dk, s * Apply all the rules in ds to de. */ static void -devfs_ruleset_applyde(struct devfs_ruleset *ds, struct devfs_dirent *de, unsigned depth) +devfs_ruleset_applyde(struct devfs_ruleset *ds, struct devfs_mount *dm, + struct devfs_dirent *de, unsigned depth) { struct devfs_krule *dk; TAILQ_FOREACH(dk, &ds->ds_rules, dk_list) - devfs_rule_run(dk, de, depth); + devfs_rule_run(dk, dm, de, depth); } /* _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
State Changed From-To: open->closed The patch has been committed to head, stable/9 and stable/8 by avg@. Thanks Andriy!