Bug 1468291 - NFS Sub directory is getting mounted on solaris 10 even when the permission is restricted in nfs.export-dir volume option
Summary: NFS Sub directory is getting mounted on solaris 10 even when the permission i...
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: GlusterFS
Classification: Community
Component: nfs
Version: mainline
Hardware: Unspecified
OS: Unspecified
urgent
urgent
Target Milestone: ---
Assignee: Niels de Vos
QA Contact:
URL:
Whiteboard:
Depends On:
Blocks: 1451224 1472773 1477190
TreeView+ depends on / blocked
 
Reported: 2017-07-06 15:13 UTC by Niels de Vos
Modified: 2017-12-08 17:34 UTC (History)
2 users (show)

Fixed In Version: glusterfs-3.13.0
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2017-09-05 17:36:29 UTC
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Embargoed:


Attachments (Terms of Use)

Description Niels de Vos 2017-07-06 15:13:39 UTC
Description of problem:

NFS Sub directory is getting mounted on solaris 10 even when the permission is restricted to some clients only in nfs.export-dir volume option   


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

How reproducible:
Consistently

Steps to Reproduce:

1.Create a 6*2 Distributed-Replicate volume.Enable GNFS on it.
2.Mount the volume to solaris client via v3 and create a directory inside the mount.

# mount -o proto=tcp,vers=3 nfs://svr.example.net:/GNFSVolume/ /mnt/solmount
# mkdir mani

3.Unmount the volume.
4.Provide permission to export subdirectory mani to some client only (which is other client,not solaris client)

# gluster v set GNFSVolume nfs.export-dir "/mani(10.70.37.142)"
volume set: success

# gluster v get GNFSVolume all | grep export
nfs.export-dirs                         on                                      
nfs.export-volumes                      on                                      
nfs.export-dir                          /mani(10.70.37.142)                     
nfs.exports-auth-enable                 (null)           

5. Mount the subdirectory mani to solaris client.

Actual results:
Mount was successful even when the client does not have that subdirectory mount permission.

# mount -o proto=tcp,vers=3 nfs://svr.example.net:/GNFSVolume/mani /mnt/solmount
# cd /mnt/solmount
# ls
ms
# mkdir dir1
# ls
dir1         ms


Expected results:
subdirectory Mount should fail on solaris client when it doesnot have permission to mount for that client.

Additional info:

Comment 1 Worker Ant 2017-07-06 15:15:52 UTC
REVIEW: https://review.gluster.org/17718 (nfs: add permission checking for mounting over WebNFS) posted (#1) for review on master by Niels de Vos (ndevos)

Comment 2 Worker Ant 2017-07-07 05:55:12 UTC
REVIEW: https://review.gluster.org/17718 (nfs: add permission checking for mounting over WebNFS) posted (#2) for review on master by Atin Mukherjee (amukherj)

Comment 3 Worker Ant 2017-07-07 07:40:11 UTC
REVIEW: https://review.gluster.org/17718 (nfs: add permission checking for mounting over WebNFS) posted (#3) for review on master by Niels de Vos (ndevos)

Comment 4 Worker Ant 2017-07-09 09:14:26 UTC
COMMIT: https://review.gluster.org/17718 committed in master by Niels de Vos (ndevos) 
------
commit e304f48fa262e5cdbe181fb3fee5dfb9c893108c
Author: Niels de Vos <ndevos>
Date:   Thu Jul 6 17:04:17 2017 +0200

    nfs: add permission checking for mounting over WebNFS
    
    Solaris 10 uses WebNFS and not the MOUNT protocol. All permission checks
    for allowing/denying clients to mount are done through the MNT handlers.
    These handlers will not give out a filehandle to the NFS-client when
    mounting is denied. This prevents clients from successful mounting.
    However, over WebNFS a well known 'root-filehandle' is used directly
    with the NFSv3 protocol.
    
    When WebNFS was used, no permission checks (the "nfs.export-dir" option)
    were applied. Now the WebNFS mount-handler in Gluster/NFS calls the
    mnt3_parse_dir_exports() function that takes care of the permission
    checking.
    
    BUG: 1468291
    Change-Id: Ic9dfd092473ba9c1c7b5fa38401cf9c0aa8395bb
    Signed-off-by: Niels de Vos <ndevos>
    Reviewed-on: https://review.gluster.org/17718
    Smoke: Gluster Build System <jenkins.org>
    Reviewed-by: soumya k <skoduri>
    CentOS-regression: Gluster Build System <jenkins.org>
    Reviewed-by: Kaleb KEITHLEY <kkeithle>

Comment 5 Niels de Vos 2017-07-19 11:48:54 UTC
A segfault can happen with this change:

