Bug 962366 - Perform NULL check on op_errstr before dereferencing it
Perform NULL check on op_errstr before dereferencing it
Status: CLOSED ERRATA
Product: Red Hat Gluster Storage
Classification: Red Hat
Component: glusterfs (Show other bugs)
2.1
Unspecified Unspecified
high Severity unspecified
: ---
: ---
Assigned To: Krutika Dhananjay
Sachidananda Urs
:
Depends On: 962362
Blocks:
  Show dependency treegraph
 
Reported: 2013-05-13 05:39 EDT by Krutika Dhananjay
Modified: 2013-09-23 18:35 EDT (History)
5 users (show)

See Also:
Fixed In Version: glusterfs-3.4.0.10rhs
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: 962362
Environment:
Last Closed: 2013-09-23 18:35:32 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 Krutika Dhananjay 2013-05-13 05:39:49 EDT
+++ This bug was initially created as a clone of Bug #962362 +++

Description of problem:
In the function _gd_syncop_brick_op_cbk(),

 51 int32_t                                                                          
 50 _gd_syncop_brick_op_cbk (struct rpc_req *req, struct iovec *iov,                 
 49                         int count, void *myframe)                                
 48 {                                                                                
 47         struct syncargs        *args  = NULL;                                    
 46         gd1_mgmt_brick_op_rsp  rsp   = {0,};                                     
 45         int                    ret   = -1;                                       
 44         call_frame_t           *frame = NULL;                                    
 43                                                                                  
 42         frame = myframe;                                                         
 41         args = frame->local;                                                     
 40         frame->local = NULL;                                                     
 39                                                                                  
 38         /* initialize */                                                         
 37         args->op_ret   = -1;                                                     
 36         args->op_errno = EINVAL;                                                 
 35                                                                                  
 34         if (-1 == req->rpc_status) {                                             
 33                 args->op_errno = ENOTCONN;                                       
 32                 goto out;                                                        
 31         }                                                                        
 30                                                                                  
 29         ret = xdr_to_generic (*iov, &rsp,                                        
 28                               (xdrproc_t)xdr_gd1_mgmt_brick_op_rsp);             
 27         if (ret < 0)                                                             
 26                 goto out;                                                        
 25                                                                                  
 24         if (rsp.output.output_len) {                                             
 23                 args->dict  = dict_new ();                                       
 22                 if (!args->dict) {                                               
 21                         ret = -1;                                                
 20                         args->op_errno = ENOMEM;                                 
 19                         goto out;                                                
 18                 }                                                                
 17                                                                                  
 16                 ret = dict_unserialize (rsp.output.output_val,                   
 15                                         rsp.output.output_len,                   
 14                                         &args->dict);                            
 13                 if (ret < 0)                                                     
 12                         goto out;                                                
 11         }                                                                        
 10                                                                                  
  9         args->op_ret = rsp.op_ret;                                               
  8         args->op_errno = rsp.op_errno;                                           
  7         args->errstr = gf_strdup (rsp.op_errstr);                                
  6                                                                                  
  5 out:                                                                             
  4         (strcmp (rsp.op_errstr, "")) != 0)                
  3                 free (rsp.op_errstr);                                            
  2         free (rsp.output.output_val);                                            
  1                                                                                  
  0         STACK_DESTROY (frame->root);
  1         __wake (args);                                                           
  2                                                                                  
  3         return 0;                                                                
  4 }

                    
perform NULL check on rsp.op_errstr before using it in strcmp(). 

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


How reproducible:


Steps to Reproduce:
1.
2.
3.
  
Actual results:


Expected results:


Additional info:

--- Additional comment from Anand Avati on 2013-05-13 05:38:04 EDT ---

REVIEW: http://review.gluster.org/4992 (glusterd: Perform NULL check on rsp.op_errstr before using it) posted (#1) for review on master by Krutika Dhananjay (kdhananj@redhat.com)
Comment 3 Sachidananda Urs 2013-07-05 10:10:55 EDT
How to verify this? Any steps or test scripts?
Comment 4 Krutika Dhananjay 2013-07-23 23:19:03 EDT
Terribly sorry about the late response. I tested the fix for this bug in upstream (Bug #962362) manually with the help of gdb.

Here are the steps:

1. Create (at least) a two node (say A and B) cluster.

2. Create a volume with bricks on both the nodes.

3. Start the volume.

4. At this point, attach gdb to glusterd on the originator node by executing the following command in the shell:

         # gdb -p `pidof glusterd`

5. Set a breakpoint at the function _gd_syncop_brick_op_cbk when the gdb prompt appears in the following way:

         (gdb) b glusterd-syncop.c:541

6. Execute volume status from the originator node.

7. When the breakpoint is hit, set req->rpc_status to -1 in the following way:

         (gdb) set req->rpc_status = -1

8. .... and continue with the execution:

         (gdb) c

9. Check if glusterd has crashed on the originator with SIG11.

10. If it doesn't, then the bug fixes the issue.

However, do let me know if you are not fine with testing the validity of the fix this way, in which case I'll try and see if the test can be automated.
Comment 5 Krutika Dhananjay 2013-08-02 03:01:57 EDT
(In reply to Krutika Dhananjay from comment #4)
> Terribly sorry about the late response. I tested the fix for this bug in
> upstream (Bug #962362) manually with the help of gdb.
> 
> Here are the steps:
> 
> 1. Create (at least) a two node (say A and B) cluster.
> 
> 2. Create a volume with bricks on both the nodes.
> 
> 3. Start the volume.
> 
> 4. At this point, attach gdb to glusterd on the originator node by executing
> the following command in the shell:
> 
>          # gdb -p `pidof glusterd`
> 
> 5. Set a breakpoint at the function _gd_syncop_brick_op_cbk when the gdb
> prompt appears in the following way:
> 
>          (gdb) b glusterd-syncop.c:541
> 
> 6. Execute volume status from the originator node.
> 
> 7. When the breakpoint is hit, set req->rpc_status to -1 in the following
> way:
> 
>          (gdb) set req->rpc_status = -1
> 
> 8. .... and continue with the execution:
> 
>          (gdb) c
> 
> 9. Check if glusterd has crashed on the originator with SIG11.
> 
> 10. If it doesn't, then the bug fixes the issue.
> 
> However, do let me know if you are not fine with testing the validity of the
> fix this way, in which case I'll try and see if the test can be automated.

Please read Step 6 as the following:

6. Execute volume stop (as opposed to volume status) from the originator node.
Comment 6 Sachidananda Urs 2013-08-02 03:09:09 EDT
Verfied on: glusterfs 3.4.0.14rhs built on Jul 30 2013 09:09:36
Comment 7 Scott Haines 2013-09-23 18:35:32 EDT
Since the problem described in this bug report should be resolved in a recent advisory, it has been closed with a resolution of ERRATA. 

For information on the advisory, and where to find the updated files, follow the link below.

If the solution does not work for you, open a new bug report.

http://rhn.redhat.com/errata/RHBA-2013-1262.html

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