Bug 1227916 - auth_cache_entry structure barely gets cached
Summary: auth_cache_entry structure barely gets cached
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: GlusterFS
Classification: Community
Component: nfs
Version: 3.7.0
Hardware: Unspecified
OS: Unspecified
medium
medium
Target Milestone: ---
Assignee: Niels de Vos
QA Contact:
URL:
Whiteboard:
Depends On: 1226714
Blocks: glusterfs-3.7.2 1227918
TreeView+ depends on / blocked
 
Reported: 2015-06-03 19:52 UTC by Niels de Vos
Modified: 2015-06-20 09:49 UTC (History)
2 users (show)

Fixed In Version: glusterfs-3.7.2
Doc Type: Bug Fix
Doc Text:
Clone Of:
: 1227918 (view as bug list)
Environment:
Last Closed: 2015-06-20 09:49:46 UTC
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Embargoed:


Attachments (Terms of Use)

Description Niels de Vos 2015-06-03 19:52:09 UTC
+++ This bug was initially created as a clone of Bug #1226714 +++

Description of problem:
The "struct auth_cache_entry" gets put into a dictionary, with a hash-string of the client and the requested mount-point/directory as key. However, due to a programming error, the hash-string is often set to NULL.

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

Additional notes:
Noticed during code-review.

--- Additional comment from Anand Avati on 2015-06-03 10:13:04 CEST ---

REVIEW: http://review.gluster.org/11019 (nfs: allocate and return the hashkey for the auth_cache_entry) posted (#2) for review on master by Niels de Vos (ndevos)

--- Additional comment from Anand Avati on 2015-06-03 12:10:28 CEST ---

REVIEW: http://review.gluster.org/11019 (nfs: allocate and return the hashkey for the auth_cache_entry) posted (#3) for review on master by Niels de Vos (ndevos)

--- Additional comment from Anand Avati on 2015-06-03 21:51:17 CEST ---

COMMIT: http://review.gluster.org/11019 committed in master by Niels de Vos (ndevos) 
------
commit b8b59fea7822f9ab1e10d7a3f730354fe82a6097
Author: Niels de Vos <ndevos>
Date:   Sun May 31 22:29:11 2015 +0200

    nfs: allocate and return the hashkey for the auth_cache_entry
    
    The allocation of the hashkey was never returned to the calling
    function.
    
    Allocating it with alloca() puts it on the stack, returning from the
    function makes the pointer invalid. Functions that are annotated with
    "inline" and call alloca(), will not always be inlined. Returning a
    pointer allocated with alloca() is in those cases not correct. One such
    confirmation was provided by GCC developer Alexandre Oliva:
     - http://gcc.gnu.org/ml/gcc-help/2004-04/msg00158.html
    
    It is more correct to call GF_MALLOC() and GF_FREE() for the hashkey. If
    this would result in preformance hit, we can always think of using
    alloca() again and turn make_hashkey() into a macro (yuck).
    
    Change-Id: Ia86a1f79d33240af4713bfb92f702b0ee6e87eb7
    BUG: 1226714
    Signed-off-by: Niels de Vos <ndevos>
    Reviewed-on: http://review.gluster.org/11019
    Reviewed-by: Kaleb KEITHLEY <kkeithle>
    Reviewed-by: jiffin tony Thottan <jthottan>
    Reviewed-by: soumya k <skoduri>
    Tested-by: Gluster Build System <jenkins.com>

Comment 1 Anand Avati 2015-06-09 17:09:28 UTC
COMMIT: http://review.gluster.org/11075 committed in release-3.7 by Niels de Vos (ndevos) 
------
commit cccd1e3cce7873e873c7266af703f4f77ff4c679
Author: Niels de Vos <ndevos>
Date:   Wed Jun 3 21:53:37 2015 +0200

    nfs: allocate and return the hashkey for the auth_cache_entry
    
    The allocation of the hashkey was never returned to the calling
    function.
    
    Allocating it with alloca() puts it on the stack, returning from the
    function makes the pointer invalid. Functions that are annotated with
    "inline" and call alloca(), will not always be inlined. Returning a
    pointer allocated with alloca() is in those cases not correct. One such
    confirmation was provided by GCC developer Alexandre Oliva:
     - http://gcc.gnu.org/ml/gcc-help/2004-04/msg00158.html
    
    It is more correct to call GF_MALLOC() and GF_FREE() for the hashkey. If
    this would result in preformance hit, we can always think of using
    alloca() again and turn make_hashkey() into a macro (yuck).
    
    Cherry picked from commit b8b59fea7822f9ab1e10d7a3f730354fe82a6097:
    > Change-Id: Ia86a1f79d33240af4713bfb92f702b0ee6e87eb7
    > BUG: 1226714
    > Signed-off-by: Niels de Vos <ndevos>
    > Reviewed-on: http://review.gluster.org/11019
    > Reviewed-by: Kaleb KEITHLEY <kkeithle>
    > Reviewed-by: jiffin tony Thottan <jthottan>
    > Reviewed-by: soumya k <skoduri>
    > Tested-by: Gluster Build System <jenkins.com>
    
    Change-Id: Ia86a1f79d33240af4713bfb92f702b0ee6e87eb7
    BUG: 1227916
    Signed-off-by: Niels de Vos <ndevos>
    Reviewed-on: http://review.gluster.org/11075
    Tested-by: Gluster Build System <jenkins.com>
    Reviewed-by: jiffin tony Thottan <jthottan>

Comment 2 Niels de Vos 2015-06-20 09:49:46 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.7.2, please reopen this bug report.

glusterfs-3.7.2 has been announced on the Gluster Packaging mailinglist [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://www.gluster.org/pipermail/packaging/2015-June/000006.html
[2] http://thread.gmane.org/gmane.comp.file-systems.gluster.user


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