(gdb) bt
#0  0x00007f8b43d91205 in _gf_ref_put (ref=ref@entry=0x0) at refcount.c:36
#1  0x00007f8b35820455 in nfs3_call_state_wipe (cs=cs@entry=0x0) at nfs3.c:559
#2  0x00007f8b35823dd2 in nfs3_lookup (req=req@entry=0x7f8b3015f3f0, fh=fh@entry=0x7f8b37066ad0, fhlen=<optimized out>, name=name@entry=0x7f8b37066b10 "disperseVol") at nfs3.c:1586
#3  0x00007f8b35824408 in nfs3svc_lookup (req=0x7f8b3015f3f0) at nfs3.c:1615
#4  0x00007f8b43ae58c5 in rpcsvc_handle_rpc_call (svc=0x7f8b3006b9f0, trans=trans@entry=0x7f8b30167270, msg=<optimized out>) at rpcsvc.c:695
#5  0x00007f8b43ae5aab in rpcsvc_notify (trans=0x7f8b30167270, mydata=<optimized out>, event=<optimized out>, data=<optimized out>) at rpcsvc.c:789
#6  0x00007f8b43ae79e3 in rpc_transport_notify (this=this@entry=0x7f8b30167270, event=event@entry=RPC_TRANSPORT_MSG_RECEIVED, data=data@entry=0x7f8b30160720) at rpc-transport.c:538
#7  0x00007f8b389163d6 in socket_event_poll_in (this=this@entry=0x7f8b30167270, notify_handled=<optimized out>) at socket.c:2306
#8  0x00007f8b3891897c in socket_event_handler (fd=34, idx=33, gen=10, data=0x7f8b30167270, poll_in=1, poll_out=0, poll_err=0) at socket.c:2458
#9  0x00007f8b43d7d0f6 in event_dispatch_epoll_handler (event=0x7f8b37067e80, event_pool=0x55d3ffe94fd0) at event-epoll.c:572
#10 event_dispatch_epoll_worker (data=0x55d3ffedb5f0) at event-epoll.c:648
#11 0x00007f8b42b81e25 in start_thread () from /lib64/libpthread.so.0
#12 0x00007f8b4244e34d in clone () from /lib64/libc.so.6
(gdb) f 2
#2  0x00007f8b35823dd2 in nfs3_lookup (req=req@entry=0x7f8b3015f3f0, fh=fh@entry=0x7f8b37066ad0, fhlen=<optimized out>, name=name@entry=0x7f8b37066b10 "disperseVol") at nfs3.c:1586
1586                    nfs3_call_state_wipe (cs);
(gdb) l
1581                    stat = nfs3_errno_to_nfsstat3 (-ret);
1582                    nfs3_log_common_res (rpcsvc_request_xid (req),
1583                                         NFS3_LOOKUP, stat, -ret,
1584                                         cs ? cs->resolvedloc.path : NULL);
1585                    nfs3_lookup_reply (req, stat, NULL, NULL, NULL);
1586                    nfs3_call_state_wipe (cs);
1587                    /* Ret must be 0 after this so that the caller does not
1588                     * also send an RPC reply.
1589                     */
1590                    ret = 0;


If there is a permission error, 'cs' may not get initialized and hence nfs3_call_state_wipe() should not be called.

Comment 6 Worker Ant 2017-07-19 12:00:15 UTC
REVIEW: https://review.gluster.org/17822 (nfs: improve error handling for WebNFS mount permissions) posted (#1) for review on master by Niels de Vos (ndevos)

Comment 7 Worker Ant 2017-07-21 10:17:40 UTC
COMMIT: https://review.gluster.org/17822 committed in master by Niels de Vos (ndevos) 
------
commit 890ae2a1b2ce72d22657e7463405e59bee1e298a
Author: Niels de Vos <ndevos>
Date:   Wed Jul 19 13:53:27 2017 +0200

    nfs: improve error handling for WebNFS mount permissions
    
    In case nfs3_funge_webnfs_zerolen_fh() returns an error, the
    nfs3_call_state_t structure will not get initialized. This means that
    calling `nfs3_call_state_wipe (cs)` will result in a segmentation fault
    after commit daed52b8eb that makes nfs3_call_state_t refcounted.
    
    Change-Id: I4c300aedf132a7fea95756dd278ff87d67722478
    BUG: 1468291
    Fixes: e3f48fa2 ("nfs: add permission checking for mounting over WebNFS")
    Signed-off-by: Niels de Vos <ndevos>
    Reviewed-on: https://review.gluster.org/17822
    Smoke: Gluster Build System <jenkins.org>
    CentOS-regression: Gluster Build System <jenkins.org>
    Reviewed-by: jiffin tony Thottan <jthottan>
    Reviewed-by: soumya k <skoduri>

Comment 8 Worker Ant 2017-07-27 12:51:58 UTC
REVIEW: https://review.gluster.org/17897 (libglusterfs: the global_xlator should have valid cbks) posted (#1) for review on master by Niels de Vos (ndevos)

Comment 9 Worker Ant 2017-07-27 12:52:03 UTC
REVIEW: https://review.gluster.org/17898 (nfs: use "/" as subdir for volume mounts) posted (#1) for review on master by Niels de Vos (ndevos)

Comment 10 Worker Ant 2017-07-27 23:28:18 UTC
COMMIT: https://review.gluster.org/17897 committed in master by Jeff Darcy (jeff.us) 
------
commit cec5036f7e99ae265bb5e0e7f3df30166466eb2c
Author: Niels de Vos <ndevos>
Date:   Thu Jul 27 14:11:27 2017 +0200

    libglusterfs: the global_xlator should have valid cbks
    
    There is a case where Gluster/NFS needs to resolve a path outside of the
    nfs-xlator itself. While resolving the path to fetch the GFID for
    creating the NFS-filehandle, gfapi may set an inode-ctx through
    glfs_resolve_at(). This inode-ctx is linked with the global_xlator.
    
    Because the global_xlator does not have any cbks, loc_wipe() will cause
    a segfault when it calls inode_unref() and xl->cbks->forget(). It is
    assumed that all xlators have a cbks symbol, otherwise loading of the
    xlator will fail. The global_xlator is not loaded in the same way, so
    there is no failure noticed when the instance is created. By adding an
    empty `struct xlator_cbks`, the global_xlator behaves similat to other
    xlators that do not implement all callbacks.
    
    I would have preferred to keep the inode-ctx setting through
    glfs_resolve_at() contained within Gluster/NFS. Unfortunately
    Gluster/NFS also uses the inode-ctx, and is not prepared to see the
    values that glfs_resolve_at() stores there.
    
    This problem is not easily reproducible because it involves mounting
    over WebNFS (like Solaris 10 can do). The segfault will also not be
    immediate, unless the following is done:
    
    1. create a subdir on a volume
    2. mount the volume/subdir over WebNFS
    3. unmount the volume/subdir
    4. mount the root of the volume
    5. delete the subdir on the volume -> segfault of Gluster/NFS
    
    Change-Id: I2bd71d033e97edc07ba93b2d4ada558f65d68999
    BUG: 1468291
    Signed-off-by: Niels de Vos <ndevos>
    Reviewed-on: https://review.gluster.org/17897
    Smoke: Gluster Build System <jenkins.org>
    Reviewed-by: Amar Tumballi <amarts>
    Reviewed-by: jiffin tony Thottan <jthottan>
    CentOS-regression: Gluster Build System <jenkins.org>
    Reviewed-by: Jeff Darcy <jeff.us>

Comment 11 Worker Ant 2017-07-31 13:53:35 UTC
REVIEW: https://review.gluster.org/17898 (nfs: use "/" as subdir for volume mounts) posted (#2) for review on master by Niels de Vos (ndevos)

Comment 12 Worker Ant 2017-08-01 05:11:52 UTC
COMMIT: https://review.gluster.org/17898 committed in master by jiffin tony Thottan (jthottan) 
------
commit 45c973576d6356dbe4da897e9f0528eac7529d48
Author: Niels de Vos <ndevos>
Date:   Tue Jul 25 09:25:53 2017 +0200

    nfs: use "/" as subdir for volume mounts
    
    For cases where subdir mounting is checked, it makes it much easier to
    return a subdir of "/" in case no subdir is passed. This reduces the
    number of corner cases where permissions are checked for subdir mounts,
    but not for volume mounts (or the other way around).
    
    The problem was identified by WebNFS mounting a volume, which got denied
    after commit e3f48fa2. Handling this would require an exception for
    non-subdir mounts, or make non-subdir mounts equal to subdir mounts.
    This change takes the 2nd approach.
    
    Change-Id: I0d810ae90b267a2cc3eac8d55368a0f1b0787f6a
    Fixes: e3f48fa2 ("nfs: add permission checking for mounting over WebNFS")
    BUG: 1468291
    Signed-off-by: Niels de Vos <ndevos>
    Reviewed-on: https://review.gluster.org/17898
    Smoke: Gluster Build System <jenkins.org>
    Reviewed-by: soumya k <skoduri>
    CentOS-regression: Gluster Build System <jenkins.org>
    Reviewed-by: jiffin tony Thottan <jthottan>

Comment 13 Shyamsundar 2017-09-05 17:36:29 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.12.0, please open a new bug report.

glusterfs-3.12.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-September/000082.html
[2] https://www.gluster.org/pipermail/gluster-users/

Comment 14 Shyamsundar 2017-12-08 17:34:10 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.