Bug #4602
closed
memory leak in ILB daemon on import-config
100%
Description
Starting ILBD (L3/L4 Integrated Load Balancer daemon) leads to memory leak:
umem_alloc_160 leak: 1 buffer, 160 bytes
ADDR BUFADDR TIMESTAMP THREAD
CACHE LASTLOG CONTENTS
8677670 8676f28 21bc28a68a0604 1
8630010 0 0
libumem.so.1`umem_cache_alloc_debug+0x157
libumem.so.1`umem_cache_alloc+0x157
libumem.so.1`umem_alloc+0xd0
libumem.so.1`malloc+0x2d
libumem.so.1`realloc+0x4f
adjust_srv_info_cmd+0x31
i_update_ksrv_rules+0x57
ilbd_create_rule+0x16b
ilbd_scf_instance_walk_pg+0x18f
ilbd_walk_rule_pgs+0x3d
i_ilbd_read_config+0x54
main_loop+0x59
main+0x197
_start+0x7d
How to reproduce:
1. Run /usr/lib/inet/ilbd with LD_PRELOAD=/usr/lib/libumem.so
2. Run ilbadm import-config <path_to_ilb_config_file>
3. Run ilbadm import-config <path_to_ilb_config_file>
4. Run gcore on ilbd PID, MDB findleaks output:
CACHE LEAKED BUFCTL CALLER
08630010 1 086776e8 adjust_srv_info_cmd+0x31
08630010 6 08677760 adjust_srv_info_cmd+0x31
08630010 1 08677670 adjust_srv_info_cmd+0x31
Updated by Serghei Samsi almost 10 years ago
Fix for adjust_srv_info_cmd leak
(file /illumos-gate/usr/src/cmd/cmd-inet/usr.lib/ilbd/ilbd_rules.c):
/*
* this function adds all servers in srvlist to the kernel(!) rule
* the name of which is passed as argument.
*/
static ilb_status_t
i_update_ksrv_rules(char *name, ilbd_sg_t *sg, ilbd_rule_t *rl)
{
ilb_status_t rc;
ilbd_srv_t *srvp;
ilb_servers_info_cmd_t *kcmd = NULL;
int i;
/*
* If the servergroup doesn't have any servers associated with
* it yet, there's nothing more to do here.
*/
if (sg->isg_srvcount == 0)
return (ILB_STATUS_OK);
/*
* walk the list of servers attached to this SG
*/
srvp = list_head(&sg->isg_srvlist);
for (i = 0; srvp != NULL; srvp = list_next(&sg->isg_srvlist, srvp)) {
rc = adjust_srv_info_cmd(&kcmd, i);
if (rc != ILB_STATUS_OK)
- return (rc);
+ {
+ if (kcmd != NULL) free(kcmd);
+ return (rc);
+ }
ILB_SGSRV_2_KSRV(&srvp->isv_srv, &kcmd->servers[i]);
/*
* "no port" means "copy rule's port" (for kernel rule)
*/
if (kcmd->servers[i].min_port == 0) {
kcmd->servers[i].min_port = rl->irl_minport;
kcmd->servers[i].max_port = rl->irl_maxport;
}
i++;
}
kcmd->cmd = ILB_ADD_SERVERS;
kcmd->num_servers = i;
(void) strlcpy(kcmd->name, name, sizeof (kcmd->name));
rc = do_ioctl(kcmd, 0);
if (rc != ILB_STATUS_OK)
- return (rc);
+ {
+ if (kcmd != NULL) free(kcmd);
+ return (rc);
+ }
for (i = 0; i < kcmd->num_servers; i++) {
int e;
if ((e = kcmd->servers[i].err) != 0) {
logerr("i_update_ksrv_rules "
"ioctl indicates failure: %s", strerror(e));
rc = ilb_map_errno2ilbstat(e);
/*
* if adding even a single server failed, we need to
* roll back the whole wad. We ignore any errors and
* return the one that was returned by the first ioctl.
*/
kcmd->cmd = ILB_DEL_SERVERS;
(void) do_ioctl(kcmd, 0);
+ if (kcmd != NULL) free(kcmd);
return (rc);
}
}
+ if (kcmd != NULL) free(kcmd);
return (ILB_STATUS_OK);
}
Updated by David Höppner almost 10 years ago
You should run cstyle. Also maybe just use a goto label ("rollback_kcmd") like the rest of the code (don't repeat the "free".) Thanks.
Updated by Serghei Samsi almost 10 years ago
Thanks David. I have posted modified version.
Fix for adjust_srv_info_cmd leak
(file /illumos-gate/usr/src/cmd/cmd-inet/usr.lib/ilbd/ilbd_rules.c):
/*
* this function adds all servers in srvlist to the kernel(!) rule
* the name of which is passed as argument.
*/
static ilb_status_t
i_update_ksrv_rules(char *name, ilbd_sg_t *sg, ilbd_rule_t *rl)
{
- ilb_status_t rc;
+ ilb_status_t rc = ILB_STATUS_OK;
ilbd_srv_t *srvp;
ilb_servers_info_cmd_t *kcmd = NULL;
int i;
/*
* If the servergroup doesn't have any servers associated with
* it yet, there's nothing more to do here.
*/
if (sg->isg_srvcount == 0)
- return (ILB_STATUS_OK);
+ goto out;
/*
* walk the list of servers attached to this SG
*/
srvp = list_head(&sg->isg_srvlist);
for (i = 0; srvp != NULL; srvp = list_next(&sg->isg_srvlist, srvp)) {
rc = adjust_srv_info_cmd(&kcmd, i);
if (rc != ILB_STATUS_OK)
- return (rc);
+ goto rollback_kcmd;
ILB_SGSRV_2_KSRV(&srvp->isv_srv, &kcmd->servers[i]);
/*
* "no port" means "copy rule's port" (for kernel rule)
*/
if (kcmd->servers[i].min_port == 0) {
kcmd->servers[i].min_port = rl->irl_minport;
kcmd->servers[i].max_port = rl->irl_maxport;
}
i++;
}
kcmd->cmd = ILB_ADD_SERVERS;
kcmd->num_servers = i;
(void) strlcpy(kcmd->name, name, sizeof (kcmd->name));
rc = do_ioctl(kcmd, 0);
if (rc != ILB_STATUS_OK)
- return (rc);
+ goto rollback_kcmd;
for (i = 0; i < kcmd->num_servers; i++) {
int e;
if ((e = kcmd->servers[i].err) != 0) {
logerr("i_update_ksrv_rules "
"ioctl indicates failure: %s", strerror(e));
rc = ilb_map_errno2ilbstat(e);
/*
* if adding even a single server failed, we need to
* roll back the whole wad. We ignore any errors and
* return the one that was returned by the first ioctl.
*/
kcmd->cmd = ILB_DEL_SERVERS;
(void) do_ioctl(kcmd, 0);
- return (rc);
+ goto rollback_kcmd;
}
}
- return (ILB_STATUS_OK);
+rollback_kcmd:
+ if (kcmd != NULL) {
+ free(kcmd);
+ kcmd = NULL;
+ }
+out:
+ return (rc);
}
Updated by Serghei Samsi over 9 years ago
Updated by Robert Mustacchi over 9 years ago
- Status changed from New to Resolved
- % Done changed from 10 to 100
- Tags deleted (
needs-triage)
Updated by Electric Monk over 9 years ago
git commit 56b8f71e3a910fbd2820f6841b40bfd85f9673c2
Author: Serghei Samsi <sscdvp@gmail.com> 4601 memory leak in ILB daemon on startup 4602 memory leak in ILB daemon on import-config 4668 Memory leak in ilbd' new_req: getpeerucred() allocation isn't released at all Reviewed by: Dan McDonald <danmcd@omniti.com> Reviewed by: Marcel Telka <marcel.telka@nexenta.com> Approved by: Robert Mustacchi <rm@joyent.com>