* [PATCH 0/2] Double-Free and Memory Leak Found In libtirpc @ 2023-08-01 9:33 Yongcheng Yang 2023-08-01 9:33 ` [PATCH 1/2] rpcb_clnt.c: fix a possible double free Yongcheng Yang 2023-08-01 9:33 ` [PATCH 2/2] rpcb_clnt.c: fix memory leak in destroy_addr Yongcheng Yang 0 siblings, 2 replies; 4+ messages in thread From: Yongcheng Yang @ 2023-08-01 9:33 UTC (permalink / raw) To: Steve Dickson; +Cc: Linux NFS Mailing list, Herb Wartens, Yongcheng Yang Hey, These 2 patches are from "Wartens, Herb" <wartens2@llnl.gov> and I have just tried to make it appliable to the libtirpc upstream version. Please help double check and apply them if there's no problem. Herb Wartens (2): rpcb_clnt.c: fix a possible double free rpcb_clnt.c: fix memory leak in destroy_addr src/rpcb_clnt.c | 32 ++++++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8 deletions(-) -- 2.31.1 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 1/2] rpcb_clnt.c: fix a possible double free 2023-08-01 9:33 [PATCH 0/2] Double-Free and Memory Leak Found In libtirpc Yongcheng Yang @ 2023-08-01 9:33 ` Yongcheng Yang 2023-08-01 9:33 ` [PATCH 2/2] rpcb_clnt.c: fix memory leak in destroy_addr Yongcheng Yang 1 sibling, 0 replies; 4+ messages in thread From: Yongcheng Yang @ 2023-08-01 9:33 UTC (permalink / raw) To: Steve Dickson; +Cc: Linux NFS Mailing list, Herb Wartens, Yongcheng Yang Signed-off-by: Herb Wartens <wartens2@llnl.gov> --- src/rpcb_clnt.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/rpcb_clnt.c b/src/rpcb_clnt.c index d178d86..630f9ad 100644 --- a/src/rpcb_clnt.c +++ b/src/rpcb_clnt.c @@ -252,12 +252,16 @@ delete_cache(addr) for (cptr = front; cptr != NULL; cptr = cptr->ac_next) { if (!memcmp(cptr->ac_taddr->buf, addr->buf, addr->len)) { /* Unlink from cache. We'll destroy it after releasing the mutex. */ - if (cptr->ac_uaddr) + if (cptr->ac_uaddr) { free(cptr->ac_uaddr); - if (prevptr) + cptr->ac_uaddr = NULL; + } + if (prevptr) { prevptr->ac_next = cptr->ac_next; - else + } + else { front = cptr->ac_next; + } cachesize--; break; } -- 2.31.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/2] rpcb_clnt.c: fix memory leak in destroy_addr 2023-08-01 9:33 [PATCH 0/2] Double-Free and Memory Leak Found In libtirpc Yongcheng Yang 2023-08-01 9:33 ` [PATCH 1/2] rpcb_clnt.c: fix a possible double free Yongcheng Yang @ 2023-08-01 9:33 ` Yongcheng Yang 2024-03-15 20:21 ` Wartens, Herb 1 sibling, 1 reply; 4+ messages in thread From: Yongcheng Yang @ 2023-08-01 9:33 UTC (permalink / raw) To: Steve Dickson; +Cc: Linux NFS Mailing list, Herb Wartens, Yongcheng Yang Signed-off-by: Herb Wartens <wartens2@llnl.gov> --- src/rpcb_clnt.c | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/src/rpcb_clnt.c b/src/rpcb_clnt.c index 630f9ad..539b521 100644 --- a/src/rpcb_clnt.c +++ b/src/rpcb_clnt.c @@ -102,19 +102,31 @@ static void destroy_addr(addr) struct address_cache *addr; { - if (addr == NULL) + if (addr == NULL) { return; - if(addr->ac_host != NULL) + } + if(addr->ac_host != NULL) { free(addr->ac_host); - if(addr->ac_netid != NULL) + addr->ac_host = NULL; + } + if(addr->ac_netid != NULL) { free(addr->ac_netid); - if(addr->ac_uaddr != NULL) + addr->ac_netid = NULL; + } + if(addr->ac_uaddr != NULL) { free(addr->ac_uaddr); + addr->ac_uaddr = NULL; + } if(addr->ac_taddr != NULL) { - if(addr->ac_taddr->buf != NULL) + if(addr->ac_taddr->buf != NULL) { free(addr->ac_taddr->buf); + addr->ac_taddr->buf = NULL; + } + free(addr->ac_taddr); + addr->ac_taddr = NULL; } free(addr); + addr = NULL; } /* -- 2.31.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 2/2] rpcb_clnt.c: fix memory leak in destroy_addr 2023-08-01 9:33 ` [PATCH 2/2] rpcb_clnt.c: fix memory leak in destroy_addr Yongcheng Yang @ 2024-03-15 20:21 ` Wartens, Herb 0 siblings, 0 replies; 4+ messages in thread From: Wartens, Herb @ 2024-03-15 20:21 UTC (permalink / raw) To: Yongcheng Yang, Steve Dickson, Woodard, Ben [LLNL Collaborator] Cc: Linux NFS Mailing list Hi All, I wish I had caught this sooner, but it appears that prior to committing the patch I submitted for this memory leak, it was modified and is missing an important part of the fix. Looking at the upstream source code in destroy_addr() we can see: 101 static void 102 destroy_addr(addr) 103 struct address_cache *addr; 104 { 105 if (addr == NULL) 106 return; 107 if (addr->ac_host != NULL) { 108 free(addr->ac_host); 109 addr->ac_host = NULL; 110 } 111 if (addr->ac_netid != NULL) { 112 free(addr->ac_netid); 113 addr->ac_netid = NULL; 114 } 115 if (addr->ac_uaddr != NULL) { 116 free(addr->ac_uaddr); 117 addr->ac_uaddr = NULL; 118 } 119 if (addr->ac_taddr != NULL) { 120 if(addr->ac_taddr->buf != NULL) { 121 free(addr->ac_taddr->buf); 122 addr->ac_taddr->buf = NULL; 123 } 124 addr->ac_taddr = NULL; 125 } 126 free(addr); 127 addr = NULL; 128 } It seems clear that this code is still not freeing addr->ac_taddr. In the patch I originally submitted to this mailing list, I made sure to free that pointer (see below): diff --git a/src/rpcb_clnt.c b/src/rpcb_clnt.c index 630f9ad..539b521 100644 --- a/src/rpcb_clnt.c +++ b/src/rpcb_clnt.c @@ -102,19 +102,31 @@ static void destroy_addr(addr) struct address_cache *addr; { - if (addr == NULL) + if (addr == NULL) { return; - if(addr->ac_host != NULL) + } + if(addr->ac_host != NULL) { free(addr->ac_host); - if(addr->ac_netid != NULL) + addr->ac_host = NULL; + } + if(addr->ac_netid != NULL) { free(addr->ac_netid); - if(addr->ac_uaddr != NULL) + addr->ac_netid = NULL; + } + if(addr->ac_uaddr != NULL) { free(addr->ac_uaddr); + addr->ac_uaddr = NULL; + } if(addr->ac_taddr != NULL) { - if(addr->ac_taddr->buf != NULL) + if(addr->ac_taddr->buf != NULL) { free(addr->ac_taddr->buf); + addr->ac_taddr->buf = NULL; + } + free(addr->ac_taddr); + addr->ac_taddr = NULL; } free(addr); + addr = NULL; } I did not notice that the patch had been modified when it was committed upstream. I wish I had more carefully verified what had been committed back then. Sorry I neglected to do so. We finally got our hands on the updated rpm with the fixes that went in upstream and are still seeing this memory leak. That is how I noticed the problem and investigated the issue once again. I believe the piece of the fix that was dropped from the patch should be: diff --git a/src/rpcb_clnt.c b/src/rpcb_clnt.c index 68fe69a..d909efc 100644 --- a/src/rpcb_clnt.c +++ b/src/rpcb_clnt.c @@ -121,6 +121,7 @@ destroy_addr(addr) free(addr->ac_taddr->buf); addr->ac_taddr->buf = NULL; } + free(addr->ac_taddr); addr->ac_taddr = NULL; } free(addr); Would love to get this addressed and pulled into a RHEL version as soon as possible. -Herb On 8/1/23, 2:34 AM, "Yongcheng Yang" <yoyang@redhat.com <mailto:yoyang@redhat.com>> wrote: Signed-off-by: Herb Wartens <wartens2@llnl.gov <mailto:wartens2@llnl.gov>> --- src/rpcb_clnt.c | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/src/rpcb_clnt.c b/src/rpcb_clnt.c index 630f9ad..539b521 100644 --- a/src/rpcb_clnt.c +++ b/src/rpcb_clnt.c @@ -102,19 +102,31 @@ static void destroy_addr(addr) struct address_cache *addr; { - if (addr == NULL) + if (addr == NULL) { return; - if(addr->ac_host != NULL) + } + if(addr->ac_host != NULL) { free(addr->ac_host); - if(addr->ac_netid != NULL) + addr->ac_host = NULL; + } + if(addr->ac_netid != NULL) { free(addr->ac_netid); - if(addr->ac_uaddr != NULL) + addr->ac_netid = NULL; + } + if(addr->ac_uaddr != NULL) { free(addr->ac_uaddr); + addr->ac_uaddr = NULL; + } if(addr->ac_taddr != NULL) { - if(addr->ac_taddr->buf != NULL) + if(addr->ac_taddr->buf != NULL) { free(addr->ac_taddr->buf); + addr->ac_taddr->buf = NULL; + } + free(addr->ac_taddr); + addr->ac_taddr = NULL; } free(addr); + addr = NULL; } /* -- 2.31.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-03-15 20:39 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-08-01 9:33 [PATCH 0/2] Double-Free and Memory Leak Found In libtirpc Yongcheng Yang 2023-08-01 9:33 ` [PATCH 1/2] rpcb_clnt.c: fix a possible double free Yongcheng Yang 2023-08-01 9:33 ` [PATCH 2/2] rpcb_clnt.c: fix memory leak in destroy_addr Yongcheng Yang 2024-03-15 20:21 ` Wartens, Herb
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox