Bug 1386635

Summary: removal of file from nfs mount crashs ganesha server
Product: [Red Hat Storage] Red Hat Gluster Storage Reporter: Niels de Vos <ndevos>
Component: upcallAssignee: Niels de Vos <ndevos>
Status: CLOSED ERRATA QA Contact: Arthy Loganathan <aloganat>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: rhgs-3.2CC: amukherj, jthottan, kkeithle, mchangir, pgurusid, rhinduja, rhs-bugs, sbhaloth, skoduri, storage-qa-internal
Target Milestone: ---Keywords: Triaged
Target Release: RHGS 3.2.0   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: nfs-ganesha-2.4.1-2, glusterfs-3.8.4-6 Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: 1347717 Environment:
Last Closed: 2017-03-23 06:24:34 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On: 1347717    
Bug Blocks: 1337867, 1351528    

Description Niels de Vos 2016-10-19 10:41:37 UTC
+++ This bug was initially created as a clone of Bug #1344714 +++

Description of problem:

The struct callback_inode_arg is a variable used by upcall infra for populating upcall entries. This variable is allocated using GF_CALLOC in glfs_h_poll_cache_invalidation(), but it also freedin FSAL_GLUSTER using free.
This will result in a crash when we try to remove a file from mount

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

How reproducible:


Steps to Reproduce:
1. enable nfs-ganesha and export a volume 
2. mount the volume
3. create a file and delete it from the mount

Actual results:
ganesha server crashes

Expected results:
should not crash

Additional info:

--- Additional comment from Vijay Bellur on 2016-06-10 09:17:53 EDT ---

REVIEW: http://review.gluster.org/14701 (libgfapi : Introducing glfs_free() api) posted (#1) for review on master by jiffin tony Thottan (jthottan)

--- Additional comment from Vijay Bellur on 2016-06-10 09:17:56 EDT ---

REVIEW: http://review.gluster.org/14702 (libgfapi/upcall : appending glfs to callback_arg, callback_inode_arg) posted (#1) for review on master by jiffin tony Thottan (jthottan)

--- Additional comment from Vijay Bellur on 2016-06-10 16:57:16 EDT ---

REVIEW: http://review.gluster.org/14702 (libgfapi/upcall : prepend "glfs_" to callback_arg, callback_inode_arg) posted (#2) for review on master by Jeff Darcy (jdarcy)

--- Additional comment from Vijay Bellur on 2016-06-10 17:09:37 EDT ---

COMMIT: http://review.gluster.org/14702 committed in master by Jeff Darcy (jdarcy) 
------
commit 2775dc64101ed37c8d9809bf9852dbf0746ee2b6
Author: Jiffin Tony Thottan <jthottan>
Date:   Fri Jun 10 18:23:43 2016 +0530

    libgfapi/upcall : prepend "glfs_" to callback_arg, callback_inode_arg
    
    Change-Id: I371525775db4f6a4d69beb94baaa53d17b16fb41
    BUG: 1344714
    Signed-off-by: Jiffin Tony Thottan <jthottan>
    Reviewed-on: http://review.gluster.org/14702
    CentOS-regression: Gluster Build System <jenkins.com>
    NetBSD-regression: NetBSD Build System <jenkins.org>
    Reviewed-by: Jeff Darcy <jdarcy>
    Tested-by: Jeff Darcy <jdarcy>
    Smoke: Gluster Build System <jenkins.com>

--- Additional comment from Vijay Bellur on 2016-06-14 04:58:05 EDT ---

REVIEW: http://review.gluster.org/14701 (libgfapi : Introducing glfs_free_callback_inode_arg() api) posted (#2) for review on master by jiffin tony Thottan (jthottan)

--- Additional comment from Vijay Bellur on 2016-06-14 06:14:16 EDT ---

REVIEW: http://review.gluster.org/14701 (libgfapi : Introducing glfs_free_callback_inode_arg() api) posted (#3) for review on master by jiffin tony Thottan (jthottan)

--- Additional comment from Vijay Bellur on 2016-06-16 09:28:10 EDT ---

REVIEW: http://review.gluster.org/14701 (libgfapi : Introducing glfs_free_callback_inode_arg() api) posted (#4) for review on master by jiffin tony Thottan (jthottan)

--- Additional comment from Vijay Bellur on 2016-06-16 09:30:18 EDT ---

REVIEW: http://review.gluster.org/14701 (libgfapi : Introducing glfs_free_callback_inode_arg() api) posted (#5) for review on master by jiffin tony Thottan (jthottan)

--- Additional comment from Vijay Bellur on 2016-06-16 09:41:28 EDT ---

REVIEW: http://review.gluster.org/14701 (libgfapi : free glfs_callback_inode_arg from application layer) posted (#6) for review on master by jiffin tony Thottan (jthottan)

--- Additional comment from Vijay Bellur on 2016-06-17 08:46:07 EDT ---

REVIEW: http://review.gluster.org/14759 (libgfapi/upcall : use calloc and free for callback_inode_arg) posted (#1) for review on master by jiffin tony Thottan (jthottan)

