Commit 0ee9052f authored by willy tarreau's avatar willy tarreau Committed by Stephen Hemminger

fix "ss -p" segfaults

I've updated Jose's patch to make it slightly simpler (eg: calloc instead
of malloc+memset), and ported it to 4.2.0 which requires it as well, and
attached it to this e-mail.

I can confirm that with this patch 4.1.1 doesn't segfault on me anymore.
The commit message should be reworked I guess though everything's in it
and I didn't want to modify his description.

Can it be merged as-is or should I reword the commit message and reference
Jose as the fix reporter ? We should not let this bug live forever.

From: "j.ps@openmailbox.org" <j.ps@openmailbox.org>

Essentially all that is needed to get rid of this issue is the
addition of:

    memset(u, 0, sizeof(*u));

after:

    if (!(u = malloc(sizeof(*u))))
            break;

Also patched some other situations (strcpy and sprintf uses) that
potentially produce the same results.
Signed-off-by: default avatarJose P Santos <j.ps@openmailbox.org>

[ wt: made Jose's patch slightly simpler, all credits to him for the diag ]
Signed-off-by: default avatarWilly Tarreau <w@1wt.eu>
parent a60223bc
...@@ -457,7 +457,9 @@ static void user_ent_hash_build(void) ...@@ -457,7 +457,9 @@ static void user_ent_hash_build(void)
user_ent_hash_build_init = 1; user_ent_hash_build_init = 1;
strcpy(name, root); strncpy(name, root, sizeof(name)-1);
name[sizeof(name)-1] = 0;
if (strlen(name) == 0 || name[strlen(name)-1] != '/') if (strlen(name) == 0 || name[strlen(name)-1] != '/')
strcat(name, "/"); strcat(name, "/");
...@@ -481,7 +483,7 @@ static void user_ent_hash_build(void) ...@@ -481,7 +483,7 @@ static void user_ent_hash_build(void)
if (getpidcon(pid, &pid_context) != 0) if (getpidcon(pid, &pid_context) != 0)
pid_context = strdup(no_ctx); pid_context = strdup(no_ctx);
sprintf(name + nameoff, "%d/fd/", pid); snprintf(name + nameoff, sizeof(name) - nameoff, "%d/fd/", pid);
pos = strlen(name); pos = strlen(name);
if ((dir1 = opendir(name)) == NULL) { if ((dir1 = opendir(name)) == NULL) {
free(pid_context); free(pid_context);
...@@ -502,7 +504,7 @@ static void user_ent_hash_build(void) ...@@ -502,7 +504,7 @@ static void user_ent_hash_build(void)
if (sscanf(d1->d_name, "%d%c", &fd, &crap) != 1) if (sscanf(d1->d_name, "%d%c", &fd, &crap) != 1)
continue; continue;
sprintf(name+pos, "%d", fd); snprintf(name+pos, sizeof(name) - pos, "%d", fd);
link_len = readlink(name, lnk, sizeof(lnk)-1); link_len = readlink(name, lnk, sizeof(lnk)-1);
if (link_len == -1) if (link_len == -1)
...@@ -2738,7 +2740,7 @@ static int unix_show(struct filter *f) ...@@ -2738,7 +2740,7 @@ static int unix_show(struct filter *f)
struct sockstat *u, **insp; struct sockstat *u, **insp;
int flags; int flags;
if (!(u = malloc(sizeof(*u)))) if (!(u = calloc(1, sizeof(*u))))
break; break;
u->name = NULL; u->name = NULL;
u->peer_name = NULL; u->peer_name = NULL;
...@@ -3088,11 +3090,13 @@ static int netlink_show_one(struct filter *f, ...@@ -3088,11 +3090,13 @@ static int netlink_show_one(struct filter *f,
strncpy(procname, "kernel", 6); strncpy(procname, "kernel", 6);
} else if (pid > 0) { } else if (pid > 0) {
FILE *fp; FILE *fp;
sprintf(procname, "%s/%d/stat", snprintf(procname, sizeof(procname), "%s/%d/stat",
getenv("PROC_ROOT") ? : "/proc", pid); getenv("PROC_ROOT") ? : "/proc", pid);
if ((fp = fopen(procname, "r")) != NULL) { if ((fp = fopen(procname, "r")) != NULL) {
if (fscanf(fp, "%*d (%[^)])", procname) == 1) { if (fscanf(fp, "%*d (%[^)])", procname) == 1) {
sprintf(procname+strlen(procname), "/%d", pid); snprintf(procname+strlen(procname),
sizeof(procname)-strlen(procname),
"/%d", pid);
done = 1; done = 1;
} }
fclose(fp); fclose(fp);
......
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