Bug 467381
Summary: | aiccu leaves file descriptor open on exec(), selinux reports problem | ||||||
---|---|---|---|---|---|---|---|
Product: | [Fedora] Fedora EPEL | Reporter: | Hugo van der Kooij <hvdkooij> | ||||
Component: | aiccu | Assignee: | Matt Domsch <matt_domsch> | ||||
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> | ||||
Severity: | high | Docs Contact: | |||||
Priority: | medium | ||||||
Version: | el5 | CC: | david.moore, drepper, dwalsh, matt_domsch | ||||
Target Milestone: | --- | ||||||
Target Release: | --- | ||||||
Hardware: | x86_64 | ||||||
OS: | Linux | ||||||
URL: | https://www.sixxs.net/forum/?msg=setup-827653 | ||||||
Whiteboard: | |||||||
Fixed In Version: | Doc Type: | Bug Fix | |||||
Doc Text: | Story Points: | --- | |||||
Clone Of: | Environment: | ||||||
Last Closed: | 2008-11-02 06:00:36 UTC | Type: | --- | ||||
Regression: | --- | Mount Type: | --- | ||||
Documentation: | --- | CRM: | |||||
Verified Versions: | Category: | --- | |||||
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||
Cloudforms Team: | --- | Target Upstream Version: | |||||
Embargoed: | |||||||
Attachments: |
|
Description
Hugo van der Kooij
2008-10-17 08:51:16 UTC
The generated local.te file reads as: module local 1.0; require { type ifconfig_t; type initrc_tmp_t; type rhgb_t; type initrc_t; class udp_socket { read write }; class dir search; } #============= ifconfig_t ============== allow ifconfig_t initrc_t:udp_socket { read write }; #============= rhgb_t ============== allow rhgb_t initrc_tmp_t:dir search; This local policy works but may contain unneeded bits. Dan, what's the best approach here? aiccu is in EPEL, but not RHEL. Thanks, Matt This is a leaked file descriptor, in aiccu, You need to close all your open file descriptors before execing other applications. fcntl(fd, F_SETFD, FD_CLOEXEC) This is not an SELinux bug. -----Original Message----- From: Jeroen Massar [jeroen] Sent: Friday, October 17, 2008 8:52 AM To: Domsch, Matt Cc: info Subject: Re: FW: [Bug 467381] aiccu package does not contain required selinux policy Matt_Domsch wrote: > FYI, most likely the logfile descriptor is being left open. We don't close them indeed, and I am fairly sure that a lot of other programs don't do that either. It is not a 'leak' though as the called program terminates almost directly anyway. I've added a note for the next version which we are working on, thus it will be included there, it is a bit stalled though because of non-existing connectivity, but with some luck that might get arranged soon and then a new AICCU will arise. Greets, Jeroen Getting a aiccu policy into F10 would be great. Yes I know a lot of programs do not close file descriptors on exec, the problem happens when you exec a confined domain, selinux will see the descriptor, close it and report it as a potential problem. if I can patch aiccu to to the fcntl(), I will. I need to get a new tunnel from sixxs.net as my current tunnels point as POPs which are offline. Then I can test. --- aiccu.orig/common/tun.c 2008-10-17 18:43:13.000000000 -0500 +++ aiccu/common/tun.c 2008-10-17 18:52:24.000000000 -0500 @@ -696,6 +696,8 @@ bool tun_start(struct tun_reader *tun) /* Create a new tap device */ tun_fd = open("/dev/net/tun", O_RDWR); + if (tun_fd >= 0) + fcntl(tun_fd, F_SETFD, FD_CLOEXEC); if (tun_fd == -1) { tun_log(LOG_ERR, "start", "Couldn't open device %s: %s (%d)\n", "/dev/net/tun", strerror(errno), errno); @@ -725,6 +727,8 @@ bool tun_start(struct tun_reader *tun) tun_log(LOG_DEBUG, "start", "Trying Configured TUN/TAP interface %s...\n", g_aiccu->ipv6_interface); snprintf(buf, sizeof(buf), "/dev/%s", g_aiccu->ipv6_interface); tun_fd = open(buf, O_RDWR); + if (tun_fd >= 0) + fcntl(tun_fd, F_SETFD, FD_CLOEXEC); if (tun_fd < 0) { /* Fall back to trying all /dev/tun* devices */ @@ -735,6 +739,7 @@ bool tun_start(struct tun_reader *tun) tun_fd = open(buf, O_RDWR); if (tun_fd >= 0) { + fcntl(tun_fd, F_SETFD, FD_CLOEXEC); /* Copy over the name of the interface so that configging goes okay */ if (g_aiccu->ipv6_interface) free(g_aiccu->ipv6_interface); snprintf(buf, sizeof(buf), "tun%u", i); this patch won't work. This is going to be a pain to fix. There are 2 cases of forks(). 1) this thing daemonizes itself. So we need the logfile and tunnel fds left open in the daemon child. So we can't FD_CLOEXEC them. 2) a pile of calls to aiccu_exec() which calls system() to invoke scripts like /sbin/ip, /sbin/ifconfig, /sbin/route, and the like. For these, we would like all the file descriptors closed before invoking system(). We don't want them closed in the parent (these are called before it daemonizes). So to fix it right, aiccu_exec() is going to have to fork from the parent, close all file descriptors in the child, then invoke system(). Created attachment 320736 [details]
aiccu-close-before-system.patch
possible patch. This makes startup much slower, as it repeatedly closes file descriptors. Might not need to close nearly this many (~4k per invocation), if I knew how many fds could possibly be open (probably at most ten).
If you closeonexec the file descriptors when you create them, they will stay open over the fork and both processes will be able to use them. They will not be open when you exec or system programs, Which I think is what you want. (In reply to comment #9) > Created an attachment (id=320736) [details] > aiccu-close-before-system.patch > > possible patch. This makes startup much slower, as it repeatedly closes file > descriptors. Might not need to close nearly this many (~4k per invocation), if > I knew how many fds could possibly be open (probably at most ten). People have to stop spreading horrible code like this. If you have to loop then do this: void closeall (void) { DIR *dir = opendir ("/proc/self/fd"); if (dir != NULL) { struct dirent *d; while ((d = readdir (dir)) != NULL) { if (d->d_name[0] == '.') continue; char *cp; long int n = strtol (d->d_name, &cp, 10); if (*cp != '\0' || n <= 2 || n == dirfd (dir)) continue; close (n); } return; } int nmax = sysconf (_SC_OPEN_MAX); for (int n = 3; n < nmax; ++n) close (n); } Dan, is there a way to see which file descriptors are being leaked during the system() calls? selinux reports the leaks, but AFAICT, not which ones. I've got a simple patch that does FD_CLOEXEC which I'll try out, but want to be sure I'm catching all fds. You're right, I was confusing fork() with fork()+exec(). http://domsch.com/linux/fedora/aiccu-2007.01.15-5.fc10.src.rpm has my patch which does FD_CLOEXEC wherever I caught them. Hugo, can you build and try this on your system, to see that selinux no longer reports problems, or if so, the messages? Thanks, Matt Sorry can't do it. SELinux is only going to report the leaks that the new confined domain is not allowed access to. http://koji.fedoraproject.org/koji/taskinfo?taskID=910811 building now, using FD_CLOEXEC. Hugo, can you download this build and try it, to see that it resolves your issue? Thanks, Matt This no longer fails for me on rawhide. I've built this for Fedora 8, 9, rawhide, and EL-5 (branches with selinux). aiccu-2007.01.15-5.fc9 has been submitted as an update for Fedora 9. http://admin.fedoraproject.org/updates/aiccu-2007.01.15-5.fc9 aiccu-2007.01.15-5.fc8 has been submitted as an update for Fedora 8. http://admin.fedoraproject.org/updates/aiccu-2007.01.15-5.fc8 aiccu-2007.01.15-5.fc9 has been pushed to the Fedora 9 stable repository. If problems still persist, please make note of it in this bug report. aiccu-2007.01.15-5.fc8 has been pushed to the Fedora 8 stable repository. If problems still persist, please make note of it in this bug report. Any reason this hasn't been pushed to Fedora 10? aiccu-2007.01.15-5.fc10 has been submitted as an update for Fedora 10. http://admin.fedoraproject.org/updates/aiccu-2007.01.15-5.fc10 David, thanks for the reminder. I had fixed it after F10 package freeze, but forgot to issue the update. I have done so now, it should be in testing with the next push. aiccu-2007.01.15-5.fc10 has been pushed to the Fedora 10 stable repository. If problems still persist, please make note of it in this bug report. |