[dm-devel] [PATCH] multipathd: "show map mpathx json" would cause realloc error possibly

2016-11-01 Thread tang . wenjun3 
From: 10144149 

Problem: multipathd dead when we run "show map mpathx json" command with
system messages as follows:
Oct 13 11:37:30 rhel7-1 multipathd: *** Error in `/sbin/multipathd': realloc(): 
invalid next size: 0x7f8cf8004210 ***
Oct 13 11:37:30 rhel7-1 multipathd: === Backtrace: =
Oct 13 11:37:30 rhel7-1 multipathd: /lib64/libc.so.6(+0x7bc67)[0x7f8d06171c67]
Oct 13 11:37:30 rhel7-1 multipathd: /lib64/libc.so.6(+0x7fb17)[0x7f8d06175b17]
Oct 13 11:37:30 rhel7-1 multipathd: 
/lib64/libc.so.6(realloc+0xd2)[0x7f8d06176702]

Reasons: in function snprint_multipath_fields_json
vector_foreach_slot (pgp->paths, pp, j) {
   fwd += snprint_path(buff + fwd, len - fwd, PRINT_JSON_PATH, pp, 0);
   if (fwd > len)
return fwd;

   fwd += snprint_json_elem_footer(buff + fwd,
len - fwd, 3, j + 1 == VECTOR_SIZE(pgp->paths));
   if (fwd > len)
   return fwd;
}

snprint_path (char * line, int len, char * format, struct path * pp, int pad)

when len - fwd = 0 , The len is not restricted in snprint_path，and the Memory 
of line is
rewritten in snprint_path, it cause realloc() failed , so fwd > len modify
fwd >= len.

consider of ben’s advice, It would probably also be smart to change all the
  if (!TAIL)
lines to
  if (TAIL <= 0)
just as an extra precaution.

Other commands also have this type of risk.

Signed-off-by: 10144149 
---
 libmultipath/print.c | 141 ++-
 1 file changed, 71 insertions(+), 70 deletions(-)

diff --git a/libmultipath/print.c b/libmultipath/print.c
index 9aa41ad..2eadb81 100644
--- a/libmultipath/print.c
+++ b/libmultipath/print.c
@@ -778,7 +778,7 @@ snprint_multipath_header (char * line, int len, char * 
format)
struct multipath_data * data;
 
do {
-   if (!TAIL)
+   if (TAIL <= 0)
break;
 
if (*f != '%') {
@@ -811,7 +811,7 @@ snprint_multipath (char * line, int len, char * format,
char buff[MAX_FIELD_LEN] = {};
 
do {
-   if (!TAIL)
+   if (TAIL <= 0)
break;
 
if (*f != '%') {
@@ -845,7 +845,7 @@ snprint_path_header (char * line, int len, char * format)
struct path_data * data;
 
do {
-   if (!TAIL)
+   if (TAIL <= 0)
break;
 
if (*f != '%') {
@@ -878,7 +878,7 @@ snprint_path (char * line, int len, char * format,
char buff[MAX_FIELD_LEN];
 
do {
-   if (!TAIL)
+   if (TAIL <= 0)
break;
 
if (*f != '%') {
@@ -913,7 +913,7 @@ snprint_pathgroup (char * line, int len, char * format,
char buff[MAX_FIELD_LEN];
 
do {
-   if (!TAIL)
+   if (TAIL <= 0)
break;
 
if (*f != '%') {
@@ -1004,11 +1004,11 @@ snprint_multipath_topology (char * buff, int len, 
struct multipath * mpp,
c += sprintf(c, "%c[%dm", 0x1B, 0); /* bold off */
 
fwd += snprint_multipath(buff + fwd, len - fwd, style, mpp, 1);
-   if (fwd > len)
+   if (fwd >= len)
return len;
fwd += snprint_multipath(buff + fwd, len - fwd, PRINT_MAP_PROPS, mpp,
 1);
-   if (fwd > len)
+   if (fwd >= len)
return len;
 
if (!mpp->pg)
@@ -1022,7 +1022,7 @@ snprint_multipath_topology (char * buff, int len, struct 
multipath * mpp,
} else
strcpy(f, "`-+- " PRINT_PG_INDENT);
fwd += snprint_pathgroup(buff + fwd, len - fwd, fmt, pgp);
-   if (fwd > len)
+   if (fwd >= len)
return len;
 
