To avoid use_after_free, reset lease_ctx->timer back to NULL after the structure has been freed. Change-Id: Icd213ec809b8af934afdb519c335a4680a1d6cdc updates: bz#1655532 Signed-off-by: Soumya Koduri <skoduri@redhat.com> (cherry picked from commit a9b0003c717087ff168bc143c70559162e53e0d5)
Internal fops (with frame->root->pid < 0) are used to heal or move data and maintains data integrity. That is they do not modify client data which holds the lease. Hence no need to recall Lease for such fops. Note: Like for locks, we would need rebalance and self-heal daemon process to heal lease state as well. Change-Id: I8988693fef8d00e17c19dcc842e2238f9eb5ab48 updates: bz#1655532 Signed-off-by: Soumya Koduri <skoduri@redhat.com>
When the glusterfs server recalls the lease, it expects client to flush data and unlock the lease. If not it sets a timer (starting from the time it sends RECALL request) and post timeout, it revokes it. Here we could have a race where in client did send UNLK lease request but because of network delay it may have reached after server revokes it. To handle such situations, treat such requests as noop and return sucesss. Change-Id: I166402d10273f4f115ff04030ecbc14676a01663 updates: bz#1655532 Signed-off-by: Soumya Koduri <skoduri@redhat.com>
From testing & code-reading, found couple of places where we incorrectly unref the inode resulting in use_after_free crash or ref leaks. This patch addresses couple of them. a) When we try to grant the very first lease for a inode, inode_ref is taken in __add_lease. This ref should be active till all the leases granted to that inode are released (i.e, till lease_cnt > 0). In addition even after lease_cnt becomes '0', the inode should be active till all the blocked fops are resumed. Hence release this ref, after resuming all those fops. To avoid granting new leases while resuming those fops, defined a new boolean (blocked_fops_resuming) to flag it in the lease_ctx. b) 'new_lease_inode' which creates new lease_inode_entry and takes ref on inode, is used while adding that entry to client_list and recall_list. Use its counter function '__destroy_lease_inode' which does unref while removing those entries from those lists. c) inode ref is also taken when added to timer->data. Unref the same after processing timer->data. Change-Id: Ie77c78ff4a971e0d9a66178597fb34faf39205fb updates: bz#1655532 Signed-off-by: Soumya Koduri <skoduri@redhat.com>
Currently, there are possibilities in few places, where a user-controlled (like filename, program parameter etc) string can be passed as 'fmt' for printf(), which can lead to segfault, if the user's string contains '%s', '%d' in it. Fixes: CVE-2018-14661 NOTE: this change is a focused fix for the CVE, but is just subset of changes in master. This is done so that we keep the changes in the codebase to minimum, and also as clang coding standard is implemented, the changes wouldn't apply cleanly from master, so there is scope for mistakes. By keeping it to minimum, we solve CVE, and also prevent errors. Fixes: bz#1647668 Change-Id: Ib547293f2d9eb618594cbff0df3b9c800e88bde4 Signed-off-by: Amar Tumballi <amarts@redhat.com>
Fixes CID 1396581 Change-Id: Ic04091b5783a75d8e1e605a9c1c28b77fea048d3 updates: bz#1647972 Signed-off-by: Vijay Bellur <vbellur@redhat.com> Signed-off-by: Susant Palai <spalai@redhat.com>
In the current scheme of glusterfs where lock migration is experimental, (ideally) the rebalance process which is migrating the file should request for a metalock. Hence, the metalock count should not be more than one for an inode. In future, if there is a need for meta-lock from other clients, this patch can be reverted. Since pl_metalk is called as part of setxattr operation, any client process(non-rebalance) residing outside trusted network can exhaust memory of the server node by issuing setxattr repetitively on the metalock key. The current patch makes sure that more than one metalock cannot be granted on an inode. Fixes CVE-2018-14660 updates: bz#1647972 Change-Id: Ie1e697766388718804a9551bc58351808fe71069 Signed-off-by: Susant Palai <spalai@redhat.com>
Patch in master: https://review.gluster.org/#/c/glusterfs/+/21534/ A compromised client can set arbitrary values for the GF_XATTROP_ENTRY_IN_KEY and GF_XATTROP_ENTRY_OUT_KEY during xattrop fop. These values are consumed by index as a filename to be created/deleted according to the key. Thus it is possible to create/delete random files even outside the gluster volume boundary. Fix: Index expects the filename to be a basename, i.e. it must not contain any pathname components like "/" or "../". Enforce this. Fixes: CVE-2018-14654 Fixes: bz#1646200 Change-Id: I35f2a39257b5917d17283d0a4f575b92f783f143 Signed-off-by: Ravishankar N <ravishankar@redhat.com>
Problem: A compromised client can send a variable length buffer value for the GF_XATTR_CLRLK_CMD virtual xattr. If the length is greater than the size of the "key" used to send the response back, locks xlator can segfault when it tries to do a dict_set because of the buffer overflow in strncpy of pl_getxattr(). Fix: Perform size checks while forming the 'key'. Note: This fix is already there in the master branch upstream as a part of the commit 052849983e51a061d7fb2c3ffd74fa78bb257084 (https://review.gluster.org/#/c/glusterfs/+/20933/) This patch just picks the code change needed to fix the vulnerability. Fixes: CVE-2018-14652 fixes: bz#1645363 Change-Id: I101693e91f9ea2bd26cef6c0b7d82527fefcb3e2 Signed-off-by: Ravishankar N <ravishankar@redhat.com>
Glusterfs leases expects lease_id to be set and sent for each fop to determine conflict resolution with the existing lease. Incase if not set (most likely if there is an older client in a mixed cluster), it makes sense to consider it as conflicitng fop and recall the lease. Also fixed the return status check for __remove_lease(), wherein non-negative value is considered as success case. This is backport of below mainline patch - https://review.gluster.org/21458 Change-Id: I5bcfba4f7c71a5af7cdedeb03436d0b818e85783 updates: #350 Signed-off-by: Soumya Koduri <skoduri@redhat.com>