[haiku-commits] haiku: hrev51970 - src/system/kernel/fs headers/private/kernel/fs headers/private/kernel

  • From: Axel Dörfler <axeld@xxxxxxxxxxxxxxxx>
  • To: haiku-commits@xxxxxxxxxxxxx
  • Date: Tue, 22 May 2018 16:29:41 -0400 (EDT)

hrev51970 adds 1 changeset to branch 'master'
old head: fc48586b9bb80e37d8258ded8516697745c6211c
new head: 8bca37d604536e5908c5a4ce224ee5afb4ebafc7
overview: 
https://git.haiku-os.org/haiku/log/?qt=range&q=8bca37d60453+%5Efc48586b9bb8

----------------------------------------------------------------------------

8bca37d60453: vfs: Bind flock locks to file descriptors
  
  * File locks created by flock should only apply for the file descriptor
    that was used to lock the file. Another fd on the same file should then
    be denied access (calling flock should fail).
  * fcntl based locks, however, are in a separate namespace and are global
    to a team.
  * This issue was found when running webkitpy test suite, and should close
    ticket #13795.
  * Don't use session or team as comparison in release_advisory_lock(), as
    that information might not be available anymore (e.g. when called from
    Team::~Team()). This fixes #14121.
  
  Change-Id: I9efb96cfcefe7e72b0060220c635a665e7e643cc
  Co-authored-by: Axel Dörfler <axeld@xxxxxxxxxxxxxxxx>

                             [ Adrien Destugues <pulkomandy@xxxxxxxxxxxxx> ]

----------------------------------------------------------------------------

Revision:    hrev51970
Commit:      8bca37d604536e5908c5a4ce224ee5afb4ebafc7
URL:         https://git.haiku-os.org/haiku/commit/?id=8bca37d60453
Author:      Adrien Destugues <pulkomandy@xxxxxxxxxxxxx>
Date:        Sun Jan 14 10:26:12 2018 UTC
Committer:   Axel Dörfler <axeld@xxxxxxxxxxxxxxxx>
Commit-Date: Tue May 22 20:29:21 2018 UTC

Ticket:      https://dev.haiku-os.org/ticket/13795
Ticket:      https://dev.haiku-os.org/ticket/14121

----------------------------------------------------------------------------

4 files changed, 75 insertions(+), 44 deletions(-)
headers/private/kernel/fs/fd.h | 26 ++++++++-----
headers/private/kernel/vfs.h   |  4 +-
src/system/kernel/fs/fd.cpp    | 14 ++++---
src/system/kernel/fs/vfs.cpp   | 75 +++++++++++++++++++++++---------------

----------------------------------------------------------------------------

diff --git a/headers/private/kernel/fs/fd.h b/headers/private/kernel/fs/fd.h
index 0312b5fd5d..f9ee92cc30 100644
--- a/headers/private/kernel/fs/fd.h
+++ b/headers/private/kernel/fs/fd.h
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2006, Axel Dörfler, axeld@xxxxxxxxxxxxxxxx.
+ * Copyright 2002-2018, Axel Dörfler, axeld@xxxxxxxxxxxxxxxx.
  * Distributed under the terms of the MIT License.
  */
 #ifndef _FD_H
@@ -22,10 +22,13 @@ struct selectsync;
 struct select_info;
 
 struct fd_ops {
-       status_t        (*fd_read)(struct file_descriptor *, off_t pos, void 
*buffer, size_t *length);
-       status_t        (*fd_write)(struct file_descriptor *, off_t pos, const 
void *buffer, size_t *length);
+       status_t        (*fd_read)(struct file_descriptor *, off_t pos, void 
*buffer,
+                                               size_t *length);
+       status_t        (*fd_write)(struct file_descriptor *, off_t pos,
+                                               const void *buffer, size_t 
*length);
        off_t           (*fd_seek)(struct file_descriptor *, off_t pos, int 
seekType);
-       status_t        (*fd_ioctl)(struct file_descriptor *, ulong op, void 
*buffer, size_t length);
+       status_t        (*fd_ioctl)(struct file_descriptor *, ulong op, void 
*buffer,
+                                               size_t length);
        status_t        (*fd_set_flags)(struct file_descriptor *, int flags);
        status_t        (*fd_select)(struct file_descriptor *, uint8 event,
                                                struct selectsync *sync);
@@ -36,7 +39,8 @@ struct fd_ops {
                                                size_t bufferSize, uint32 
*_count);
        status_t        (*fd_rewind_dir)(struct file_descriptor *);
        status_t        (*fd_read_stat)(struct file_descriptor *, struct stat 
*);
-       status_t        (*fd_write_stat)(struct file_descriptor *, const struct 
stat *, int statMask);
+       status_t        (*fd_write_stat)(struct file_descriptor *, const struct 
stat *,
+               int statMask);
        status_t        (*fd_close)(struct file_descriptor *);
        void            (*fd_free)(struct file_descriptor *);
 };