vector_foreach_slot (pgp->paths, pp, i) {
@@ -1035,13 +1035,14 @@ snprint_multipath_topology (char * buff, int len, 
struct multipath * mpp,
else
strcpy(f, " `- " PRINT_PATH_INDENT);
fwd += snprint_path(buff + fwd, len - fwd, fmt, pp, 1);
-   if (fwd > len)
+   if (fwd >= len)
return len;
}
}
return fwd;
 }
 
+
 static int
 snprint_json (char * buff, int len, int indent, char *json_str)
 {
@@ -1049,7 +1050,7 @@ snprint_json (char * buff, int len, int indent, char 
*json_str)
 
for (i = 0; i < indent; i++) {
fwd += snprintf(buff + fwd, len - fwd, PRINT_JSON_INDENT);
-   if (fwd > len)
+   if (fwd >= len)
return fwd;
}
 
@@ -1063,7 +1064,7 @@ snprint_json_header (char * buff, int len)
int fwd = 0;
 
fwd +=  snprint_json(buff, len, 0, PRINT_JSON_START_ELEM);
-   if (fwd > len)
+   if (fwd >= len)

Re: [dm-devel] [PATCH] multipathd: "show map mpathx json" would cause realloc error possibly

2016-11-01 Thread Benjamin Marzinski 
On Tue, Nov 01, 2016 at 01:54:38PM +0800, tang.jun...@zte.com.cn wrote:
>A return value from calling snprintf() of size or more means
>that the output was truncated, so I think this patch solved a
>big problem, it avoids memory collapse.

ACK

It would probably also be smart to change all the
if (!TAIL)
break;

lines to

if (TAIL <= 0)
break;

just as an extra precaution.

-Ben

> 
>> ��:         tang.wenj...@zte.com.cn
>> �ռ���:         Christophe Varoqui <christophe.varo...@opensvc.com>,
>> :        zhang.ka...@zte.com.cn, dm-devel@redhat.com, 10144149
><tang.wenj...@zte.com.cn>
>> ����:         2016-10-14 11:23
>    > ����:        [dm-devel] [PATCH] multipathd: "show map mpathx json" would
>cause        realloc error possibly
>> ��:        dm-devel-boun...@redhat.com
>>
>>
>>
>> From: 10144149 <tang.wenj...@zte.com.cn>
>>
>> Problem: multipathd dead when we run "show map spathx json" command with
>> system messages as follows:
>> Oct 13 11:37:30 rhel7-1 multipathd: *** Error in `/sbin/multipathd':
>realloc(): invalid next size: 0x7f8cf8004210 ***
>> Oct 13 11:37:30 rhel7-1 multipathd: === Backtrace: =
>> Oct 13 11:37:30 rhel7-1 multipathd:
>/lib64/libc.so.6(+0x7bc67)[0x7f8d06171c67]
>> Oct 13 11:37:30 rhel7-1 multipathd:
>/lib64/libc.so.6(+0x7fb17)[0x7f8d06175b17]
>> Oct 13 11:37:30 rhel7-1 multipathd:
>/lib64/libc.so.6(realloc+0xd2)[0x7f8d06176702]
>>
>> Reasons: in function snprint_multipath_fields_json
>> vector_foreach_slot (pgp->paths, pp, j) {
>>        fwd += snprint_path(buff + fwd, len - fwd, PRINT_JSON_PATH, pp,
>0);
>>        if (fwd > len)
>>             return fwd;
>>
>>        fwd += snprint_json_elem_footer(buff + fwd,
>>                 len - fwd, 3, j + 1 == VECTOR_SIZE(pgp->paths));
>>        if (fwd > len)
>>            return fwd;
>> }
>>
>> snprint_path (char * line, int len, char * format, struct path * pp, int
>pad)
>>
>> when len - fwd = 0 , The len is not restricted in snprint_path��and the
>Memory of line is
>> rewritten in snprint_path, it cause realloc() failed , so fwd > len
>modify
>> fwd >= len.
>>
>> Other commands also have this type of risk.
>>
>> Signed-off-by: 10144149 <tang.wenj...@zte.com.cn>
>> ---
>>  libmultipath/print.c | 131
>++-
>>  1 file changed, 66 insertions(+), 65 deletions(-)
>>
>> diff --git a/libmultipath/print.c b/libmultipath/print.c
>> index 9aa41ad..78c065f 100644
>> --- a/libmultipath/print.c
>> +++ b/libmultipath/print.c
>> @@ -1004,11 +1004,11 @@ snprint_multipath_topology (char * buff, int
>len, struct multipath * mpp,
>>                                    c += sprintf(c, "%c[%dm", 0x1B, 0);
>/* bold off */
>>  
>>                   fwd += snprint_multipath(buff + fwd, len - fwd, style,
>mpp, 1);
>> -                 if (fwd > len)
>> +                 if (fwd >= len)
>>                                    return len;
>>                   fwd += snprint_multipath(buff + fwd, len - fwd,
>PRINT_MAP_PROPS, mpp,
>>                                                                      
>1);
>> -                 if (fwd > len)
>> +                 if (fwd >= len)
>>                                    return len;
>>  
>>                   if (!mpp->pg)
>> @@ -1022,7 +1022,7 @@ snprint_multipath_topology (char * buff, int len,
>struct multipath * mpp,
>>                                    } else
>>                                                     strcpy(f, "`-+- "
>PRINT_PG_INDENT);
>>                                    fwd += snprint_pathgroup(buff + fwd,
>len - fwd, fmt, pgp);
>> -                                  if (fwd > len)
>> +                                  if (fwd >= len)
>>                                                     return len;
>>  
>>                                    vector_foreach_slot (pgp->paths, pp,
>i) {
>> @@ -1035,13 +1035,14 @@ snprint_

[dm-devel] [PATCH] multipathd: "show map mpathx json" would cause realloc error possibly

2016-10-13 Thread tang . wenjun3 
From: 10144149 

Problem: multipathd dead when we run "show map spathx json" command with
system messages as follows:
Oct 13 11:37:30 rhel7-1 multipathd: *** Error in `/sbin/multipathd': realloc(): 
invalid next size: 0x7f8cf8004210 ***
Oct 13 11:37:30 rhel7-1 multipathd: === Backtrace: =
Oct 13 11:37:30 rhel7-1 multipathd: /lib64/libc.so.6(+0x7bc67)[0x7f8d06171c67]
Oct 13 11:37:30 rhel7-1 multipathd: /lib64/libc.so.6(+0x7fb17)[0x7f8d06175b17]
Oct 13 11:37:30 rhel7-1 multipathd: 
/lib64/libc.so.6(realloc+0xd2)[0x7f8d06176702]

Reasons: in function snprint_multipath_fields_json
vector_foreach_slot (pgp->paths, pp, j) {
   fwd += snprint_path(buff + fwd, len - fwd, PRINT_JSON_PATH, pp, 0);
   if (fwd > len)
return fwd;

   fwd += snprint_json_elem_footer(buff + fwd,
len - fwd, 3, j + 1 == VECTOR_SIZE(pgp->paths));
   if (fwd > len)
   return fwd;
}

snprint_path (char * line, int len, char * format, struct path * pp, int pad)

when len - fwd = 0 , The len is not restricted in snprint_path，and the Memory 
of line is
rewritten in snprint_path, it cause realloc() failed , so fwd > len modify
fwd >= len.

Other commands also have this type of risk.

Signed-off-by: 10144149 
---
 libmultipath/print.c | 131 ++-
 1 file changed, 66 insertions(+), 65 deletions(-)

diff --git a/libmultipath/print.c b/libmultipath/print.c
index 9aa41ad..78c065f 100644
--- a/libmultipath/print.c
+++ b/libmultipath/print.c
@@ -1004,11 +1004,11 @@ snprint_multipath_topology (char * buff, int len, 
struct multipath * mpp,
c += sprintf(c, "%c[%dm", 0x1B, 0); /* bold off */
 
fwd += snprint_multipath(buff + fwd, len - fwd, style, mpp, 1);
-   if (fwd > len)
+   if (fwd >= len)
return len;
fwd += snprint_multipath(buff + fwd, len - fwd, PRINT_MAP_PROPS, mpp,
 1);
-   if (fwd > len)
+   if (fwd >= len)
return len;
 
if (!mpp->pg)
@@ -1022,7 +1022,7 @@ snprint_multipath_topology (char * buff, int len, struct 
multipath * mpp,
} else
strcpy(f, "`-+- " PRINT_PG_INDENT);
fwd += snprint_pathgroup(buff + fwd, len - fwd, fmt, pgp);
-   if (fwd > len)
+   if (fwd >= len)
return len;
 
vector_foreach_slot (pgp->paths, pp, i) {
@@ -1035,13 +1035,14 @@ snprint_multipath_topology (char * buff, int len, 
struct multipath * mpp,
else
strcpy(f, " `- " PRINT_PATH_INDENT);
fwd += snprint_path(buff + fwd, len - fwd, fmt, pp, 1);
-   if (fwd > len)
+   if (fwd >= len)
return len;
}
}
return fwd;
 }
 
