This service will be undergoing maintenance at 00:00 UTC, 2016-08-01. It is expected to last about 1 hours

Bug 390591

Summary: leaked file descriptor results in selinux denial
Product: [Fedora] Fedora Reporter: Orion Poplawski <orion>
Component: autofsAssignee: Ian Kent <ikent>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: low Docs Contact:
Priority: low    
Version: 9CC: drepper.fsp, dwalsh, ikent, jhutar, jmoyer, roth
Target Milestone: ---Keywords: SELinux
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: 5.0.3-41 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2009-02-24 15:56:35 EST Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Attachments:
Description Flags
Patch to use CLOEXEC flag for open(), pipe() and socket() if possible
none
Updated patch to use CLOEXEC flag for open(), pipe() and socket() if possible
none
setmntent patch
none
Patch to make setmntent(3) use CLOEXEC feature if possible none

Description Orion Poplawski 2007-11-19 11:22:20 EST
Description of problem:

These are infrequent, but I occasionally see:

Nov 18 16:00:27 machias kernel: audit(1195426827.133:12): avc:  denied  { read }
for  pid=22857 comm="umount" path="/proc/2045/mounts" dev=proc ino=1720914
scontext=system_u:system_r:mount_t:s0 tcontext=system_u:system_r:automount_t:s0
tclass=file

Probably a leaked file descriptor.  See bug #219999 for some background.

Version-Release number of selected component (if applicable):
autofs-5.0.1-27
Comment 1 Orion Poplawski 2008-03-04 13:47:59 EST
Still a problem with autofs-5.0.2-27
Comment 2 Carl Roth 2008-06-11 19:50:01 EDT
Still a problem with autofs-5.0.3-15
Comment 3 Carl Roth 2008-06-11 19:52:33 EDT
would a leaked file descriptor result in a 'read' denial, or simply a 'getattr'?
Comment 4 Orion Poplawski 2008-06-25 14:04:20 EDT
(In reply to comment #3)
> would a leaked file descriptor result in a 'read' denial, or simply a 'getattr'?
> 

Depends on how the file is opened.  You'll see "write" if the file is opened for
writing.  And it's not when an actual read or write is done, but when the
process is started I believe and the kernel checks the permissions for any open
files.  One trick is to open files with O_CLOEXEC so they get closed
automatically on exec.
Comment 5 Ulrich Drepper 2008-07-30 12:08:13 EDT
The current rawhide glibc and kernel now have the appropriate support to fix the
problem.  Change all socket calls from something like this

   fd = socket(PF_INET, SOCK_STREAM, 0);
   if (fd >= 0)
     fcntl(fd, F_SETFD, FD_CLOEXEC);

to something like this

#ifdef SOCK_CLOEXEC
  fd = socket(PF_INET, SOCK_STREAM|SOCK_CLOEXEC, 0);
  if (fd < 0 && errno == EINVAL)
#endif
    {
      fd = socket(PF_INET, SOCK_STREAM, 0);
      if (fd >= 0)
        fcntl(fd, F_SETFD, FD_CLOEXEC);
    }

This new functionality sets the close-on-exec flags atomically.  There is no way
for the file descriptor to leak and this error will go away.

If needed, you can also set the O_NONBLOCK flag at the same time.  Just add
|SOCK_NONBLOCK to the call as well.

If you have to use accept, pipe, or some other file descriptor producing
interface, they all have variants which allow setting FD_CLOEXEC, too.
Comment 6 Jan Hutaƙ 2008-10-14 08:42:03 EDT
*** Bug 466746 has been marked as a duplicate of this bug. ***
Comment 7 Bug Zapper 2008-11-26 03:34:36 EST
This message is a reminder that Fedora 8 is nearing its end of life.
Approximately 30 (thirty) days from now Fedora will stop maintaining
and issuing updates for Fedora 8.  It is Fedora's policy to close all
bug reports from releases that are no longer maintained.  At that time
this bug will be closed as WONTFIX if it remains open with a Fedora 
'version' of '8'.

Package Maintainer: If you wish for this bug to remain open because you
plan to fix it in a currently maintained version, simply change the 'version' 
to a later Fedora version prior to Fedora 8's end of life.

Bug Reporter: Thank you for reporting this issue and we are sorry that 
we may not be able to fix it before Fedora 8 is end of life.  If you 
would still like to see this bug fixed and are able to reproduce it 
against a later version of Fedora please change the 'version' of this 
bug to the applicable version.  If you are unable to change the version, 
please add a comment here and someone will do it for you.

Although we aim to fix as many bugs as possible during every release's 
lifetime, sometimes those efforts are overtaken by events.  Often a 
more recent Fedora release includes newer upstream software that fixes 
bugs or makes them obsolete.

The process we are following is described here: 
http://fedoraproject.org/wiki/BugZappers/HouseKeeping
Comment 8 Ian Kent 2009-01-08 00:18:14 EST
Created attachment 328438 [details]
Patch to use CLOEXEC flag for open(), pipe() and socket() if possible

Ulrich, could you have a quick look at this patch please.
Are the conditional compilation macros correct?
Comment 9 Ulrich Drepper 2009-01-08 01:21:36 EST
(In reply to comment #8)
> Ulrich, could you have a quick look at this patch please.
> Are the conditional compilation macros correct?

I assume you checked that you caught all cases.  it's fine to use wrapper functions.  The implementation of the wrapper function is not quite right, though.  The kernel unfortunately doesn't return an error if an unknown flag is passed.

For the open functions you'll have to use something like this:

static int cloexec_works;

static inline int open_fd(const char *path, int flags)
{
	int fd;

#if defined(O_CLOEXEC) && defined(SOCK_CLOEXEC)
	flags |= O_CLOEXEC;
#endif
	fd = open(path, flags);
	if (fd == -1)
		return -1;
	if (cloexec_works == 0) {
		int fl = fcntl(fd, F_GETFD);
		cloexec_works = (fl & FD_CLOEXEC) ? 1 : -1;
	}
	if (cloexec_work > 0)
		return fd;
	fcntl(fd, F_SETFD, FD_CLOEXEC);
	return fd;
}


The pipe and socket code is fine, you'll get error codes there.  But you might want to avoid using the new functionality if after the first call you know that the kernel is too old.  See the cloexec_works variable above.

For the fopen() function, add "e" to the option string.  Then, again, check whether the FD_CLOEXEC flag worked.
Comment 10 Ian Kent 2009-01-08 02:00:11 EST
(In reply to comment #9)
> (In reply to comment #8)
> > Ulrich, could you have a quick look at this patch please.
> > Are the conditional compilation macros correct?
> 
> I assume you checked that you caught all cases.  it's fine to use wrapper

Yeah, think so, for the ones that matter.
I'll be checking again anyway.

> functions.  The implementation of the wrapper function is not quite right,
> though.  The kernel unfortunately doesn't return an error if an unknown flag is
> passed.
> 
> For the open functions you'll have to use something like this:
> 
> static int cloexec_works;

Right, sounds good, I'll fix that.

...

> The pipe and socket code is fine, you'll get error codes there.  But you might
> want to avoid using the new functionality if after the first call you know that
> the kernel is too old.  See the cloexec_works variable above.

Yep.

> 
> For the fopen() function, add "e" to the option string.  Then, again, check
> whether the FD_CLOEXEC flag worked.

Cool, I didn't see the "e" flag when I was looking around.

Thanks for the help.
Ian
Comment 11 Ian Kent 2009-01-13 02:31:29 EST
Created attachment 328842 [details]
Updated patch to use CLOEXEC flag for open(), pipe() and socket() if possible
Comment 12 Orion Poplawski 2009-01-20 16:57:27 EST
Is there a version for F10 with this patch available for testing?
Comment 13 Ian Kent 2009-01-20 20:31:14 EST
(In reply to comment #12)
> Is there a version for F10 with this patch available for testing?

Only in Rawhide so far but it would be good to test on F-10.
I'll add the patch later today. I don't have an F-10 install
so I will need to ask you to check a couple of things to make
sure the new CLOEXEC feature is available with F-10.
Comment 14 Ian Kent 2009-01-22 02:36:12 EST
*** Bug 443321 has been marked as a duplicate of this bug. ***
Comment 15 Ian Kent 2009-01-22 02:40:12 EST
I have built autofs-5.0.3-38 for F-10 and requested it be
pushed to testing. Can you try it out please.

Ulrich, was this included in F-10?
Ian
Comment 16 Ulrich Drepper 2009-01-22 09:30:50 EST
(In reply to comment #15)
> Ulrich, was this included in F-10?

All of the functions to support O_CLOEXEC except accept4() are supported in the kernel and glibc in F10.
Comment 17 Orion Poplawski 2009-02-04 16:09:28 EST
Well, doesn't seem to help.  Running 5.0.3-38.fc10 and still seeing:

Feb  4 01:50:12 orca kernel: type=1400 audit(1233737412.810:39): avc:  denied  { read } for  pid=23237 comm="umount" path="/proc/2169/mounts" dev=proc ino=71812 scontext=system_u:system_r:mount_t:s0 tcontext=system_u:system_r:automount_t:s0 tclass=file

root      2169     1  0 Feb03 ?        00:00:02 automount

Adding Dan Walsh to see if he has any insight.

What is /proc/*/mounts?  Why would one process try to read another ones?
Comment 18 Ulrich Drepper 2009-02-04 16:19:58 EST
(In reply to comment #17)
> Feb  4 01:50:12 orca kernel: type=1400 audit(1233737412.810:39): avc:  denied 
> { read } for  pid=23237 comm="umount" path="/proc/2169/mounts" dev=proc

This only means that Ian overlooked an open() call.  It should be easy to locate, giving that it's the special mounts file.
Comment 19 Orion Poplawski 2009-02-04 16:28:21 EST
Ah, probably more to do with the use of setmntent/getmntent/endmntent.
Comment 20 Ulrich Drepper 2009-02-04 18:32:17 EST
(In reply to comment #19)
> Ah, probably more to do with the use of setmntent/getmntent/endmntent.

This is indeed a possibility.

I should just change glibc to always set the close-on-exec flag.  The FILE stream is supposed to be used only for the *mntent functions and a stream cannot be used across exec.
Comment 21 Ulrich Drepper 2009-02-04 18:35:43 EST
Actually, no, just add "e" to the mode parameter for setmntent().  glibc already handles the rest correctly.
Comment 22 Orion Poplawski 2009-02-04 18:41:16 EST
Created attachment 330940 [details]
setmntent patch

This look right?
Comment 23 Orion Poplawski 2009-02-04 18:44:24 EST
Or does it also need to be wrapped with

#if defined(O_CLOEXEC) && defined(SOCK_CLOEXEC)
        if (cloexec_works != -1) {

like fopen().
Comment 24 Ulrich Drepper 2009-02-04 18:47:38 EST
(In reply to comment #23)
> Or does it also need to be wrapped with
> 
> #if defined(O_CLOEXEC) && defined(SOCK_CLOEXEC)
>         if (cloexec_works != -1) {
> 
> like fopen().

setmtnent() has to be handled exactly like fopen().  That's what's used behind the scene.
Comment 25 Ian Kent 2009-02-04 20:41:38 EST
(In reply to comment #21)
> Actually, no, just add "e" to the mode parameter for setmntent().  glibc
> already handles the rest correctly.

What will happen if this is used on older versions of glibc
that don't understand the "e" option.

IOW, do I need to wrap this so it will be version independent
as this patch will be committed upstream once we sort it out.

Ian
Comment 26 Ian Kent 2009-02-04 20:43:35 EST
(In reply to comment #18)
> (In reply to comment #17)
> > Feb  4 01:50:12 orca kernel: type=1400 audit(1233737412.810:39): avc:  denied 
> > { read } for  pid=23237 comm="umount" path="/proc/2169/mounts" dev=proc
> 
> This only means that Ian overlooked an open() call.  It should be easy to
> locate, giving that it's the special mounts file.

Hahaha, true, I'm Unix challenged and can't even use a simple
find/grep combination, ;)
Comment 27 Ian Kent 2009-02-04 20:48:25 EST
(In reply to comment #25)
> (In reply to comment #21)
> > Actually, no, just add "e" to the mode parameter for setmntent().  glibc
> > already handles the rest correctly.
> 
> What will happen if this is used on older versions of glibc
> that don't understand the "e" option.
> 
> IOW, do I need to wrap this so it will be version independent
> as this patch will be committed upstream once we sort it out.

Oh, hang on, you already say it needs to be done the same way
in as fopen() comment #24. Will do.

Ian
Comment 28 Ian Kent 2009-02-05 02:19:41 EST
Created attachment 330967 [details]
Patch to make setmntent(3) use CLOEXEC feature if possible
Comment 29 Ian Kent 2009-02-05 02:21:27 EST
Built autofs-5.0.3-39 with patch in comment #28 and requested
push to testing. Can we test this please.
Comment 30 Fedora Update System 2009-02-24 15:56:28 EST
autofs-5.0.3-41 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.