Bug 90687
| Summary: | [patch] side effect of -delete option of find(1) | ||||||
|---|---|---|---|---|---|---|---|
| Product: | Base System | Reporter: | Anatoli Klassen <anatoli> | ||||
| Component: | bin | Assignee: | Andriy Gapon <avg> | ||||
| Status: | Closed FIXED | ||||||
| Severity: | Affects Only Me | ||||||
| Priority: | Normal | ||||||
| Version: | 5.4-RELEASE | ||||||
| Hardware: | Any | ||||||
| OS: | Any | ||||||
| Attachments: |
|
||||||
|
Description
Anatoli Klassen
2005-12-20 15:00:12 UTC
in message <20051220145417.1B52D1DA14@26th.net>, wrote Anatoli Klassen thusly... > > > >Environment: > System: FreeBSD mercury.26th.net 5.4-RELEASE-p6 FreeBSD 5.4-RELEASE-p6 #0: Wed Jul 27 12:58:07 CEST 2005 root@mercury.26th.net:/usr/obj/usr/src/sys/MERCURY i386 > > >Description: > If -delete option is used it cancel -L option silently. So find does > something different as it does with just -print option. The result could be > deleting of all symbolic links instead of broken ones only even if user has > already validated this with previous -print run. > > >How-To-Repeat: > Create a file, symbolic link to it and a broken symbolic link: > touch a && ln -s a b && ln -s c d > > Now detect all broken links: > find -L . -type l > - works fine, shows that "d" is broken. > > Then try to delete the broken links > find -L . -type l -delete > - all links are deleted, not only broken ones. In your example above ... - "b" & "d" are found due to "-type l" criteria, - information about "a" is returned, instead of "b", due to "-L" option - file information about "a" is suppressed as it is not a link, - information about "d" is returned as it is a link and and the linked file does not exist. - in the end, all you have is "d" in the output. Refer to "-L" option description ... -L Cause the file information and file type (see stat(2)) returned for each symbolic link to be those of the file referenced by the link, not the link itself. If the referenced file does not exist, the file information and type will be for the link itself. - Parv -- > In your example above ... > > - "b" & "d" are found due to "-type l" criteria, > - information about "a" is returned, instead of "b", due to "-L" > option > - file information about "a" is suppressed as it is not a link, > - information about "d" is returned as it is a link and and the > linked file does not exist. > - in the end, all you have is "d" in the output. Exactly. But not try to add the -delete option - both "b" and "d" will be deleted, not only "d". On Tue, 20 Dec 2005, Anatoli Klassen wrote:
>
> >How-To-Repeat:
> Create a file, symbolic link to it and a broken symbolic link:
> touch a && ln -s a b && ln -s c d
>
> Now detect all broken links:
> find -L . -type l
> - works fine, shows that "d" is broken.
>
> Then try to delete the broken links
> find -L . -type l -delete
> - all links are deleted, not only broken ones.
I'm also seeing this problem, and I just noticed that this is
given as an example in the find(1) manpage:
EXAMPLES
..
find -L /usr/ports/packages -type l -delete
Delete all broken symbolic links in /usr/ports/packages.
..
--
David Taylor
I just stumbled upon this issue while reading the manpage for find(1). IMHO the fact that -delete turns -follow off is a good thing, so I consider this more of a bug in the manpage. The included patch removes the erroneous example and adds a word or two about -delete not honoring -L. There is a mention of -delete not interacting well with options that alter the traversal algorithm, but that is somewhat vague in this case. --- find.1.diff begins here --- Index: find.1 =================================================================== RCS file: /opt/freebsd/cvs/src/usr.bin/find/find.1,v retrieving revision 1.82 diff -u -r1.82 find.1 --- find.1 28 Feb 2007 10:19:25 -0000 1.82 +++ find.1 4 Sep 2007 02:56:40 -0000 @@ -306,6 +306,24 @@ .Dq Pa \&. for security reasons. Depth-first traversal processing is implied by this option. +Moreover, beware that +.Ic -delete +does +.Ar not +honor the semantics of +.Ic -L +since it turns off following of symbolic links for security reasons. +Thus, +.Pp +.Dl "find -L /usr/ports/packages -type l -delete" +.Pp +would delete +.Ar all +symbolic links under +.Ar /usr/ports/packages +as if +.Ic -L +had not been defined in the command line at all. .It Ic -depth Always true; same as the @@ -843,9 +861,6 @@ Use the .Xr echo 1 command to print out a list of all the files. -.It Li "find -L /usr/ports/packages -type l -delete" -Delete all broken symbolic links in -.Pa /usr/ports/packages . .It Li "find /usr/src -name CVS -prune -o -depth +6 -print" Find files and directories that are at least seven levels deep in the working directory ---- find.1.diff ends here ---- The find manpage says:
> The find utility recursively descends the directory tree for each
> _pathname_ listed, evaluating an _expression_ (composed of the
> ``primaries'' and ``operands'' listed below) in terms of each file in
> the tree.
Most operands return True or False depending on the nature of a found
file (i.e. -nouser: True if the file belongs to an unknown user), other
performs actions (i.e -print: This primary always evaluates to true. It
prints the pathname of the current file to standard output).
The -delete operand also return true all the time. In other word, as
-print or -delete are just two operands that return true, we can expect
an expression using the first or the second act on the same files,
displaying filenames for the first case, and deleting files for the
second.
According to me, silently disabling symbolic links following depending
on witch operands are set looks odd. In fact, if I am not sure of what
I am doing, I may first run find with a -print operand, and if
everything is okay, then replace -print with -delete. It is exactly
what I did after reading the man page example regarding the "Delete all
broken symbolic links in /usr/ports/packages". I got 2-3 broken links
as a result and so confidently replaced -print by -delete to actually do
the job. I basically see all this as a ``for'' loop that behaves
differently depending on what is performed in the without-side-effects
loop block.
Moreover, 1.8 revision (introduction of the -delete operand) commit
message states that "the code is extremely paranoid", consider race
condition, etc. But the behaviour of find is to not traverse symbolic
links if not told otherwise. In other word, it will have the same
behaviour as rm: considering symbolic links, not referenced files, and
does not fills the requirements for a symbolic link race condition. So
maybe the tool may be less paranoid and only the paranoidness of the
user may be taken into account?
Without the restriction told about here, find would be a good tool to
delete particular files according to random criteria (and then the
-delete operand makes sense).
At least, if for ``security reasons'' we don't want the user to use -L
and -delete at the same time, we'd better raise an error and do nothing:
"find: -L and -delete cannot be used simultaneously"
... or even sort of a -shoot-me-in-the-foot option to force find to do
``risky'' stuff rather than doing something the user probably doesn't
wants.
BTW, for those who reached the PR page after deleting a brunch of
symlinks in /usr/ports/packages, you can resurrect them with:
# for PORT in `pkg_info -q -a -o`; do cd /usr/ports/$PORT && \
make package-links; done
Romain
I can still reproduce this issue with 7.1. I think that this is the most dangerous unexpected behavior that I have encountered using FreeBSD so far! -- Andriy Gapon Author: avg Date: Tue May 19 14:23:54 2009 New Revision: 192381 URL: http://svn.freebsd.org/changeset/base/192381 Log: find: do not silently disable -L when -delete is used First of all, current behavior is not documented and confusing, and it can be very dangerous in the following sequence: find -L . -type l find -L . -type l -delete (the second line is even suggested by find(1)). Instead simply refuse to proceed when -L and -delete are both used. A descriptive error message is provided. The following command can be safely used to remove broken links: find -L . -type l -print0 | xargs rm -0 To do: update find(1) PR: bin/90687 Obtained from: Anatoli Klassen <anatoli@aksoft.net> Approved by: jhb (mentor) Modified: head/usr.bin/find/function.c Modified: head/usr.bin/find/function.c ============================================================================== --- head/usr.bin/find/function.c Tue May 19 14:10:14 2009 (r192380) +++ head/usr.bin/find/function.c Tue May 19 14:23:54 2009 (r192381) @@ -427,11 +427,13 @@ f_delete(PLAN *plan __unused, FTSENT *en /* sanity check */ if (isdepth == 0 || /* depth off */ - (ftsoptions & FTS_NOSTAT) || /* not stat()ing */ - !(ftsoptions & FTS_PHYSICAL) || /* physical off */ - (ftsoptions & FTS_LOGICAL)) /* or finally, logical on */ + (ftsoptions & FTS_NOSTAT)) /* not stat()ing */ errx(1, "-delete: insecure options got turned on"); + if (!(ftsoptions & FTS_PHYSICAL) || /* physical off */ + (ftsoptions & FTS_LOGICAL)) /* or finally, logical on */ + errx(1, "-delete: forbidden when symlinks are followed"); + /* Potentially unsafe - do not accept relative paths whatsoever */ if (strchr(entry->fts_accpath, '/') != NULL) errx(1, "-delete: %s: relative path potentially not safe", @@ -462,8 +464,6 @@ c_delete(OPTION *option, char ***argvp _ { ftsoptions &= ~FTS_NOSTAT; /* no optimise */ - ftsoptions |= FTS_PHYSICAL; /* disable -follow */ - ftsoptions &= ~FTS_LOGICAL; /* disable -follow */ isoutput = 1; /* possible output */ isdepth = 1; /* -depth implied */ _______________________________________________ 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" Responsible Changed From-To: freebsd-bugs->avg Work is in-progress by avg (r192381) Responsible Changed From-To: avg->jilles Jilles, you'd better have this. Responsible Changed From-To: jilles->avg This is my first day helping out sorting bugs. Looks like I've got a lot to learn. This patch has already been taken care of by avg. Author: jilles Date: Sun Sep 20 16:47:56 2009 New Revision: 197363 URL: http://svn.freebsd.org/changeset/base/197363 Log: Update find(1) man page for -L/-delete interaction. It is a bit unfortunate that the example to delete broken symlinks now uses rm(1), but allowing this with -delete would require fixing fts(3) to not imply FTS_NOCHDIR if FTS_LOGICAL is given (or hacks in the -delete option). PR: bin/90687 MFC after: 2 weeks Modified: head/usr.bin/find/find.1 Modified: head/usr.bin/find/find.1 ============================================================================== --- head/usr.bin/find/find.1 Sun Sep 20 15:47:31 2009 (r197362) +++ head/usr.bin/find/find.1 Sun Sep 20 16:47:56 2009 (r197363) @@ -312,6 +312,7 @@ character in its pathname relative to .Dq Pa \&. for security reasons. Depth-first traversal processing is implied by this option. +Following symlinks is incompatible with this option. .It Ic -depth Always true; same as the @@ -920,7 +921,7 @@ recent than the current time minus one m Use the .Xr echo 1 command to print out a list of all the files. -.It Li "find -L /usr/ports/packages -type l -delete" +.It Li "find -L /usr/ports/packages -type l -exec rm -- {} +" Delete all broken symbolic links in .Pa /usr/ports/packages . .It Li "find /usr/src -name CVS -prune -o -depth +6 -print" _______________________________________________ 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" Author: jilles Date: Sun Oct 4 17:22:51 2009 New Revision: 197749 URL: http://svn.freebsd.org/changeset/base/197749 Log: MFC r197363: Update find(1) man page for -L/-delete interaction. It is a bit unfortunate that the example to delete broken symlinks now uses rm(1), but allowing this with -delete would require fixing fts(3) to not imply FTS_NOCHDIR if FTS_LOGICAL is given (or hacks in the -delete option). PR: bin/90687 Modified: stable/7/usr.bin/find/ (props changed) stable/7/usr.bin/find/find.1 Modified: stable/7/usr.bin/find/find.1 ============================================================================== --- stable/7/usr.bin/find/find.1 Sun Oct 4 17:16:11 2009 (r197748) +++ stable/7/usr.bin/find/find.1 Sun Oct 4 17:22:51 2009 (r197749) @@ -306,6 +306,7 @@ character in its pathname relative to .Dq Pa \&. for security reasons. Depth-first traversal processing is implied by this option. +Following symlinks is incompatible with this option. .It Ic -depth Always true; same as the @@ -843,7 +844,7 @@ recent than the current time minus one m Use the .Xr echo 1 command to print out a list of all the files. -.It Li "find -L /usr/ports/packages -type l -delete" +.It Li "find -L /usr/ports/packages -type l -exec rm -- {} +" Delete all broken symbolic links in .Pa /usr/ports/packages . .It Li "find /usr/src -name CVS -prune -o -depth +6 -print" _______________________________________________ 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" Author: jilles Date: Thu Oct 29 21:25:16 2009 New Revision: 198628 URL: http://svn.freebsd.org/changeset/base/198628 Log: MFC r197363: Update find(1) man page for -L/-delete interaction. It is a bit unfortunate that the example to delete broken symlinks now uses rm(1), but allowing this with -delete would require fixing fts(3) to not imply FTS_NOCHDIR if FTS_LOGICAL is given (or hacks in the -delete option). PR: bin/90687 Modified: stable/8/usr.bin/find/ (props changed) stable/8/usr.bin/find/find.1 Modified: stable/8/usr.bin/find/find.1 ============================================================================== --- stable/8/usr.bin/find/find.1 Thu Oct 29 21:23:44 2009 (r198627) +++ stable/8/usr.bin/find/find.1 Thu Oct 29 21:25:16 2009 (r198628) @@ -312,6 +312,7 @@ character in its pathname relative to .Dq Pa \&. for security reasons. Depth-first traversal processing is implied by this option. +Following symlinks is incompatible with this option. .It Ic -depth Always true; same as the @@ -920,7 +921,7 @@ recent than the current time minus one m Use the .Xr echo 1 command to print out a list of all the files. -.It Li "find -L /usr/ports/packages -type l -delete" +.It Li "find -L /usr/ports/packages -type l -exec rm -- {} +" Delete all broken symbolic links in .Pa /usr/ports/packages . .It Li "find /usr/src -name CVS -prune -o -depth +6 -print" _______________________________________________ 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 I think that the PR is resolved within the scope of the original report/request. If any further enhancements to find(1) in this area are wanted or proposed, then they should be filed under a new PR. |