--- Additional comment from Niels de Vos on 2016-09-12 07:39:20 CEST ---

All 3.8.x bugs are now reported against version 3.8 (without .x). For more information, see http://www.gluster.org/pipermail/gluster-devel/2016-September/050859.html

--- Additional comment from Niels de Vos on 2016-09-23 17:29:15 CEST ---



--- Additional comment from Worker Ant on 2016-10-14 12:50:43 CEST ---

REVIEW: http://review.gluster.org/15639 (libglusterfs: add gf_get_mem_type()) posted (#1) for review on release-3.8 by Niels de Vos (ndevos)

--- Additional comment from Worker Ant on 2016-10-14 12:50:47 CEST ---

REVIEW: http://review.gluster.org/15640 (gfapi: redesign the public interface for upcall consumers) posted (#1) for review on release-3.8 by Niels de Vos (ndevos)

--- Additional comment from Worker Ant on 2016-10-18 09:00:10 CEST ---

COMMIT: http://review.gluster.org/15639 committed in release-3.8 by Niels de Vos (ndevos) 
------
commit 8d1b94f6e43175df60ca9067db1811997d3ef4fc
Author: Niels de Vos <ndevos>
Date:   Fri Oct 14 12:49:29 2016 +0200

    libglusterfs: add gf_get_mem_type()
    
    gfapi needs to provide a function towards applications to free memory
    that it allocated. Depending on how the application is compiled/linked,
    it could use a different memory allocator than Gluster itself. Therefore
    it is not safe for gfapi to request applications to free memory with
    'standard' free().
    
    Examples for this are Gluster allocated structures with GF_CALLOC() when
    memory accounting is enabled (the default). Some gfapi functions use
    malloc() to allocate memory as a workaround, but the free() from the
    jemalloc implementation should not be combined with the malloc() from
    glibc.
    
    Cherry picked from commit db4e26ed71a01e5f760fbc3c7051962426f102c9:
    > Change-Id: I626cd1a60abf8965f9263290f4045d1f69fc2093
    > BUG: 1344714
    > Signed-off-by: Niels de Vos <ndevos>
    > Reviewed-on: http://review.gluster.org/15108
    > Smoke: Gluster Build System <jenkins.org>
    > Reviewed-by: soumya k <skoduri>
    > Reviewed-by: jiffin tony Thottan <jthottan>
    > NetBSD-regression: NetBSD Build System <jenkins.org>
    > CentOS-regression: Gluster Build System <jenkins.org>
    > Reviewed-by: Shyamsundar Ranganathan <srangana>
    
    This also includes commit da2aabe3 which fixes an unused variable
    warning.
    
    Change-Id: I626cd1a60abf8965f9263290f4045d1f69fc2093
    BUG: 1347717
    Signed-off-by: Niels de Vos <ndevos>
    Reviewed-on: http://review.gluster.org/15639
    Smoke: Gluster Build System <jenkins.org>
    NetBSD-regression: NetBSD Build System <jenkins.org>
    CentOS-regression: Gluster Build System <jenkins.org>
    Reviewed-by: jiffin tony Thottan <jthottan>

--- Additional comment from Worker Ant on 2016-10-19 12:36:11 CEST ---

REVIEW: http://review.gluster.org/15680 (gfapi: add glfs_free() to glfs.h) posted (#1) for review on release-3.8 by Niels de Vos (ndevos)

Comment 2 Niels de Vos 2016-10-19 10:43:25 UTC
Patches posted upstream: http://review.gluster.org/#/q/topic:bug-1347717

Comment 3 Niels de Vos 2016-10-19 11:05:40 UTC
Soumya, do you have an other bug to use for the nfs-ganesha change?

Comment 4 Soumya Koduri 2016-10-19 11:20:57 UTC
No. There weren't any other bugs filed for this issue.

Comment 5 Soumya Koduri 2016-10-19 11:35:01 UTC
and I think we could use same bug even if there are fixes in different components.. 
Atin, could you please confirm? We have another fix needed in ganesha sources. Do we need to file another bug to track that? Thanks

Comment 6 Atin Mukherjee 2016-10-20 05:21:18 UTC
(In reply to Soumya Koduri from comment #5)
> and I think we could use same bug even if there are fixes in different
> components.. 
> Atin, could you please confirm? We have another fix needed in ganesha
> sources. Do we need to file another bug to track that? Thanks

If we are sure about that its just a single issue which requires fixes across different components we can track it through a single bug itself.

Comment 14 Atin Mukherjee 2016-11-30 05:43:34 UTC
Moving this bug to Post as this still needs to have one more ganesha patch to go in.

Comment 17 Arthy Loganathan 2017-01-16 12:47:32 UTC
Verified the fix in build,

glusterfs-ganesha-3.8.4-11.el7rhgs.x86_64
nfs-ganesha-2.4.1-5.el7rhgs.x86_64
nfs-ganesha-gluster-2.4.1-5.el7rhgs.x86_64

Comment 19 errata-xmlrpc 2017-03-23 06:24:34 UTC
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.

https://rhn.redhat.com/errata/RHEA-2017-0493.html