Red Hat Bugzilla – Bug 1410114
Double cap_free call in ping_common.c
Last modified: 2017-08-01 16:47:16 EDT
Pseudo patch: modify_capability(..) ... if (cap_set_proc(cap_p) < 0) { perror("ping: cap_set_proc"); goto out; } cap_free(cap_p); + cap_p = NULL; rc = 0; out: if (cap_p) cap_free(cap_p); return rc; ...
The cap_free() already does that, plus it has several sanity checks: int cap_free(void *data_p) { if ( !data_p ) return 0; if ( good_cap_t(data_p) ) { data_p = -1 + (__u32 *) data_p; memset(data_p, 0, sizeof(__u32) + sizeof(struct _cap_struct)); free(data_p); data_p = NULL; return 0; } if ( good_cap_string(data_p) ) { int length = strlen(data_p) + sizeof(__u32); data_p = -1 + (__u32 *) data_p; memset(data_p, 0, length); free(data_p); data_p = NULL; return 0; } _cap_debug("don't recognize what we're supposed to liberate"); errno = EINVAL; return -1; }
This issue should not normally happen. I found it by accident while debugging the issue that ended up with filling https://bugzilla.redhat.com/show_bug.cgi?id=1410154 [crash due to not finding libnss_wins.so.2 dependencies in getaddrinfo] and uses ping in one of the steps. I did some tests with ping from rpbmbuild/BUILD/... and the problem still exists; it will read 4 bytes of released memory due to calling cap_free again on a released pointer. first call it will be a good_cap_t(), so, it will change the pointer to 4 bytes behind, where it "hides" the state information, *and* release the pointer, so, some new memory allocation can get that same pointer at some point, and write to it; the second call, will fail check of good_cap_t, but only if the memory was not reallocated and not overwritten, as it is a dangling pointer.
This looks like a bug in libcap to me. Do you have a reproducer?
This is how I found the issue: $ rpmbuild -ba SPECS/iputils.spec [...] $ sudo valgrind BUILD/iputils-s20160308/ping redhat.com ==13693== Invalid read of size 4 ==13693== at 0x4E34779: cap_free (cap_alloc.c:119) ==13693== by 0x405CF6: modify_capability (ping_common.c:166) ==13693== by 0x402AAA: enable_capability_raw (ping.h:271) ==13693== by 0x402AAA: main (ping.c:449) ==13693== Address 0x65526e0 is 0 bytes inside a block of size 36 free'd ==13693== at 0x4C28CDD: free (vg_replace_malloc.c:530) ==13693== by 0x4E347FF: cap_free (cap_alloc.c:122) ==13693== by 0x405D3C: modify_capability (ping_common.c:161) ==13693== by 0x402AAA: enable_capability_raw (ping.h:271) ==13693== by 0x402AAA: main (ping.c:449) ==13693== Block was alloc'd at ==13693== at 0x4C27BE3: malloc (vg_replace_malloc.c:299) ==13693== by 0x4E3481F: cap_init (cap_alloc.c:19) ==13693== by 0x4E34915: cap_get_proc (cap_proc.c:16) ==13693== by 0x405CB0: modify_capability (ping_common.c:138) ==13693== by 0x402AAA: enable_capability_raw (ping.h:271) ==13693== by 0x402AAA: main (ping.c:449) ==13693== ==13693== Invalid read of size 4 ==13693== at 0x4E34779: cap_free (cap_alloc.c:119) ==13693== by 0x405CF6: modify_capability (ping_common.c:166) ==13693== by 0x402AF0: disable_capability_raw (ping.h:272) ==13693== by 0x402AF0: main (ping.c:454) ==13693== Address 0x6552750 is 0 bytes inside a block of size 36 free'd ==13693== at 0x4C28CDD: free (vg_replace_malloc.c:530) ==13693== by 0x4E347FF: cap_free (cap_alloc.c:122) ==13693== by 0x405D3C: modify_capability (ping_common.c:161) ==13693== by 0x402AF0: disable_capability_raw (ping.h:272) ==13693== by 0x402AF0: main (ping.c:454) ==13693== Block was alloc'd at ==13693== at 0x4C27BE3: malloc (vg_replace_malloc.c:299) ==13693== by 0x4E3481F: cap_init (cap_alloc.c:19) ==13693== by 0x4E34915: cap_get_proc (cap_proc.c:16) ==13693== by 0x405CB0: modify_capability (ping_common.c:138) ==13693== by 0x402AF0: disable_capability_raw (ping.h:272) ==13693== by 0x402AF0: main (ping.c:454) ==13693== PING redhat.com (10.4.164.55) 56(84) bytes of data. 64 bytes from redirect-redhat-com.edge.prod.ext.phx2.redhat.com (10.4.164.55): icmp_seq=1 ttl=248 time=180 ms --- redhat.com ping statistics --- 1 packets transmitted, 1 received, 0% packet loss, time 1ms rtt min/avg/max/mdev = 180.740/180.740/180.740/0.000 ms I did the above mostly because I did want to run the full process under valgrind, and as well check loading of shared objects in the getaddrinfo call. It would be a far more serious issue if this were handled after libraries were loaded, as then memory would very likely have been released. It is not a bug in libcap, because the second call to cap_free is passing, again, a pointer it had already released, and the suggested patch just sets the pointer to NULL to remember about it, and prevent the second cap_free call.
https://github.com/iputils/iputils/pull/71
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://access.redhat.com/errata/RHBA-2017:1987