>From 8e239cf781baa4d0e42457ff4737b4518db229cb Mon Sep 17 00:00:00 2001
From: Bernhard Voelker
Date: Fri, 2 Aug 2013 12:16:44 +0200
Subject: [PATCH] find: fix fd leak with --execdir option (bug#34976)
Prevent "Failed to save working dir[...]: Too many open files"
error by closing the file descriptor of the working directory.
* find/exec.c (impl_pred_exec): Free the working directory if find
executes the command in the local dir, i.e. if it has been saved
by record_exec_dir(). Re-indent code.
* find/testsuite/sv-34976-execdir-fd-leak.sh: Add test.
* find/testsuite/Makefile.am (test_shell_progs): Mention the test.
* NEWS: Mention the fix.
---
NEWS | 3 +
find/exec.c | 94 +++++++++++++++---------------
find/testsuite/Makefile.am | 9 ++-
find/testsuite/sv-34976-execdir-fd-leak.sh | 76 ++++++++++++++++++++++++
4 files changed, 134 insertions(+), 48 deletions(-)
create mode 100755 find/testsuite/sv-34976-execdir-fd-leak.sh
diff --git a/NEWS b/NEWS
index 4349a21..2fbc5da 100644
--- a/NEWS
+++ b/NEWS
@@ -43,6 +43,9 @@ https://savannah.gnu.org/bugs/?group=findutils:
#36652: Better document that -0/-d turns off the effect of -E.
+#34976: find -execdir leaks file descriptors for the working directory
+
+
* Major changes in release 4.5.11, 2013-02-02
** Documentation Changes
diff --git a/find/exec.c b/find/exec.c
index aa69fe3..a191d36 100644
--- a/find/exec.c
+++ b/find/exec.c
@@ -112,8 +112,8 @@ record_exec_dir (struct exec_val *execp)
bool
impl_pred_exec (const char *pathname,
- struct stat *stat_buf,
- struct predicate *pred_ptr)
+ struct stat *stat_buf,
+ struct predicate *pred_ptr)
{
struct exec_val *execp = &pred_ptr->args.exec_vec;
char *buf = NULL;
@@ -127,36 +127,36 @@ impl_pred_exec (const char *pathname,
if (local)
{
/* For -execdir/-okdir predicates, the parser did not fill in
- the wd_for_exec member of sturct exec_val. So for those
- predicates, we do so now.
+ the wd_for_exec member of sturct exec_val. So for those
+ predicates, we do so now.
*/
if (!record_exec_dir (execp))
- {
- error (EXIT_FAILURE, errno,
- _("Failed to save working directory in order to "
- "run a command on %s"),
- safely_quote_err_filename (0, pathname));
- /*NOTREACHED*/
- }
+ {
+ error (EXIT_FAILURE, errno,
+ _("Failed to save working directory in order to "
+ "run a command on %s"),
+ safely_quote_err_filename (0, pathname));
+ /*NOTREACHED*/
+ }
target = buf = base_name (state.rel_pathname);
if ('/' == target[0])
- {
- /* find / execdir ls -d {} \; */
- prefix = NULL;
- pfxlen = 0;
- }
+ {
+ /* find / execdir ls -d {} \; */
+ prefix = NULL;
+ pfxlen = 0;
+ }
else
- {
- prefix = "./";
- pfxlen = 2u;
- }
+ {
+ prefix = "./";
+ pfxlen = 2u;
+ }
}
else
{
/* For the others (-exec, -ok), the parser should
- have set wd_for_exec to initial_wd, indicating
- that the exec should take place from find's initial
- working directory.
+ have set wd_for_exec to initial_wd, indicating
+ that the exec should take place from find's initial
+ working directory.
*/
assert (execp->wd_for_exec == initial_wd);
target = pathname;
@@ -171,14 +171,14 @@ impl_pred_exec (const char *pathname,
* depending on the command line length limits.
*/
bc_push_arg (&execp->ctl,
- &execp->state,
- target, strlen (target)+1,
- prefix, pfxlen,
- 0);
+ &execp->state,
+ target, strlen (target)+1,
+ prefix, pfxlen,
+ 0);
/* remember that there are pending execdirs. */
if (execp->state.todo)
- state.execdirs_outstanding = true;
+ state.execdirs_outstanding = true;
/* POSIX: If the primary expression is punctuated by a plus
* sign, the primary shall always evaluate as true
@@ -190,29 +190,31 @@ impl_pred_exec (const char *pathname,
int i;
for (i=0; inum_args; ++i)
- {
- bc_do_insert (&execp->ctl,
- &execp->state,
- execp->replace_vec[i],
- strlen (execp->replace_vec[i]),
- prefix, pfxlen,
- target, strlen (target),
- 0);
- }
+ {
+ bc_do_insert (&execp->ctl,
+ &execp->state,
+ execp->replace_vec[i],
+ strlen (execp->replace_vec[i]),
+ prefix, pfxlen,
+ target, strlen (target),
+ 0);
+ }
/* Actually invoke the command. */
bc_do_exec (&execp->ctl, &execp->state);
if (WIFEXITED(execp->last_child_status))
- {
- if (0 == WEXITSTATUS(execp->last_child_status))
- result = true; /* The child succeeded. */
- else
- result = false;
- }
+ {
+ if (0 == WEXITSTATUS(execp->last_child_status))
+ result = true; /* The child succeeded. */
+ else
+ result = false;
+ }
else
- {
- result = false;
- }
+ {
+ result = false;
+ }
+ if (local)
+ free_cwd (execp->wd_for_exec);
}
if (buf)
{
diff --git a/find/testsuite/Makefile.am b/find/testsuite/Makefile.am
index a1e49b8..71c82e3 100644
--- a/find/testsuite/Makefile.am
+++ b/find/testsuite/Makefile.am
@@ -246,8 +246,13 @@ find.posix/user-missing.exp
EXTRA_DIST_GOLDEN = \
test_escapechars.golden
-test_shell_progs = sv-bug-32043.sh test_escapechars.sh test_escape_c.sh \
- test_inode.sh sv-34079.sh
+test_shell_progs = \
+sv-bug-32043.sh \
+test_escapechars.sh \
+test_escape_c.sh \
+test_inode.sh \
+sv-34079.sh \
+sv-34976-execdir-fd-leak.sh
EXTRA_DIST = $(EXTRA_DIST_EXP) $(EXTRA_DIST_XO) $(EXTRA_DIST_GOLDEN) \
$(test_shell_progs) binary_locations.sh
diff --git a/find/testsuite/sv-34976-execdir-fd-leak.sh b/find/testsuite/sv-34976-execdir-fd-leak.sh
new file mode 100755
index 0000000..2d5dace
--- /dev/null
+++ b/find/testsuite/sv-34976-execdir-fd-leak.sh
@@ -0,0 +1,76 @@
+#! /bin/sh
+# Copyright (C) 2013 Free Software Foundation, Inc.
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see .
+#
+
+# This test verifies that find does not leak a file descriptor for the working
+# directory specified by the -execdir option [Savannah bug #34976].
+
+testname="$(basename $0)"
+
+. "${srcdir}"/binary_locations.sh
+
+# Test if restricting the number of file descriptors via ulimit -n works.
+test_ulimit() {
+ n="$1" # number of files
+ l="$2" # limit to use
+ (
+ ulimit -n "$l" || exit 1
+ for i in $(seq $n) ; do
+ printf "exec %d> /dev/null || exit 1\n" $i
+ done | sh ;
+ ) 2>/dev/null
+}
+# Opening 30 files with a limit of 40 should work.
+test_ulimit 30 40 || { echo "SKIP: ulimit does not work" >&2; exit 0 ; }
+# Opening 30 files with a limit of 20 should fail.
+test_ulimit 30 20 && { echo "SKIP: ulimit does not work" >&2; exit 0 ; }
+
+die() {
+ echo "$@" >&2
+ exit 1
+}
+
+# Create test files, each 100 in the directories ".", "one" and "two".
+make_test_data() {
+ d="$1"
+ (
+ cd "$1" || exit 1
+ mkdir one two || exit 1
+ for i in $(seq 0 100) ; do
+ printf "./%03d one/%03d two/%03d " $i $i $i
+ done \
+ | xargs touch || exit 1
+ ) \
+ || die "failed to set up the test in ${outdir}"
+}
+
+outdir="$(mktemp -d)" || die "FAIL: could not create a test files."
+
+# Create some test files.
+make_test_data "${outdir}" || die "FAIL: failed to set up the test in ${outdir}"
+
+fail=0
+for exe in "${ftsfind}" "${oldfind}"; do
+ ( ulimit -n 30 && \
+ ${exe} "${outdir}" -type f -execdir cat '{}' \; >/dev/null; ) \
+ || { \
+ echo "Option -execdir of ${exe} leaks file descriptors" >&2 ; \
+ fail=1 ; \
+ }
+done
+
+rm -rf "${outdir}" || exit 1
+exit $fail
--
1.8.3.1