Bug 1475641 - gluster core dump due to assert failed GF_ASSERT (brick_index < wordcount);
gluster core dump due to assert failed GF_ASSERT (brick_index < wordcount);
Status: CLOSED CURRENTRELEASE
Product: GlusterFS
Classification: Community
Component: cli (Show other bugs)
3.12
Unspecified Unspecified
unspecified Severity unspecified
: ---
: ---
Assigned To: Atin Mukherjee
: Triaged
Depends On: 1396327
Blocks:
  Show dependency treegraph
 
Reported: 2017-07-27 00:35 EDT by Atin Mukherjee
Modified: 2017-09-05 13:37 EDT (History)
3 users (show)

See Also:
Fixed In Version: glusterfs-3.12.0
Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: 1396327
Environment:
Last Closed: 2017-09-05 13:37:48 EDT
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Atin Mukherjee 2017-07-27 00:35:43 EDT
+++ 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@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@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@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@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@pl.atyp.us) 
------
commit c136024613c697fec87aaff3a070862b92c57977
Author: Atin Mukherjee <amukherj@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@nokia.com
    
    Change-Id: Ia6303457a2aa279465aa75d4e1cfcc948893d5de
    BUG: 1396327
    Signed-off-by: Atin Mukherjee <amukherj@redhat.com>
    Reviewed-on: https://review.gluster.org/17870
    Smoke: Gluster Build System <jenkins@build.gluster.org>
    CentOS-regression: Gluster Build System <jenkins@build.gluster.org>
    Reviewed-by: Jeff Darcy <jeff@pl.atyp.us>
Comment 1 Worker Ant 2017-07-27 00:41:19 EDT
REVIEW: https://review.gluster.org/17892 (cli: error out incomplete CLI commands) posted (#1) for review on release-3.12 by Atin Mukherjee (amukherj@redhat.com)
Comment 2 Worker Ant 2017-07-31 13:29:33 EDT
COMMIT: https://review.gluster.org/17892 committed in release-3.12 by Shyamsundar Ranganathan (srangana@redhat.com) 
------
commit 282bff2e32903b0be1773c81ad8b9836980661cf
Author: Atin Mukherjee <amukherj@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@nokia.com
    
    >Reviewed-on: https://review.gluster.org/17870
    >Smoke: Gluster Build System <jenkins@build.gluster.org>
    >CentOS-regression: Gluster Build System <jenkins@build.gluster.org>
    >Reviewed-by: Jeff Darcy <jeff@pl.atyp.us>
    >(cherry picked from commit c136024613c697fec87aaff3a070862b92c57977)
    
    Change-Id: Ia6303457a2aa279465aa75d4e1cfcc948893d5de
    BUG: 1475641
    Signed-off-by: Atin Mukherjee <amukherj@redhat.com>
    Reviewed-on: https://review.gluster.org/17892
    Smoke: Gluster Build System <jenkins@build.gluster.org>
    CentOS-regression: Gluster Build System <jenkins@build.gluster.org>
    Reviewed-by: Shyamsundar Ranganathan <srangana@redhat.com>
Comment 3 Shyamsundar 2017-09-05 13:37:48 EDT
This bug is getting closed because a release has been made available that should address the reported issue. In case the problem is still not fixed with glusterfs-3.12.0, please open a new bug report.

glusterfs-3.12.0 has been announced on the Gluster mailinglists [1], packages for several distributions should become available in the near future. Keep an eye on the Gluster Users mailinglist [2] and the update infrastructure for your distribution.

[1] http://lists.gluster.org/pipermail/announce/2017-September/000082.html
[2] https://www.gluster.org/pipermail/gluster-users/

Note You need to log in before you can comment on or make changes to this bug.