@@ -76,11 +80,13 @@ enum fd_types {
 /* Prototypes */
 
 extern struct file_descriptor *alloc_fd(void);
-extern int new_fd_etc(struct io_context *, struct file_descriptor *, int 
firstIndex);
+extern int new_fd_etc(struct io_context *, struct file_descriptor *,
+       int firstIndex);
 extern int new_fd(struct io_context *, struct file_descriptor *);
 extern struct file_descriptor *get_fd(struct io_context *, int);
 extern struct file_descriptor *get_open_fd(struct io_context *, int);
-extern void close_fd(struct file_descriptor *descriptor);
+extern void close_fd(struct io_context *context,
+       struct file_descriptor *descriptor);
 extern status_t close_fd_index(struct io_context *context, int fd);
 extern void put_fd(struct file_descriptor *descriptor);
 extern void disconnect_fd(struct file_descriptor *descriptor);
@@ -92,11 +98,13 @@ extern bool fd_is_valid(int fd, bool kernel);
 extern struct vnode *fd_vnode(struct file_descriptor *descriptor);
 
 extern bool fd_close_on_exec(struct io_context *context, int fd);
-extern void fd_set_close_on_exec(struct io_context *context, int fd, bool 
closeFD);
+extern void fd_set_close_on_exec(struct io_context *context, int fd,
+       bool closeFD);
 
 static struct io_context *get_current_io_context(bool kernel);
 
-extern status_t user_fd_kernel_ioctl(int fd, ulong op, void *buffer, size_t 
length);
+extern status_t user_fd_kernel_ioctl(int fd, ulong op, void *buffer,
+       size_t length);
 
 /* The prototypes of the (sys|user)_ functions are currently defined in vfs.h 
*/
 
diff --git a/headers/private/kernel/vfs.h b/headers/private/kernel/vfs.h
index fc818ba68d..f754c75973 100644
--- a/headers/private/kernel/vfs.h
+++ b/headers/private/kernel/vfs.h
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016, Axel Dörfler, axeld@xxxxxxxxxxxxxxxx.
+ * Copyright 2002-2018, Axel Dörfler, axeld@xxxxxxxxxxxxxxxx.
  * Distributed under the terms of the MIT License.
  *
  * Copyright 2001-2002, Travis Geiselbrecht. All rights reserved.
@@ -124,6 +124,8 @@ status_t    vfs_entry_ref_to_path(dev_t device, ino_t 
inode, const char *leaf,
                                bool kernel, char *path, size_t pathLength);
 status_t       vfs_get_cwd(dev_t *_mountID, ino_t *_vnodeID);
 void           vfs_unlock_vnode_if_locked(struct file_descriptor *descriptor);
+status_t       vfs_release_posix_lock(io_context* context,
+                               struct file_descriptor* descriptor);
 status_t       vfs_unmount(dev_t mountID, uint32 flags);
 status_t       vfs_disconnect_vnode(dev_t mountID, ino_t vnodeID);
 status_t       vfs_resolve_parent(struct vnode* parent, dev_t* device,
diff --git a/src/system/kernel/fs/fd.cpp b/src/system/kernel/fs/fd.cpp
index 25b85d9ea3..225a47a6c8 100644
--- a/src/system/kernel/fs/fd.cpp
+++ b/src/system/kernel/fs/fd.cpp
@@ -1,6 +1,6 @@
 /*
  * Copyright 2009-2011, Ingo Weinhold, ingo_weinhold@xxxxxx.
- * Copyright 2002-2015, Axel Dörfler, axeld@xxxxxxxxxxxxxxxx.
+ * Copyright 2002-2018, Axel Dörfler, axeld@xxxxxxxxxxxxxxxx.
  * Distributed under the terms of the MIT License.
  */
 
@@ -237,8 +237,12 @@ put_fd(struct file_descriptor* descriptor)
        its close hook when appropriate.
 */
 void
-close_fd(struct file_descriptor* descriptor)
+close_fd(struct io_context* context, struct file_descriptor* descriptor)
 {
+       // POSIX advisory locks need to be released when any file descriptor 
closes
+       if (descriptor->type == FDTYPE_FILE)
+               vfs_release_posix_lock(context, descriptor);
+
        if (atomic_add(&descriptor->open_count, -1) == 1) {
                vfs_unlock_vnode_if_locked(descriptor);
 
@@ -256,7 +260,7 @@ close_fd_index(struct io_context* context, int fd)
        if (descriptor == NULL)
                return B_FILE_ERROR;
 
-       close_fd(descriptor);
+       close_fd(context, descriptor);
        put_fd(descriptor);
                // the reference associated with the slot
 
@@ -452,7 +456,7 @@ dup2_fd(int oldfd, int newfd, bool kernel)
        // Say bye bye to the evicted fd
        if (evicted) {
                deselect_select_infos(evicted, selectInfos, true);
-               close_fd(evicted);
+               close_fd(context, evicted);
                put_fd(evicted);
        }
 
@@ -605,7 +609,7 @@ select_fd(int32 fd, struct select_info* info, bool kernel)
                deselect_select_infos(descriptor, info, false);
 
                // Release our open reference of the descriptor.
-               close_fd(descriptor);
+               close_fd(context, descriptor);
                return B_FILE_ERROR;
        }
 
diff --git a/src/system/kernel/fs/vfs.cpp b/src/system/kernel/fs/vfs.cpp
index 98c51c5ea6..4f1657a55c 100644
--- a/src/system/kernel/fs/vfs.cpp
+++ b/src/system/kernel/fs/vfs.cpp
@@ -172,6 +172,7 @@ namespace {
 
 struct advisory_lock : public DoublyLinkedListLinkImpl<advisory_lock> {
        list_link               link;
+       void*                   bound_to;
        team_id                 team;
        pid_t                   session;
        off_t                   start;
@@ -1628,7 +1629,8 @@ test_advisory_lock(struct vnode* vnode, struct flock* 
flock)
        if \a flock is NULL.
 */
 static status_t
-release_advisory_lock(struct vnode* vnode, struct flock* flock)
+release_advisory_lock(struct vnode* vnode, struct io_context* context,
+       struct file_descriptor* descriptor, struct flock* flock)
 {
        FUNCTION(("release_advisory_lock(vnode = %p, flock = %p)\n", vnode, 
flock));
 
@@ -1636,10 +1638,6 @@ release_advisory_lock(struct vnode* vnode, struct flock* 
flock)
        if (locking == NULL)
                return B_OK;
 
-       // TODO: use the thread ID instead??
-       team_id team = team_get_current_team_id();
-       pid_t session = thread_get_current_thread()->team->session_id;
-
        // find matching lock entries
 
        LockList::Iterator iterator = locking->locks.GetIterator();
@@ -1647,9 +1645,12 @@ release_advisory_lock(struct vnode* vnode, struct flock* 
flock)
                struct advisory_lock* lock = iterator.Next();
                bool removeLock = false;
 
-               if (lock->session == session)
+               if (descriptor != NULL && lock->bound_to == descriptor) {
+                       // Remove flock() locks
                        removeLock = true;
-               else if (lock->team == team && advisory_lock_intersects(lock, 
flock)) {
+               } else if (lock->bound_to == context
+                               && advisory_lock_intersects(lock, flock)) {
+                       // Remove POSIX locks
                        bool endsBeyond = false;
                        bool startsBefore = false;
                        if (flock != NULL) {
@@ -1678,6 +1679,7 @@ release_advisory_lock(struct vnode* vnode, struct flock* 
flock)
 
                                lock->end = flock->l_start - 1;
 
+                               secondLock->bound_to = context;
                                secondLock->team = lock->team;
                                secondLock->session = lock->session;
                                // values must already be normalized when 
getting here
@@ -1735,19 +1737,22 @@ release_advisory_lock(struct vnode* vnode, struct 
flock* flock)
        will wait for the lock to become available, if there are any collisions
        (it will return B_PERMISSION_DENIED in this case if \a wait is \c 
false).
 
-       If \a session is -1, POSIX semantics are used for this lock. Otherwise,
+       If \a descriptor is NULL, POSIX semantics are used for this lock. 
Otherwise,
        BSD flock() semantics are used, that is, all children can unlock the 
file
        in question (we even allow parents to remove the lock, though, but that
        seems to be in line to what the BSD's are doing).
 */
 static status_t
-acquire_advisory_lock(struct vnode* vnode, pid_t session, struct flock* flock,
-       bool wait)
+acquire_advisory_lock(struct vnode* vnode, io_context* context,
+       struct file_descriptor* descriptor, struct flock* flock, bool wait)
 {
        FUNCTION(("acquire_advisory_lock(vnode = %p, flock = %p, wait = %s)\n",
                vnode, flock, wait ? "yes" : "no"));
+       dprintf("acquire_advisory_lock(vnode = %p, flock = %p, wait = %s)\n",
+               vnode, flock, wait ? "yes" : "no");
 
        bool shared = flock->l_type == F_RDLCK;
+       void* boundTo = descriptor != NULL ? (void*)descriptor : (void*)context;
        status_t status = B_OK;
 
        // TODO: do deadlock detection!
@@ -1771,7 +1776,8 @@ acquire_advisory_lock(struct vnode* vnode, pid_t session, 
struct flock* flock,
                        struct advisory_lock* lock = iterator.Next();
 
                        // TODO: locks from the same team might be joinable!
-                       if (lock->team != team && 
advisory_lock_intersects(lock, flock)) {
+                       if ((lock->team != team || lock->bound_to != boundTo)
+                                       && advisory_lock_intersects(lock, 
flock)) {
                                // locks do overlap
                                if (!shared || !lock->shared) {
                                        // we need to wait
@@ -1788,7 +1794,7 @@ acquire_advisory_lock(struct vnode* vnode, pid_t session, 
struct flock* flock,
 
                if (!wait) {
                        put_advisory_locking(locking);
-                       return session != -1 ? B_WOULD_BLOCK : 
B_PERMISSION_DENIED;
+                       return descriptor != NULL ? B_WOULD_BLOCK : 
B_PERMISSION_DENIED;
                }
 
                status = switch_sem_etc(locking->lock, waitForLock, 1,
@@ -1809,8 +1815,9 @@ acquire_advisory_lock(struct vnode* vnode, pid_t session, 
struct flock* flock,
                return B_NO_MEMORY;
        }
 
+       lock->bound_to = boundTo;
        lock->team = team_get_current_team_id();
-       lock->session = session;
+       lock->session = thread_get_current_thread()->team->session_id;
        // values must already be normalized when getting here
        lock->start = flock->l_start;
        lock->end = flock->l_start - 1 + flock->l_len;
@@ -3645,7 +3652,7 @@ free_io_context(io_context* context)
 
        for (i = 0; i < context->table_size; i++) {
                if (struct file_descriptor* descriptor = context->fds[i]) {
-                       close_fd(descriptor);
+                       close_fd(context, descriptor);
                        put_fd(descriptor);
                }
        }
@@ -4877,6 +4884,19 @@ vfs_unlock_vnode_if_locked(struct file_descriptor* 
descriptor)
 }
 
 
+/*!    Releases any POSIX locks on the file descriptor. */
+status_t
+vfs_release_posix_lock(io_context* context, struct file_descriptor* descriptor)
+{
+       struct vnode* vnode = descriptor->u.vnode;
+
+       if (HAS_FS_CALL(vnode, release_lock))
+               return FS_CALL(vnode, release_lock, descriptor->cookie, NULL);
+
+       return release_advisory_lock(vnode, context, NULL, NULL);
+}
+
+
 /*!    Closes all file descriptors of the specified I/O context that
        have the O_CLOEXEC flag set.
 */
@@ -4901,7 +4921,7 @@ vfs_exec_io_context(io_context* context)
                mutex_unlock(&context->io_mutex);
 
                if (remove) {
-                       close_fd(descriptor);
+                       close_fd(context, descriptor);
                        put_fd(descriptor);
                }
        }
@@ -5634,7 +5654,7 @@ file_close(struct file_descriptor* descriptor)
                if (HAS_FS_CALL(vnode, release_lock))
                        status = FS_CALL(vnode, release_lock, 
descriptor->cookie, NULL);
                else
-                       status = release_advisory_lock(vnode, NULL);
+                       status = release_advisory_lock(vnode, NULL, descriptor, 
NULL);
        }
        return status;
 }
@@ -6083,8 +6103,9 @@ common_fcntl(int fd, int op, size_t argument, bool kernel)
        FUNCTION(("common_fcntl(fd = %d, op = %d, argument = %lx, %s)\n",
                fd, op, argument, kernel ? "kernel" : "user"));
 
-       struct file_descriptor* descriptor = 
get_fd(get_current_io_context(kernel),
-               fd);
+       struct io_context* context = get_current_io_context(kernel);
+
+       struct file_descriptor* descriptor = get_fd(context, fd);
        if (descriptor == NULL)
                return B_FILE_ERROR;
 
@@ -6108,7 +6129,6 @@ common_fcntl(int fd, int op, size_t argument, bool kernel)
        switch (op) {
                case F_SETFD:
                {
-                       struct io_context* context = 
get_current_io_context(kernel);
                        // Set file descriptor flags
 
                        // O_CLOEXEC is the only flag available at this time
@@ -6122,8 +6142,6 @@ common_fcntl(int fd, int op, size_t argument, bool kernel)
 
                case F_GETFD:
                {
-                       struct io_context* context = 
get_current_io_context(kernel);
-
                        // Get file descriptor flags
                        mutex_lock(&context->io_mutex);
                        status = fd_close_on_exec(context, fd) ? FD_CLOEXEC : 0;
@@ -6160,8 +6178,6 @@ common_fcntl(int fd, int op, size_t argument, bool kernel)
                case F_DUPFD:
                case F_DUPFD_CLOEXEC:
                {
-                       struct io_context* context = 
get_current_io_context(kernel);
-
                        status = new_fd_etc(context, descriptor, (int)argument);
                        if (status >= 0) {
                                mutex_lock(&context->io_mutex);
@@ -6220,8 +6236,10 @@ common_fcntl(int fd, int op, size_t argument, bool 
kernel)
                                if (HAS_FS_CALL(vnode, release_lock)) {
                                        status = FS_CALL(vnode, release_lock, 
descriptor->cookie,
                                                &flock);
-                               } else
-                                       status = release_advisory_lock(vnode, 
&flock);
+                               } else {
+                                       status = release_advisory_lock(vnode, 
context, NULL,
+                                               &flock);
+                               }
                        } else {
                                // the open mode must match the lock type
                                if (((descriptor->open_mode & O_RWMASK) == 
O_RDONLY
@@ -6234,7 +6252,7 @@ common_fcntl(int fd, int op, size_t argument, bool kernel)
                                                status = FS_CALL(vnode, 
acquire_lock,
                                                        descriptor->cookie, 
&flock, op == F_SETLKW);
                                        } else {
-                                               status = 
acquire_advisory_lock(vnode, -1,
+                                               status = 
acquire_advisory_lock(vnode, context, NULL,
                                                        &flock, op == F_SETLKW);
                                        }
                                }
@@ -9131,14 +9149,13 @@ _user_flock(int fd, int operation)
                if (HAS_FS_CALL(vnode, release_lock))
                        status = FS_CALL(vnode, release_lock, 
descriptor->cookie, &flock);
                else
-                       status = release_advisory_lock(vnode, &flock);
+                       status = release_advisory_lock(vnode, NULL, descriptor, 
&flock);
        } else {
                if (HAS_FS_CALL(vnode, acquire_lock)) {
                        status = FS_CALL(vnode, acquire_lock, 
descriptor->cookie, &flock,
                                (operation & LOCK_NB) == 0);
                } else {
-                       status = acquire_advisory_lock(vnode,
-                               thread_get_current_thread()->team->session_id, 
&flock,
+                       status = acquire_advisory_lock(vnode, NULL, descriptor, 
&flock,
                                (operation & LOCK_NB) == 0);
                }
        }


Other related posts:

  • » [haiku-commits] haiku: hrev51970 - src/system/kernel/fs headers/private/kernel/fs headers/private/kernel - Axel Dörfler