Bug 1396327 - gluster core dump due to assert failed GF_ASSERT (brick_index < wordcount);
Summary: gluster core dump due to assert failed GF_ASSERT (brick_index < wordcount);
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: GlusterFS
Classification: Community
Component: cli
Version: mainline
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Atin Mukherjee
QA Contact:
URL:
Whiteboard:
Depends On:
Blocks: 1475641
TreeView+ depends on / blocked
 
Reported: 2016-11-18 02:52 UTC by George
Modified: 2017-12-08 17:32 UTC (History)
3 users (show)

Fixed In Version: glusterfs-3.13.0
Clone Of:
: 1475641 (view as bug list)
Environment:
Last Closed: 2017-12-08 17:32:42 UTC
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Embargoed:


Attachments (Terms of Use)

Description George 2016-11-18 02:52:38 UTC
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:

Comment 1 George 2016-11-18 02:57:21 UTC
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);

Comment 2 George 2016-11-18 04:24:55 UTC
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;

Comment 3 Worker Ant 2016-11-18 05:30:56 UTC
REVIEW: http://review.gluster.org/15874 (cli: error out incomplete CLI commands) posted (#1) for review on master by Atin Mukherjee (amukherj)

Comment 4 Atin Mukherjee 2016-11-18 05:35:51 UTC
(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?

Comment 5 George 2016-11-18 05:40:21 UTC
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.

Comment 6 Worker Ant 2017-07-17 05:18:09 UTC
REVIEW: https://review.gluster.org/15874 (cli: error out incomplete CLI commands) posted (#2) for review on master by Atin Mukherjee (amukherj)

Comment 7 Worker Ant 2017-07-17 09:31:23 UTC
REVIEW: https://review.gluster.org/15874 (cli: error out incomplete CLI commands) posted (#3) for review on master by Atin Mukherjee (amukherj)

Comment 8 Worker Ant 2017-07-26 07:17:38 UTC
REVIEW: https://review.gluster.org/17870 (cli: error out incomplete CLI commands) posted (#1) for review on master by Atin Mukherjee (amukherj)

Comment 9 Worker Ant 2017-07-26 20:37:32 UTC
COMMIT: https://review.gluster.org/17870 committed in master by Jeff Darcy (jeff.us) 
------
commit c136024613c697fec87aaff3a070862b92c57977
Author: Atin Mukherjee <amukherj>
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
    
    Change-Id: Ia6303457a2aa279465aa75d4e1cfcc948893d5de
    BUG: 1396327
    Signed-off-by: Atin Mukherjee <amukherj>
    Reviewed-on: https://review.gluster.org/17870
    Smoke: Gluster Build System <jenkins.org>
    CentOS-regression: Gluster Build System <jenkins.org>
    Reviewed-by: Jeff Darcy <jeff.us>

Comment 10 Shyamsundar 2017-12-08 17:32:42 UTC
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.13.0, please open a new bug report.

glusterfs-3.13.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-December/000087.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.