[Bugs] [Bug 1475641] New: gluster core dump due to assert failed GF_ASSERT ( brick_index < wordcount);

[bugzilla at redhat.com]

https://bugzilla.redhat.com/show_bug.cgi?id=1475641

            Bug ID: 1475641
           Summary: gluster core dump due to assert failed GF_ASSERT
                    (brick_index < wordcount);
           Product: GlusterFS
           Version: 3.12
         Component: cli
          Keywords: Triaged
          Assignee: bugs at gluster.org
          Reporter: amukherj at redhat.com
                CC: amukherj at redhat.com, bugs at gluster.org,
                    george.lian at nokia.com
        Depends On: 1396327



+++ This bug was initially created as a clone of Bug #1396327 +++

Description of problem:
coredump when incomplete gluster CLI command.
all with brick-name command has this issue, such like add-brick, remove-brick

Version-Release number of selected component (if applicable):
3.6.9 and mainline

How reproducible:


Steps to Reproduce:
1. gluster volume add-brick export  replica 2 force
2.
3.

Actual results:
coredump


Expected results:
prompt wrong parameter used.

Additional info:

--- Additional comment from George on 2016-11-17 21:57:21 EST ---

suggest fix on function cli_cmd_bricks_parse in cli-cmd-parser.c

        GF_ASSERT (brick_index > 0);
+        if(strcmp(words[brick_index],"force") == 0)
+        {
+               cli_err ("Wrong brick name used.");
+               ret = -1;
+               goto out;
+        }
        GF_ASSERT (brick_index < wordcount);

--- Additional comment from George on 2016-11-17 23:24:55 EST ---

Sorry, the last fix is not work.
Now we have fix it on 3.6.9 version, main branch should also have the same
issue.
the changes patch on 3.6.9 FYI:

--- a/cli/src/cli-cmd-parser.c
+++ b/cli/src/cli-cmd-parser.c
@@ -544,6 +544,12 @@

         brick_index = index;

+        if (strcmp (words[brick_index], "force") == 0) {
+               cli_err ("Wrong brick name.");
+               ret = -1;
+               goto out;
+        }
+
         if (strcmp (words[wordcount - 1], "force") == 0) {
                 is_force = _gf_true;
                 wc = wordcount - 1;
@@ -1301,6 +1307,12 @@
         brick_index = index;

 parse_bricks:
+
+        if (strcmp (words[brick_index], "force") == 0) {
+               cli_err ("Wrong brick name.");
+               ret = -1;
+               goto out;
+        }

         if (strcmp (words[wordcount - 1], "force") == 0) {
                 is_force = _gf_true;

--- Additional comment from Worker Ant on 2016-11-18 00:30:56 EST ---

REVIEW: http://review.gluster.org/15874 (cli: error out incomplete CLI
commands) posted (#1) for review on master by Atin Mukherjee
(amukherj at redhat.com)

--- Additional comment from Atin Mukherjee on 2016-11-18 00:35:51 EST ---

(In reply to George from comment #1)
> suggest fix on function cli_cmd_bricks_parse in cli-cmd-parser.c
> 
>         GF_ASSERT (brick_index > 0);
> +        if(strcmp(words[brick_index],"force") == 0)
> +        {
> +               cli_err ("Wrong brick name used.");
> +               ret = -1;
> +               goto out;
> +        }
>         GF_ASSERT (brick_index < wordcount);

Why wouldn't it work? I just sent a patch and tested the same?

--- Additional comment from George on 2016-11-18 00:40:21 EST ---

Sorry, it could work.
but because bric_index == wordcount  in this case,
this strcmp statement somehow is confused and maybe has risk.

I would like suggest use the second fix. though this fix also work for this
defect.

--- Additional comment from Worker Ant on 2017-07-17 01:18:09 EDT ---

REVIEW: https://review.gluster.org/15874 (cli: error out incomplete CLI
commands) posted (#2) for review on master by Atin Mukherjee
(amukherj at redhat.com)

--- Additional comment from Worker Ant on 2017-07-17 05:31:23 EDT ---

REVIEW: https://review.gluster.org/15874 (cli: error out incomplete CLI
commands) posted (#3) for review on master by Atin Mukherjee
(amukherj at redhat.com)

--- Additional comment from Worker Ant on 2017-07-26 03:17:38 EDT ---

REVIEW: https://review.gluster.org/17870 (cli: error out incomplete CLI
commands) posted (#1) for review on master by Atin Mukherjee
(amukherj at redhat.com)

--- Additional comment from Worker Ant on 2017-07-26 16:37:32 EDT ---

COMMIT: https://review.gluster.org/17870 committed in master by Jeff Darcy
(jeff at pl.atyp.us) 
------
commit c136024613c697fec87aaff3a070862b92c57977
Author: Atin Mukherjee <amukherj at redhat.com>
Date:   Wed Jul 26 12:46:42 2017 +0530

    cli: error out incomplete CLI commands

    cli_cmd_bricks_parse () & cli_cmd_volume_remove_brick_parse () were not
    handling the the error cases where the command is incomplete with
    missing brick details which could lead to glusterd crashes.

    Credit : george.lian at nokia.com

    Change-Id: Ia6303457a2aa279465aa75d4e1cfcc948893d5de
    BUG: 1396327
    Signed-off-by: Atin Mukherjee <amukherj at redhat.com>
    Reviewed-on: https://review.gluster.org/17870
    Smoke: Gluster Build System <jenkins at build.gluster.org>
    CentOS-regression: Gluster Build System <jenkins at build.gluster.org>
    Reviewed-by: Jeff Darcy <jeff at pl.atyp.us>


Referenced Bugs:

https://bugzilla.redhat.com/show_bug.cgi?id=1396327
[Bug 1396327] gluster core dump due to assert failed GF_ASSERT (brick_index
< wordcount);
-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are the assignee for the bug.