Re: "ss -p" segfaults (updated to 4.2)

[Stephen Hemminger]

Applied, and did some editing on commit msg
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: "ss -p" segfaults (updated to 4.2)

[Willy Tarreau]

On Mon, Oct 12, 2015 at 09:50:19AM -0700, Stephen Hemminger wrote:
> Applied, and did some editing on commit msg

Thank you Stephen!

Willy

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: "ss -p" segfaults (updated to 4.2)

[j...@openmailbox.org]

Hi Willy,

It seems fine to me.
You may reword the commit message at will.

Thx,
Jose

On 2015-10-06 11:09, Willy Tarreau wrote:
> Hi guys,
> 
> 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.
> 
> Thanks,
> Willy
> 
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: "ss -p" segfaults (updated to 4.2)

[Willy Tarreau]

Hi guys,

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.

Thanks,
Willy

>From 618028d6c5bfa4fb9898f2ec1ab5483e6f5392d4 Mon Sep 17 00:00:00 2001
From: "j...@openmailbox.org" 
Date: Tue, 21 Jul 2015 10:54:05 +0100
Subject: "ss -p" segfaults

Patch for 4.2.0

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: Jose P Santos 

[ wt: made Jose's patch slightly simpler, all credits to him for the diag ]
Signed-off-by: Willy Tarreau 
---
 misc/ss.c | 16 ++--
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/misc/ss.c b/misc/ss.c
index 2f34962..8b0d606 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -457,7 +457,9 @@ static void user_ent_hash_build(void)
 
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] != '/')
strcat(name, "/");
 
@@ -481,7 +483,7 @@ static void user_ent_hash_build(void)
if (getpidcon(pid, _context) != 0)
pid_context = strdup(no_ctx);
 
-   sprintf(name + nameoff, "%d/fd/", pid);
+   snprintf(name + nameoff, sizeof(name) - nameoff, "%d/fd/", pid);
pos = strlen(name);
if ((dir1 = opendir(name)) == NULL) {
free(pid_context);
@@ -502,7 +504,7 @@ static void user_ent_hash_build(void)
if (sscanf(d1->d_name, "%d%c", , ) != 1)
continue;
 
-   sprintf(name+pos, "%d", fd);
+   snprintf(name+pos, sizeof(name) - pos, "%d", fd);
 
link_len = readlink(name, lnk, sizeof(lnk)-1);
if (link_len == -1)
@@ -2736,7 +2738,7 @@ static int unix_show(struct filter *f)
struct sockstat *u, **insp;
int flags;
 
-   if (!(u = malloc(sizeof(*u
+   if (!(u = calloc(1, sizeof(*u
break;
u->name = NULL;
u->peer_name = NULL;
@@ -3086,11 +3088,13 @@ static int netlink_show_one(struct filter *f,
strncpy(procname, "kernel", 6);
} else if (pid > 0) {
FILE *fp;
-   sprintf(procname, "%s/%d/stat",
+   snprintf(procname, sizeof(procname), "%s/%d/stat",
getenv("PROC_ROOT") ? : "/proc", pid);
if ((fp = fopen(procname, "r")) != NULL) {
if (fscanf(fp, "%*d (%[^)])", procname) == 1) {
-   sprintf(procname+strlen(procname), 
"/%d", pid);
+   snprintf(procname+strlen(procname),
+   
sizeof(procname)-strlen(procname),
+   "/%d", pid);
done = 1;
}
fclose(fp);
-- 
1.7.12.1