Hello All,
We have opened up two separate RedHat bugs for these issues. I added patches to those bugs, but was asked to send the patches here as well since the patches might need to go upstream first.
1) https://bugzilla.redhat.com/show_bug.cgi?id=2224666
We have an application called HPSS that heavily uses libtirpc. When we updated to RHEL8.8 our application started crashing all of a sudden. We believe the change that introduced this problem was 2112116:
2022-08-03 Steve Dickson mailto:[email protected] 1.1.4-8
- rpcb_clnt.c add mechanism to try v2 protocol first (bz 2107650)
- Multithreaded cleanup (bz 2112116)
252 for (cptr = front; cptr != NULL; cptr = cptr->ac_next) {
253 if (!memcmp(cptr->ac_taddr->buf, addr->buf, addr->len)) {
254 /* Unlink from cache. We'll destroy it after releasing the mutex. */
255 if (cptr->ac_uaddr)
256 free(cptr->ac_uaddr);
257 if (prevptr)
258 prevptr->ac_next = cptr->ac_next;
259 else
260 front = cptr->ac_next;
261 cachesize--;
262 break;
263 }
264 prevptr = cptr;
265 }
266
267 mutex_unlock(&rpcbaddr_cache_lock);
268 destroy_addr(cptr);
so we have free'd cptr->ac_uaddr. I believe after that free probably safer to set cptr->ac_uaddr to NULL.
Note that destroy_addr() will also try to free it.
2) https://bugzilla.redhat.com/show_bug.cgi?id=2225226
While inspecting the changes between the versions of libtirpc in question, I noticed a memory leak as well.
/*
+ * Destroys a cached address entry structure.
+ *
+ */
+static void
+destroy_addr(addr)
+ struct address_cache *addr;
+{
+ if (addr == NULL)
+ return;
+ if(addr->ac_host != NULL)
+ free(addr->ac_host);
+ if(addr->ac_netid != NULL)
+ free(addr->ac_netid);
+ if(addr->ac_uaddr != NULL)
+ free(addr->ac_uaddr);
+ if(addr->ac_taddr != NULL) {
+ if(addr->ac_taddr->buf != NULL)
+ free(addr->ac_taddr->buf);
+ }
+ free(addr);
+}
Pretty clear that addr->ac_taddr never was properly free’d. I also verified that with valgrind.
I am happy to add more detail, but hopefully others on this list can access the bugs in question. If not let me know and I can add more detail here if needed. Thanks.
-Herb
Just to be clear...
These patches were not made for the upstream branch (might not apply as cleanly as expected). I applied and tested them against libtirpc-1.1.4-8.el8. I have not gone through the trouble of verifying/testing them against upstream sources. Was just asked to mail these patches here by RH in the bug. Hopefully this is still helpful.
-Herb
> On Jul 27, 2023, at 9:08 AM, Wartens, Herb <[email protected]> wrote:
>
> Hello All,
> We have opened up two separate RedHat bugs for these issues. I added patches to those bugs, but was asked to send the patches here as well since the patches might need to go upstream first.
>
> 1) https://bugzilla.redhat.com/show_bug.cgi?id=2224666
>
> We have an application called HPSS that heavily uses libtirpc. When we updated to RHEL8.8 our application started crashing all of a sudden. We believe the change that introduced this problem was 2112116:
>
> 2022-08-03 Steve Dickson mailto:[email protected] 1.1.4-8
> - rpcb_clnt.c add mechanism to try v2 protocol first (bz 2107650)
> - Multithreaded cleanup (bz 2112116)
>
> 252 for (cptr = front; cptr != NULL; cptr = cptr->ac_next) {
> 253 if (!memcmp(cptr->ac_taddr->buf, addr->buf, addr->len)) {
> 254 /* Unlink from cache. We'll destroy it after releasing the mutex. */
> 255 if (cptr->ac_uaddr)
> 256 free(cptr->ac_uaddr);
> 257 if (prevptr)
> 258 prevptr->ac_next = cptr->ac_next;
> 259 else
> 260 front = cptr->ac_next;
> 261 cachesize--;
> 262 break;
> 263 }
> 264 prevptr = cptr;
> 265 }
> 266
> 267 mutex_unlock(&rpcbaddr_cache_lock);
> 268 destroy_addr(cptr);
>
> so we have free'd cptr->ac_uaddr. I believe after that free probably safer to set cptr->ac_uaddr to NULL.
> Note that destroy_addr() will also try to free it.
>
> 2) https://bugzilla.redhat.com/show_bug.cgi?id=2225226
>
> While inspecting the changes between the versions of libtirpc in question, I noticed a memory leak as well.
>
> /*
> + * Destroys a cached address entry structure.
> + *
> + */
> +static void
> +destroy_addr(addr)
> + struct address_cache *addr;
> +{
> + if (addr == NULL)
> + return;
> + if(addr->ac_host != NULL)
> + free(addr->ac_host);
> + if(addr->ac_netid != NULL)
> + free(addr->ac_netid);
> + if(addr->ac_uaddr != NULL)
> + free(addr->ac_uaddr);
> + if(addr->ac_taddr != NULL) {
> + if(addr->ac_taddr->buf != NULL)
> + free(addr->ac_taddr->buf);
> + }
> + free(addr);
> +}
>
> Pretty clear that addr->ac_taddr never was properly free’d. I also verified that with valgrind.
>
> I am happy to add more detail, but hopefully others on this list can access the bugs in question. If not let me know and I can add more detail here if needed. Thanks.
>
> -Herb
>
> <libtirpc-1.1.4-LLNL-memleak.patch><libtirpc-1.1.4-LLNL-crash.patch>
On 7/27/23 5:48 PM, Wartens, Herb wrote:
>
> Just to be clear...
> These patches were not made for the upstream branch (might not apply as cleanly as expected). I applied and tested them against libtirpc-1.1.4-8.el8. I have not gone through the trouble of verifying/testing them against upstream sources. Was just asked to mail these patches here by RH in the bug. Hopefully this is still helpful.
Thank you! I'm looking into them now...
steved.
>
> -Herb
>
>
>> On Jul 27, 2023, at 9:08 AM, Wartens, Herb <[email protected]> wrote:
>>
>> Hello All,
>> We have opened up two separate RedHat bugs for these issues. I added patches to those bugs, but was asked to send the patches here as well since the patches might need to go upstream first.
>>
>> 1) https://bugzilla.redhat.com/show_bug.cgi?id=2224666
>>
>> We have an application called HPSS that heavily uses libtirpc. When we updated to RHEL8.8 our application started crashing all of a sudden. We believe the change that introduced this problem was 2112116:
>>
>> 2022-08-03 Steve Dickson mailto:[email protected] 1.1.4-8
>> - rpcb_clnt.c add mechanism to try v2 protocol first (bz 2107650)
>> - Multithreaded cleanup (bz 2112116)
>>
>> 252 for (cptr = front; cptr != NULL; cptr = cptr->ac_next) {
>> 253 if (!memcmp(cptr->ac_taddr->buf, addr->buf, addr->len)) {
>> 254 /* Unlink from cache. We'll destroy it after releasing the mutex. */
>> 255 if (cptr->ac_uaddr)
>> 256 free(cptr->ac_uaddr);
>> 257 if (prevptr)
>> 258 prevptr->ac_next = cptr->ac_next;
>> 259 else
>> 260 front = cptr->ac_next;
>> 261 cachesize--;
>> 262 break;
>> 263 }
>> 264 prevptr = cptr;
>> 265 }
>> 266
>> 267 mutex_unlock(&rpcbaddr_cache_lock);
>> 268 destroy_addr(cptr);
>>
>> so we have free'd cptr->ac_uaddr. I believe after that free probably safer to set cptr->ac_uaddr to NULL.
>> Note that destroy_addr() will also try to free it.
>>
>> 2) https://bugzilla.redhat.com/show_bug.cgi?id=2225226
>>
>> While inspecting the changes between the versions of libtirpc in question, I noticed a memory leak as well.
>>
>> /*
>> + * Destroys a cached address entry structure.
>> + *
>> + */
>> +static void
>> +destroy_addr(addr)
>> + struct address_cache *addr;
>> +{
>> + if (addr == NULL)
>> + return;
>> + if(addr->ac_host != NULL)
>> + free(addr->ac_host);
>> + if(addr->ac_netid != NULL)
>> + free(addr->ac_netid);
>> + if(addr->ac_uaddr != NULL)
>> + free(addr->ac_uaddr);
>> + if(addr->ac_taddr != NULL) {
>> + if(addr->ac_taddr->buf != NULL)
>> + free(addr->ac_taddr->buf);
>> + }
>> + free(addr);
>> +}
>>
>> Pretty clear that addr->ac_taddr never was properly free’d. I also verified that with valgrind.
>>
>> I am happy to add more detail, but hopefully others on this list can access the bugs in question. If not let me know and I can add more detail here if needed. Thanks.
>>
>> -Herb
>>
>> <libtirpc-1.1.4-LLNL-memleak.patch><libtirpc-1.1.4-LLNL-crash.patch>
>