Bug 1049735 - Avoid NULL referencing of auth->authops->request_init during RPC init
Summary: Avoid NULL referencing of auth->authops->request_init during RPC init
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: GlusterFS
Classification: Community
Component: rpc
Version: mainline
Hardware: x86_64
OS: Linux
low
low
Target Milestone: ---
Assignee: Harshavardhana
QA Contact:
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2014-01-08 06:05 UTC by Harshavardhana
Modified: 2015-03-23 01:04 UTC (History)
3 users (show)

Fixed In Version: glusterfs-3.6.0beta1
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2014-11-11 08:26:43 UTC
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Embargoed:


Attachments (Terms of Use)

Description Harshavardhana 2014-01-08 06:05:07 UTC
Code section is bogus!
------------------------------------------
370:       if (!auth->authops->request_init)
371:              ret = auth->authops->request_init (req, auth->authprivate);
------------------------------------------
Seems to have been never been used historically since
logically above code has never been true to actually execute
"authops->request_init() --> auth_glusterfs_{v2,}_request_init()"
On top of that under "rpcsvc_request_init()"
verf.flavour and verf.datalen are initialized from what is
provided through 'callmsg'.
------------------------------------------
        req->verf.flavour = rpc_call_verf_flavour (callmsg);
        req->verf.datalen = rpc_call_verf_len (callmsg);
        /* AUTH */
        rpcsvc_auth_request_init (req);
        return req;
------------------------------------------
So the code in 'auth_glusterfs_{v2,}_request_init()'
performing this operation will over-write the original
flavour and datalen.
------------------------------------------
      if (!req)
                return -1;
        memset (req->verf.authdata, 0, GF_MAX_AUTH_BYTES);
        req->verf.datalen = 0;
        req->verf.flavour = AUTH_NULL;
------------------------------------------

Comment 1 Anand Avati 2014-01-08 06:08:21 UTC
REVIEW: http://review.gluster.org/6591 (rpc/auth: Avoid NULL dereference in rpcsvc_auth_request_init()) posted (#4) for review on master by Harshavardhana (harsha)

Comment 2 Anand Avati 2014-01-09 04:42:24 UTC
COMMIT: http://review.gluster.org/6591 committed in master by Vijay Bellur (vbellur) 
------
commit 2b05c1588ac60af26e1b16f9f27ef8d5e4e50a5f
Author: Harshavardhana <harsha>
Date:   Tue Dec 24 08:23:13 2013 -0800

    rpc/auth: Avoid NULL dereference in rpcsvc_auth_request_init()
    
    Code section is bogus!
    ------------------------------------------
    370:       if (!auth->authops->request_init)
    371:              ret = auth->authops->request_init (req, auth->authprivate);
    ------------------------------------------
    
    Seems to have been never been used historically since
    logically above code has never been true to actually execute
    "authops->request_init() --> auth_glusterfs_{v2,}_request_init()"
    
    On top of that under "rpcsvc_request_init()"
    verf.flavour and verf.datalen are initialized from what is
    provided through 'callmsg'.
    ------------------------------------------
            req->verf.flavour = rpc_call_verf_flavour (callmsg);
            req->verf.datalen = rpc_call_verf_len (callmsg);
    
            /* AUTH */
            rpcsvc_auth_request_init (req);
            return req;
    ------------------------------------------
    
    So the code in 'auth_glusterfs_{v2,}_request_init()'
    performing this operation will over-write the original
    flavour and datalen.
    
    ------------------------------------------
          if (!req)
                    return -1;
            memset (req->verf.authdata, 0, GF_MAX_AUTH_BYTES);
            req->verf.datalen = 0;
            req->verf.flavour = AUTH_NULL;
    ------------------------------------------
    
    Refactoring the whole code into a more understandable version
    and also avoiding a potential NULL dereference
    
    Change-Id: I1a430fcb4d26de8de219bd0cb3c46c141649d47d
    BUG: 1049735
    Signed-off-by: Harshavardhana <harsha>
    Reviewed-on: http://review.gluster.org/6591
    Reviewed-by: Santosh Pradhan <spradhan>
    Tested-by: Gluster Build System <jenkins.com>
    Reviewed-by: Vijay Bellur <vbellur>

Comment 3 Niels de Vos 2014-09-22 12:34:49 UTC
A beta release for GlusterFS 3.6.0 has been released. Please verify if the release solves this bug report for you. In case the glusterfs-3.6.0beta1 release does not have a resolution for this issue, leave a comment in this bug and move the status to ASSIGNED. If this release fixes the problem for you, leave a note and change the status to VERIFIED.

Packages for several distributions should become available in the near future. Keep an eye on the Gluster Users mailinglist [2] and the update (possibly an "updates-testing" repository) infrastructure for your distribution.

[1] http://supercolony.gluster.org/pipermail/gluster-users/2014-September/018836.html
[2] http://supercolony.gluster.org/pipermail/gluster-users/

Comment 4 Niels de Vos 2014-11-11 08:26:43 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.6.1, please reopen this bug report.

glusterfs-3.6.1 has been announced [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://supercolony.gluster.org/pipermail/gluster-users/2014-November/019410.html
[2] http://supercolony.gluster.org/mailman/listinfo/gluster-users


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