Re: Double-Free and Memory Leak Found In libtirpc

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





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 <wartens2@xxxxxxxx> 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:steved@xxxxxxxxxx 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>





[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux