Re: find -delete broken, or just used improperly?

2013-05-24 Thread Jilles Tjoelker
On Tue, May 21, 2013 at 11:06:39AM -0400, John Baldwin wrote:
 On Monday, May 20, 2013 5:47:31 pm Jilles Tjoelker wrote:
  The below patch allows deleting the pathname given to find itself:

  Index: usr.bin/find/function.c
  ===
  --- usr.bin/find/function.c (revision 250661)
  +++ usr.bin/find/function.c (working copy)
  @@ -442,7 +442,8 @@
  errx(1, -delete: forbidden when symlinks are followed);
   
  /* Potentially unsafe - do not accept relative paths whatsoever */
  -   if (strchr(entry-fts_accpath, '/') != NULL)
  +   if (entry-fts_level  FTS_ROOTLEVEL 
  +   strchr(entry-fts_accpath, '/') != NULL)
  errx(1, -delete: %s: relative path potentially not safe,
  entry-fts_accpath);

 I'm curious, how would you instruct a patched find to avoid deleteing
 the /tmp/foo directory (e.g. if you wanted this to be a job that
 pruned empty dirs from /tmp/foo but never pruned the directory
 itself).  Would -mindepth 1 do it?  (Just asking.  I have also found
 this message annoying but most of the jobs I have seen it on probably
 don't want to delete the root path, just descendants.)

-mindepth 1 works, as does cd /tmp/foo  find . -... (-delete silently
ignores . and ..).

-- 
Jilles Tjoelker
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org


Re: find -delete broken, or just used improperly?

2013-05-24 Thread John Baldwin
On Friday, May 24, 2013 6:24:11 am Jilles Tjoelker wrote:
 On Tue, May 21, 2013 at 11:06:39AM -0400, John Baldwin wrote:
  On Monday, May 20, 2013 5:47:31 pm Jilles Tjoelker wrote:
   The below patch allows deleting the pathname given to find itself:
 
   Index: usr.bin/find/function.c
   ===
   --- usr.bin/find/function.c   (revision 250661)
   +++ usr.bin/find/function.c   (working copy)
   @@ -442,7 +442,8 @@
 errx(1, -delete: forbidden when symlinks are followed);

 /* Potentially unsafe - do not accept relative paths whatsoever */
   - if (strchr(entry-fts_accpath, '/') != NULL)
   + if (entry-fts_level  FTS_ROOTLEVEL 
   + strchr(entry-fts_accpath, '/') != NULL)
 errx(1, -delete: %s: relative path potentially not safe,
 entry-fts_accpath);
 
  I'm curious, how would you instruct a patched find to avoid deleteing
  the /tmp/foo directory (e.g. if you wanted this to be a job that
  pruned empty dirs from /tmp/foo but never pruned the directory
  itself).  Would -mindepth 1 do it?  (Just asking.  I have also found
  this message annoying but most of the jobs I have seen it on probably
  don't want to delete the root path, just descendants.)
 
 -mindepth 1 works, as does cd /tmp/foo  find . -... (-delete silently
 ignores . and ..).

Right, my only concern is that this fix will introduce a change in behavior
that I think might be significant, so we should make sure to advertise it
well in UPDATING, etc.

-- 
John Baldwin
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org


Re: find -delete broken, or just used improperly?

2013-05-21 Thread John Baldwin
On Monday, May 20, 2013 5:47:31 pm Jilles Tjoelker wrote:
 On Mon, May 20, 2013 at 03:23:16PM -0400, Kurt Lidl wrote:
  OK, maybe I'm missing something obvious, but...
 
  find(1) says:
 
   -delete
   Delete found files and/or directories.  Always returns true.
   This executes from the current working directory as find 
  recurses
   down the tree.  It will not attempt to delete a filename with a
   ``/'' character in its pathname relative to ``.'' for security
   reasons.  Depth-first traversal processing is implied by this
   option.  Following symlinks is incompatible with this option.
 
  However, it fails even when the path is absolute:
 
  bhyve9# mkdir /tmp/foo
  bhyve9# find /tmp/foo -empty -delete
  find: -delete: /tmp/foo: relative path potentially not safe
 
  Shouldn't this work?
 
 The relative path refers to a pathname that contains a slash other
 than at the beginning or end and may therefore refer to somewhere else
 if a directory is concurrently replaced by a symlink.
 
 When -L is not specified and . can be opened, the fts(3) code
 underlying find(1) is careful to avoid following symlinks or being
 dropped in different locations by moving the directory fts is currently
 traversing. If a problematic concurrent modification is detected, fts
 will not enter the directory or abort. Files found in the search are
 returned via the current working directory and a pathname not containing
 a slash.
 
 For paranoia, find(1) verifies this when -delete is used. However, it is
 too paranoid about the root of the traversal. It is already assumed that
 the initial pathname does not refer to directories or symlinks that
 might be replaced by untrusted users; otherwise, the whole traversal
 would be unsafe. Therefore, it is not necessary to do the check for
 fts_level == FTS_ROOTLEVEL.
 
 The below patch allows deleting the pathname given to find itself:
 
 Index: usr.bin/find/function.c
 ===
 --- usr.bin/find/function.c   (revision 250661)
 +++ usr.bin/find/function.c   (working copy)
 @@ -442,7 +442,8 @@
   errx(1, -delete: forbidden when symlinks are followed);
  
   /* Potentially unsafe - do not accept relative paths whatsoever */
 - if (strchr(entry-fts_accpath, '/') != NULL)
 + if (entry-fts_level  FTS_ROOTLEVEL 
 + strchr(entry-fts_accpath, '/') != NULL)
   errx(1, -delete: %s: relative path potentially not safe,
   entry-fts_accpath);

I'm curious, how would you instruct a patched find to avoid deleteing the
/tmp/foo directory (e.g. if you wanted this to be a job that pruned empty
dirs from /tmp/foo but never pruned the directory itself).  Would -mindepth 1
do it?  (Just asking.  I have also found this message annoying but most of
the jobs I have seen it on probably don't want to delete the root path,
just descendants.)

-- 
John Baldwin
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org


Re: find -delete broken, or just used improperly?

2013-05-21 Thread Kurt Lidl

On 5/21/13 11:06 AM, John Baldwin wrote:

On Monday, May 20, 2013 5:47:31 pm Jilles Tjoelker wrote:

On Mon, May 20, 2013 at 03:23:16PM -0400, Kurt Lidl wrote:

OK, maybe I'm missing something obvious, but...



find(1) says:



  -delete
  Delete found files and/or directories.  Always returns true.
  This executes from the current working directory as find recurses
  down the tree.  It will not attempt to delete a filename with a
  ``/'' character in its pathname relative to ``.'' for security
  reasons.  Depth-first traversal processing is implied by this
  option.  Following symlinks is incompatible with this option.



However, it fails even when the path is absolute:



bhyve9# mkdir /tmp/foo
bhyve9# find /tmp/foo -empty -delete
find: -delete: /tmp/foo: relative path potentially not safe



Shouldn't this work?


The relative path refers to a pathname that contains a slash other
than at the beginning or end and may therefore refer to somewhere else
if a directory is concurrently replaced by a symlink.

When -L is not specified and . can be opened, the fts(3) code
underlying find(1) is careful to avoid following symlinks or being
dropped in different locations by moving the directory fts is currently
traversing. If a problematic concurrent modification is detected, fts
will not enter the directory or abort. Files found in the search are
returned via the current working directory and a pathname not containing
a slash.

For paranoia, find(1) verifies this when -delete is used. However, it is
too paranoid about the root of the traversal. It is already assumed that
the initial pathname does not refer to directories or symlinks that
might be replaced by untrusted users; otherwise, the whole traversal
would be unsafe. Therefore, it is not necessary to do the check for
fts_level == FTS_ROOTLEVEL.

The below patch allows deleting the pathname given to find itself:

Index: usr.bin/find/function.c
===
--- usr.bin/find/function.c (revision 250661)
+++ usr.bin/find/function.c (working copy)
@@ -442,7 +442,8 @@
errx(1, -delete: forbidden when symlinks are followed);

/* Potentially unsafe - do not accept relative paths whatsoever */
-   if (strchr(entry-fts_accpath, '/') != NULL)
+   if (entry-fts_level  FTS_ROOTLEVEL 
+   strchr(entry-fts_accpath, '/') != NULL)
errx(1, -delete: %s: relative path potentially not safe,
entry-fts_accpath);


I'm curious, how would you instruct a patched find to avoid deleteing the
/tmp/foo directory (e.g. if you wanted this to be a job that pruned empty
dirs from /tmp/foo but never pruned the directory itself).  Would -mindepth 1
do it?  (Just asking.  I have also found this message annoying but most of
the jobs I have seen it on probably don't want to delete the root path,
just descendants.)


Using -mindepth 1 does indeed preserve the target directory.
Without it, the target directory is removed if all the children
files/directories are empty.

I've just finished a complete build of a recent stable/9:

make buildworld  make buildkernel  cd release  make release

With WITHOUT_SHAREDOCS= set in my src.conf, and Jilles' patch to
find, and the following patch to Makefile.inc1, it now builds a
release properly.

bhyve9# hg diff Makefile.inc1
diff -r ca8ef48b4ba6 Makefile.inc1
--- a/Makefile.inc1 Thu May 16 10:21:04 2013 -0400
+++ b/Makefile.inc1 Tue May 21 13:58:13 2013 -0400
@@ -754,7 +754,7 @@
${IMAKEENV} rm -rf ${INSTALLTMP}
 .if make(distributeworld)
 .for dist in ${EXTRA_DISTRIBUTIONS}
-   find ${DESTDIR}/${DISTDIR}/${dist} -empty -delete
+   find ${DESTDIR}/${DISTDIR}/${dist} -mindepth 1 -empty -delete
 .endfor
 .if defined(NO_ROOT)
 .for dist in base ${EXTRA_DISTRIBUTIONS}

If one doesn't have the -mindepth 1 in the find command, it will
remove the usr/share/doc directory entirely, which will cause a failure
later in the 'make release' target code...

-Kurt





___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org


find -delete broken, or just used improperly?

2013-05-20 Thread Kurt Lidl
OK, maybe I'm missing something obvious, but...

find(1) says:

 -delete
 Delete found files and/or directories.  Always returns true.
 This executes from the current working directory as find recurses
 down the tree.  It will not attempt to delete a filename with a
 ``/'' character in its pathname relative to ``.'' for security
 reasons.  Depth-first traversal processing is implied by this
 option.  Following symlinks is incompatible with this option.

However, it fails even when the path is absolute:

bhyve9# mkdir /tmp/foo
bhyve9# find /tmp/foo -empty -delete
find: -delete: /tmp/foo: relative path potentially not safe

Shouldn't this work?

I ran into this during a build of stable/9 with WITHOUT_SHAREDOCS
set, which ultimately triggers this bit of /usr/src/Makefile.inc1:

.for dist in ${EXTRA_DISTRIBUTIONS}
find ${DESTDIR}/${DISTDIR}/${dist} -empty -delete
.endfor

The actual observed failure is this:

=== etc/sendmail (distribute)
cd /usr/src/etc/sendmail;  make install -DNO_SUBDIR 
DESTDIR=/usr/obj/usr/src/release/dist/base SHARED=copies
find //usr/obj/usr/src/release/dist/doc -empty -delete
find: -delete: //usr/obj/usr/src/release/dist/doc: relative path potentially 
not safe
*** [distributeworld] Error code 1

Stop in /usr/src.
*** [distributeworld] Error code 1

Stop in /usr/src.
*** [base.txz] Error code 1

Stop in /usr/src/release.
*** [release] Error code 1

Stop in /usr/src/release.


Thanks for any insight.
-Kurt
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org


Re: find -delete broken, or just used improperly?

2013-05-20 Thread Jilles Tjoelker
On Mon, May 20, 2013 at 03:23:16PM -0400, Kurt Lidl wrote:
 OK, maybe I'm missing something obvious, but...

 find(1) says:

  -delete
  Delete found files and/or directories.  Always returns true.
  This executes from the current working directory as find recurses
  down the tree.  It will not attempt to delete a filename with a
  ``/'' character in its pathname relative to ``.'' for security
  reasons.  Depth-first traversal processing is implied by this
  option.  Following symlinks is incompatible with this option.

 However, it fails even when the path is absolute:

 bhyve9# mkdir /tmp/foo
 bhyve9# find /tmp/foo -empty -delete
 find: -delete: /tmp/foo: relative path potentially not safe

 Shouldn't this work?

The relative path refers to a pathname that contains a slash other
than at the beginning or end and may therefore refer to somewhere else
if a directory is concurrently replaced by a symlink.

When -L is not specified and . can be opened, the fts(3) code
underlying find(1) is careful to avoid following symlinks or being
dropped in different locations by moving the directory fts is currently
traversing. If a problematic concurrent modification is detected, fts
will not enter the directory or abort. Files found in the search are
returned via the current working directory and a pathname not containing
a slash.

For paranoia, find(1) verifies this when -delete is used. However, it is
too paranoid about the root of the traversal. It is already assumed that
the initial pathname does not refer to directories or symlinks that
might be replaced by untrusted users; otherwise, the whole traversal
would be unsafe. Therefore, it is not necessary to do the check for
fts_level == FTS_ROOTLEVEL.

The below patch allows deleting the pathname given to find itself:

Index: usr.bin/find/function.c
===
--- usr.bin/find/function.c (revision 250661)
+++ usr.bin/find/function.c (working copy)
@@ -442,7 +442,8 @@
errx(1, -delete: forbidden when symlinks are followed);
 
/* Potentially unsafe - do not accept relative paths whatsoever */
-   if (strchr(entry-fts_accpath, '/') != NULL)
+   if (entry-fts_level  FTS_ROOTLEVEL 
+   strchr(entry-fts_accpath, '/') != NULL)
errx(1, -delete: %s: relative path potentially not safe,
entry-fts_accpath);
 

-- 
Jilles Tjoelker
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org