Bug 1171241
| Summary: | libcman segfaults when /dev/zero does not exist | ||||||
|---|---|---|---|---|---|---|---|
| Product: | Red Hat Enterprise Linux 6 | Reporter: | Cedric Buissart <cbuissar> | ||||
| Component: | cluster | Assignee: | Christine Caulfield <ccaulfie> | ||||
| Status: | CLOSED ERRATA | QA Contact: | cluster-qe <cluster-qe> | ||||
| Severity: | unspecified | Docs Contact: | |||||
| Priority: | unspecified | ||||||
| Version: | 6.7 | CC: | ccaulfie, cluster-maint, ivlnka, rpeterso, teigland | ||||
| Target Milestone: | rc | ||||||
| Target Release: | --- | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| Whiteboard: | |||||||
| Fixed In Version: | Doc Type: | Bug Fix | |||||
| Doc Text: | Story Points: | --- | |||||
| Clone Of: | Environment: | ||||||
| Last Closed: | 2016-05-10 19:05:57 UTC | Type: | Bug | ||||
| Regression: | --- | Mount Type: | --- | ||||
| Documentation: | --- | CRM: | |||||
| Verified Versions: | Category: | --- | |||||
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||
| Cloudforms Team: | --- | Target Upstream Version: | |||||
| Embargoed: | |||||||
| Attachments: |
|
||||||
Good catch! It'll affect any client of cman, including cman_tool - so is dead easy to reproduce.. I prefer putting the fcntl into an else clause:
diff --git a/cman/lib/libcman.c b/cman/lib/libcman.c
index a99f5a0..5c01c94 100644
--- a/cman/lib/libcman.c
+++ b/cman/lib/libcman.c
@@ -328,7 +328,10 @@ static cman_handle_t open_socket(const char *name, int name
h = NULL;
errno = saved_errno;
}
- fcntl(h->zero_fd, F_SETFD, 1); /* Set close-on-exec */
+ else
+ {
+ fcntl(h->zero_fd, F_SETFD, 1); /* Set close-on-exec */
+ }
return (cman_handle_t)h;
}
In fact that patch is already in STABLE3 branch. Created attachment 965865 [details]
Patch that also uses FD_CLOEXEC symbolic name
A more considered patch, that also uses FD_CLOEXEC symbols instead of '1'. Mostly taken from the STABLE3 branch
commit 23bae572bfed8abde2f03b878505e511ae92a43a
Author: Christine Caulfield <ccaulfie>
Date: Fri Nov 20 15:48:26 2015 +0000
libcman: Don't segfault if /dev/zero doesn't exist
Since the problem described in this bug report should be resolved in a recent advisory, it has been closed with a resolution of ERRATA. For information on the advisory, and where to find the updated files, follow the link below. If the solution does not work for you, open a new bug report. https://rhn.redhat.com/errata/RHBA-2016-0729.html |
Description of problem: libcman's opensocket will cause the application to segfault if /dev/zero is missing. Although it is unlikely to happen, it would look cleaner to exit nicely than crashing. Version-Release number of selected component (if applicable): RHEL6 How reproducible: 100% Steps to Reproduce: 1. Get an rgmanager cluster, with a service called foo 2. mv /dev/zero{,-tmp} 3. cman_tool status Actual results: core dumped Expected results: clean exit Additional info: This is a side effect of upstream commit 4f604bd22ed. Code extract from cman/lib/libcman.c : 278 static cman_handle_t open_socket(const char *name, int namelen, void *privdata) 279 { [...] 320 /* Get a handle on /dev/zero too. This is always active so we 321 can return it from cman_get_fd() if we have cached messages */ 322 h->zero_fd = open("/dev/zero", O_RDONLY); 323 if (h->zero_fd < 0) 324 { 325 int saved_errno = errno; 326 close(h->fd); 327 free(h); 328 h = NULL; 329 errno = saved_errno; 330 } 331 fcntl(h->zero_fd, F_SETFD, 1); /* Set close-on-exec */ 332 333 return (cman_handle_t)h; The fcntl() line has been added by commit 4f604bd22ed. But now, if we go through the if() branch, we will inexorably hit a NULL dereference. suggested patch return NULL, as we also do above in the same function in case of error : 328 h = NULL; 329 errno = saved_errno; 330 + return NULL; 330 } Note : * currently untested. * I understand it does not solve any problem. it's just an attempt to avoid a segfault.