netfilter buglog - Jun 2012 - [Bug 793] New: ulogd -d does not close all fds

http://bugzilla.netfilter.org/show_bug.cgi?id=793

           Summary: ulogd -d does not close all fds
           Product: ulogd
           Version: SVN (please provide timestamp)
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: normal
          Priority: P2
         Component: ulogd
        AssignedTo: netfilter-buglog at lists.netfilter.org
        ReportedBy: joant at cloudant.com
   Estimated Hours: 0.0


When calling the ulogd init script from another program (e.g. chef), any fds
being held open by the parent (other than stdin/out/err) are held open by
ulogd. In our environment, with flock being used to execute chef, ulogd holds
open the flock file /var/lock/chef-client.

The fix would be to ulogd.c, line 1222ff, something like:
      // close any open file descriptors
      for (i=getdtablesize();i>=0;--i)
        close(i);

or, if POSIX compliance is desired, use sysconf(_SC_OPEN_MAX).

-- 
Configure bugmail: http://bugzilla.netfilter.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
You are watching all bug changes.

http://bugzilla.netfilter.org/show_bug.cgi?id=793

Pablo Neira Ayuso <pablo at netfilter.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |pablo at netfilter.org

--- Comment #1 from Pablo Neira Ayuso <pablo at netfilter.org> 2012-06-29
15:48:23 CEST ---
We can't do such brute force file description closure.

Instead, we have to guess what descriptors are not closed and fix it in the
exit path.

What configuration did you enable for ulogd?

-- 
Configure bugmail: http://bugzilla.netfilter.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
You are watching all bug changes.
You are watching someone on the CC list of the bug.

http://bugzilla.netfilter.org/show_bug.cgi?id=793

--- Comment #2 from Joan Touzet <joant at cloudant.com> 2012-06-29
16:34:43 CEST ---
Why not? When you fork you should not be assuming any other fd is open, nor
carrying it with you. Name one example of where you *don't* want to close a
passed in fd. The code I pasted is from the daemonization of 

In our case, the call stream looks something like:

cron
 \-> /usr/bin/flock --timeout 300 /var/lock/chef-client --command
'/usr/bin/chef-client' >/dev/null 2>&1
   \-> Install /etc/ulogd.conf, contents below
   \-> /usr/sbin/invoke-rc.d ulogd start
       \-> (debian) start-stop-daemon --quiet --start -exec /usr/sbin/ulogd
--
-d
         \-> ulogd continues to hold an fd on /var/lock/chef-client
         \-> chef-client never runs again because the flock is never released

Our ulogd configuration:

#######################################################################
# Cloudant ulogd configuration
# v1.0.0 - 2012-06-01 - joant
#######################################################################
[global]
nlgroup=1
logfile="/var/log/ulog/ulogd.log"
# loglevel: debug(1), info(3), notice(5), error(7) or fatal(8)
loglevel=5
rmem=131071
bufsize=150000
plugin="/usr/lib/ulogd/ulogd_BASE.so"
plugin="/usr/lib/ulogd/ulogd_LOGEMU.so"

[LOGEMU]
file="/var/log/ulog/syslogemu.log"
sync=0


Our workaround: chef-client invokes ulogd this way instead:
   exec 3>&- && /usr/sbin/invoke-rc.d ulogd start

-- 
Configure bugmail: http://bugzilla.netfilter.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
You are watching all bug changes.
You are watching someone on the CC list of the bug.

http://bugzilla.netfilter.org/show_bug.cgi?id=793

--- Comment #3 from Pablo Neira Ayuso <pablo at netfilter.org> 2012-06-29
18:19:22 CEST ---
(In reply to comment #2)
> Why not? When you fork you should not be assuming any other fd is open, nor
> carrying it with you. Name one example of where you *don't* want to
close a
> passed in fd. The code I pasted is from the daemonization of 
the daemonization code for ulogd already closes descriptors 0, 1 and 2 if you
invoke it with -d.
> In our case, the call stream looks something like:
> 
> cron
>  \-> /usr/bin/flock --timeout 300 /var/lock/chef-client --command
> '/usr/bin/chef-client' >/dev/null 2>&1
>    \-> Install /etc/ulogd.conf, contents below
>    \-> /usr/sbin/invoke-rc.d ulogd start
>        \-> (debian) start-stop-daemon --quiet --start -exec
/usr/sbin/ulogd --
> -d
/usr/sbin/ulogd -- -d

Unless I'm missing anything, I think you have to remove that -- before -d,
otherwise, the -d parameter gets ignored.

/usr/sbin/ulogd -d

-- 
Configure bugmail: http://bugzilla.netfilter.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
You are watching all bug changes.
You are watching someone on the CC list of the bug.

http://bugzilla.netfilter.org/show_bug.cgi?id=793

--- Comment #4 from Joan Touzet <joant at cloudant.com> 2012-06-29
18:25:30 CEST ---
That -- tells start-stop-daemon to pass that flag to /usr/sbin/ulogd rather
than consume it itself. ps shows that ulogd is being run with -d:

  root     31873  0.0  0.0  10204   256 ?        Ss   15:03   0:00
/usr/sbin/ulogd -d

Again the problem is that fd 3 is not being closed, not that 0/1/2 are not
being closed. Your fork() is cloning that fd and inappropriately never closing
it.

-- 
Configure bugmail: http://bugzilla.netfilter.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
You are watching all bug changes.
You are watching someone on the CC list of the bug.

http://bugzilla.netfilter.org/show_bug.cgi?id=793

--- Comment #5 from Pablo Neira Ayuso <pablo at netfilter.org> 2012-06-29
18:38:10 CEST ---
(In reply to comment #4)
> That -- tells start-stop-daemon to pass that flag to /usr/sbin/ulogd rather
> than consume it itself. ps shows that ulogd is being run with -d:
> 
>   root     31873  0.0  0.0  10204   256 ?        Ss   15:03   0:00
> /usr/sbin/ulogd -d
> 
> Again the problem is that fd 3 is not being closed, not that 0/1/2 are not
> being closed. Your fork() is cloning that fd and inappropriately never
closing
> it.
OK, then we have to fix ulogd to fork without leaving any fd behind.

I agree that this needs to be fixed but using the brute-force-closing all
descriptors that you propose is not the way to go.

-- 
Configure bugmail: http://bugzilla.netfilter.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
You are watching all bug changes.
You are watching someone on the CC list of the bug.

http://bugzilla.netfilter.org/show_bug.cgi?id=793

Pablo Neira Ayuso <pablo at netfilter.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|netfilter-buglog at lists.netf |pablo at netfilter.org
                   |ilter.org                   |

-- 
Configure bugmail: http://bugzilla.netfilter.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.

http://bugzilla.netfilter.org/show_bug.cgi?id=793

Pablo Neira Ayuso <pablo at netfilter.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED

--- Comment #6 from Pablo Neira Ayuso <pablo at netfilter.org> 2013-03-26
22:57:17 CET ---
File descriptor 3 is used to parse the configuration file according to strace,
which was not appropriately closed. Fixed this bug in commit
3179bd4de89de7c2388849f5bc48e8f5aad9e5b9 available in the git tree.

-- 
Configure bugmail: http://bugzilla.netfilter.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.

http://bugzilla.netfilter.org/show_bug.cgi?id=793

Pablo Neira Ayuso <pablo at netfilter.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
         Resolution|FIXED                       |

--- Comment #7 from Pablo Neira Ayuso <pablo at netfilter.org> 2013-03-27
00:12:07 CET ---
Reopening. Bad patch, my fault. The file descriptor leak is not happening
there.

-- 
Configure bugmail: http://bugzilla.netfilter.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.

http://bugzilla.netfilter.org/show_bug.cgi?id=793

--- Comment #8 from Pablo Neira Ayuso <pablo at netfilter.org> 2013-03-27
00:24:18 CET ---
Created attachment 393
  --> http://bugzilla.netfilter.org/attachment.cgi?id=393
fix file descriptor leak in the exit path of the parent process

With this patch, valgrind reports:

    ==23720== FILE DESCRIPTORS: 3 open at exit.
    ==23720== Open file descriptor 2: /dev/pts/2
    ==23720==    <inherited from parent>
    ==23720=    ==23720== Open file descriptor 1: /dev/pts/2
    ==23720==    <inherited from parent>
    ==23720=    ==23720== Open file descriptor 0: /dev/pts/2
    ==23720==    <inherited from parent>

-- 
Configure bugmail: http://bugzilla.netfilter.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.