+
 static int
 snprint_json (char * buff, int len, int indent, char *json_str)
 {
@@ -1049,7 +1050,7 @@ snprint_json (char * buff, int len, int indent, char 
*json_str)
 
for (i = 0; i < indent; i++) {
fwd += snprintf(buff + fwd, len - fwd, PRINT_JSON_INDENT);
-   if (fwd > len)
+   if (fwd >= len)
return fwd;
}
 
@@ -1063,7 +1064,7 @@ snprint_json_header (char * buff, int len)
int fwd = 0;
 
fwd +=  snprint_json(buff, len, 0, PRINT_JSON_START_ELEM);
-   if (fwd > len)
+   if (fwd >= len)
return fwd;
 
fwd +=  snprintf(buff + fwd, len  - fwd, PRINT_JSON_START_VERSION,
@@ -1078,7 +1079,7 @@ snprint_json_elem_footer (char * buff, int len, int 
indent, int last)
 
for (i = 0; i < indent; i++) {
fwd += snprintf(buff + fwd, len - fwd, PRINT_JSON_INDENT);
-   if (fwd > len)
+   if (fwd >= len)
return fwd;
}
 
@@ -1098,50 +1099,50 @@ snprint_multipath_fields_json (char * buff, int len,
struct pathgroup *pgp;
 
fwd += snprint_multipath(buff, len, PRINT_JSON_MAP, mpp, 0);
-   if (fwd > len)
+   if (fwd >= len)
return fwd;
 
fwd += snprint_json(buff + fwd, len - fwd, 2, PRINT_JSON_START_GROUPS);
-   if (fwd > len)
+   if (fwd >= len)
return fwd;
 
vector_foreach_slot (mpp->pg, pgp, i) {
 
pgp->selector = mpp->selector;
fwd += snprint_pathgroup(buff + fwd, len - fwd, 
PRINT_JSON_GROUP, pgp);
-   if (fwd > len)
+   if (fwd >= len)
return fwd;
 
fwd += snprintf(buff + fwd, len - fwd, PRINT_JSON_GROUP_NUM, i 
+ 1);
-   if (fwd > len)
+   if (fwd >= len)
return fwd;
 
fwd += snprint_json(buff + fwd, len - fwd, 3, 
PRINT_JSON_START_PATHS);
-   if