lxc-user-nic: fix memleak

get_new_nicname() calls lxc_mkifname() which allocates memory and returns it to the caller. The way get_new_nicname() and get_nic_if_avail() were implemented they hid that fact by returning a boolean. That doesn't make sense. Let's rather have them return a pointer to the allocated nic name which the caller needs to free. Signed-off-by: 's avatarChristian Brauner <christian.brauner@ubuntu.com>
parent d04e77c3
...@@ -510,25 +510,29 @@ out_del: ...@@ -510,25 +510,29 @@ out_del:
return false; return false;
} }
/* /* get_new_nicname() will return the name (vethXXXXXX) which is attached on the
* Get a new nic. * host to the lxc bridge. The returned string must be freed by caller.
* *dest will contain the name (vethXXXXXX) which is attached
* on the host to the lxc bridge
*/ */
static bool get_new_nicname(char **dest, char *br, int pid, char **cnic) static char *get_new_nicname(char *br, int pid, char **cnic)
{ {
int ret; int ret;
char *nicname;
char template[IFNAMSIZ]; char template[IFNAMSIZ];
ret = snprintf(template, sizeof(template), "vethXXXXXX"); ret = snprintf(template, sizeof(template), "vethXXXXXX");
if (ret < 0 || (size_t)ret >= sizeof(template)) if (ret < 0 || (size_t)ret >= sizeof(template))
return false; return NULL;
*dest = lxc_mkifname(template); nicname = lxc_mkifname(template);
if (!create_nic(*dest, br, pid, cnic)) if (!nicname)
return false; return NULL;
return true; if (!create_nic(nicname, br, pid, cnic)) {
free(nicname);
return NULL;
}
return nicname;
} }
static bool get_nic_from_line(char *p, char **nic) static bool get_nic_from_line(char *p, char **nic)
...@@ -642,13 +646,12 @@ static int count_entries(char *buf, off_t len, char *me, char *t, char *br) ...@@ -642,13 +646,12 @@ static int count_entries(char *buf, off_t len, char *me, char *t, char *br)
* The dbfile has lines of the format: * The dbfile has lines of the format:
* user type bridge nicname * user type bridge nicname
*/ */
static bool get_nic_if_avail(int fd, struct alloted_s *names, int pid, static char *get_nic_if_avail(int fd, struct alloted_s *names, int pid,
char *intype, char *br, int allowed, char *intype, char *br, int allowed, char **cnic)
char **nicname, char **cnic)
{ {
int ret; int ret;
off_t len, slen; off_t len, slen;
char *newline, *owner; char *newline, *nicname, *owner;
struct stat sb; struct stat sb;
struct alloted_s *n; struct alloted_s *n;
int count = 0; int count = 0;
...@@ -658,13 +661,13 @@ static bool get_nic_if_avail(int fd, struct alloted_s *names, int pid, ...@@ -658,13 +661,13 @@ static bool get_nic_if_avail(int fd, struct alloted_s *names, int pid,
cull_entries(fd, n->name, intype, br); cull_entries(fd, n->name, intype, br);
if (allowed == 0) if (allowed == 0)
return false; return NULL;
owner = names->name; owner = names->name;
if (fstat(fd, &sb) < 0) { if (fstat(fd, &sb) < 0) {
usernic_error("Failed to fstat: %s\n", strerror(errno)); usernic_error("Failed to fstat: %s\n", strerror(errno));
return false; return NULL;
} }
len = sb.st_size; len = sb.st_size;
...@@ -672,7 +675,7 @@ static bool get_nic_if_avail(int fd, struct alloted_s *names, int pid, ...@@ -672,7 +675,7 @@ static bool get_nic_if_avail(int fd, struct alloted_s *names, int pid,
buf = mmap(NULL, len, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0); buf = mmap(NULL, len, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0);
if (buf == MAP_FAILED) { if (buf == MAP_FAILED) {
usernic_error("Failed to establish shared memory mapping: %s\n", strerror(errno)); usernic_error("Failed to establish shared memory mapping: %s\n", strerror(errno));
return false; return NULL;
} }
owner = NULL; owner = NULL;
...@@ -688,24 +691,29 @@ static bool get_nic_if_avail(int fd, struct alloted_s *names, int pid, ...@@ -688,24 +691,29 @@ static bool get_nic_if_avail(int fd, struct alloted_s *names, int pid,
} }
if (owner == NULL) if (owner == NULL)
return false; return NULL;
if (!get_new_nicname(nicname, br, pid, cnic)) nicname = get_new_nicname(br, pid, cnic);
return false; if (!nicname) {
usernic_error("%s", "Failed to get new nic name\n");
return NULL;
}
/* owner ' ' intype ' ' br ' ' *nicname + '\n' + '\0' */ /* owner ' ' intype ' ' br ' ' *nicname + '\n' + '\0' */
slen = strlen(owner) + strlen(intype) + strlen(br) + strlen(*nicname) + 5; slen = strlen(owner) + strlen(intype) + strlen(br) + strlen(nicname) + 5;
newline = alloca(slen); newline = alloca(slen);
if (!newline) { if (!newline) {
free(nicname);
usernic_error("Failed allocate memory: %s\n", strerror(errno)); usernic_error("Failed allocate memory: %s\n", strerror(errno));
return false; return NULL;
} }
ret = snprintf(newline, slen, "%s %s %s %s\n", owner, intype, br, *nicname); ret = snprintf(newline, slen, "%s %s %s %s\n", owner, intype, br, nicname);
if (ret < 0 || ret >= slen) { if (ret < 0 || ret >= slen) {
if (lxc_netdev_delete_by_name(*nicname) != 0) if (lxc_netdev_delete_by_name(nicname) != 0)
usernic_error("Error unlinking %s\n", *nicname); usernic_error("Error unlinking %s\n", nicname);
return false; free(nicname);
return NULL;
} }
if (len) if (len)
munmap(buf, len); munmap(buf, len);
...@@ -716,15 +724,16 @@ static bool get_nic_if_avail(int fd, struct alloted_s *names, int pid, ...@@ -716,15 +724,16 @@ static bool get_nic_if_avail(int fd, struct alloted_s *names, int pid,
buf = mmap(NULL, len + slen, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0); buf = mmap(NULL, len + slen, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0);
if (buf == MAP_FAILED) { if (buf == MAP_FAILED) {
usernic_error("Failed to establish shared memory mapping: %s\n", strerror(errno)); usernic_error("Failed to establish shared memory mapping: %s\n", strerror(errno));
if (lxc_netdev_delete_by_name(*nicname) != 0) if (lxc_netdev_delete_by_name(nicname) != 0)
usernic_error("Error unlinking %s\n", *nicname); usernic_error("Error unlinking %s\n", nicname);
return false; free(nicname);
return NULL;
} }
strcpy(buf + len, newline); strcpy(buf + len, newline);
munmap(buf, len + slen); munmap(buf, len + slen);
return true; return nicname;
} }
static bool create_db_dir(char *fnam) static bool create_db_dir(char *fnam)
...@@ -916,9 +925,7 @@ int main(int argc, char *argv[]) ...@@ -916,9 +925,7 @@ int main(int argc, char *argv[])
{ {
int fd, n, pid, ret; int fd, n, pid, ret;
char *me; char *me;
char nicname[100]; char *cnic = NULL, *nicname = NULL, *vethname = NULL;
char *cnic = NULL, *vethname = NULL;
bool gotone = false;
struct alloted_s *alloted = NULL; struct alloted_s *alloted = NULL;
/* Set a sane env, because we are setuid-root. */ /* Set a sane env, because we are setuid-root. */
...@@ -973,11 +980,12 @@ int main(int argc, char *argv[]) ...@@ -973,11 +980,12 @@ int main(int argc, char *argv[])
n = get_alloted(me, argv[4], argv[5], &alloted); n = get_alloted(me, argv[4], argv[5], &alloted);
if (n > 0) if (n > 0)
gotone = get_nic_if_avail(fd, alloted, pid, argv[4], argv[5], n, (char **)&nicname, &cnic); nicname = get_nic_if_avail(fd, alloted, pid, argv[4],
argv[5], n, &cnic);
close(fd); close(fd);
free_alloted(&alloted); free_alloted(&alloted);
if (!gotone) { if (!nicname) {
usernic_error("%s", "Quota reached\n"); usernic_error("%s", "Quota reached\n");
exit(EXIT_FAILURE); exit(EXIT_FAILURE);
} }
...@@ -989,10 +997,12 @@ int main(int argc, char *argv[]) ...@@ -989,10 +997,12 @@ int main(int argc, char *argv[])
ret = lxc_netdev_delete_by_name(cnic); ret = lxc_netdev_delete_by_name(cnic);
if (ret < 0) if (ret < 0)
usernic_error("Failed to delete \"%s\"\n", cnic); usernic_error("Failed to delete \"%s\"\n", cnic);
free(nicname);
exit(EXIT_FAILURE); exit(EXIT_FAILURE);
} }
/* Write the name of the interface pair to the stdout: eth0:veth9MT2L4 */ /* Write the name of the interface pair to the stdout: eth0:veth9MT2L4 */
fprintf(stdout, "%s:%s\n", vethname, nicname); fprintf(stdout, "%s:%s\n", vethname, nicname);
free(nicname);
exit(EXIT_SUCCESS); exit(EXIT_SUCCESS);
} }
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment