From 74bd1e47ed022a7c163d761f622be6debfd9933a Mon Sep 17 00:00:00 2001 From: Alexandre Julliard Date: Sat, 27 Mar 2004 20:48:42 +0000 Subject: [PATCH] Check file sharing permissions based on the file inode instead of the file name. Added regression test for sharing permissions. --- dlls/kernel/tests/file.c | 61 ++++++++++++++++++++++++++++++++++++++++ server/fd.c | 49 +++++++++++++++++++++++++++++++- server/file.c | 59 +++----------------------------------- server/file.h | 4 ++- 4 files changed, 116 insertions(+), 57 deletions(-) diff --git a/dlls/kernel/tests/file.c b/dlls/kernel/tests/file.c index 40366e95b33..f7341061b9a 100644 --- a/dlls/kernel/tests/file.c +++ b/dlls/kernel/tests/file.c @@ -914,6 +914,66 @@ static void test_LockFile(void) DeleteFileA( filename ); } +static inline int is_sharing_compatible( DWORD access1, DWORD sharing1, DWORD access2, DWORD sharing2 ) +{ + if (!access1 || !access2) return 1; + if ((access1 & GENERIC_READ) && !(sharing2 & FILE_SHARE_READ)) return 0; + if ((access1 & GENERIC_WRITE) && !(sharing2 & FILE_SHARE_WRITE)) return 0; + if ((access2 & GENERIC_READ) && !(sharing1 & FILE_SHARE_READ)) return 0; + if ((access2 & GENERIC_WRITE) && !(sharing1 & FILE_SHARE_WRITE)) return 0; + return 1; +} + +static void test_file_sharing(void) +{ + static const DWORD access_modes[4] = { 0, GENERIC_READ, GENERIC_WRITE, GENERIC_READ|GENERIC_WRITE }; + static const DWORD sharing_modes[4] = { 0, FILE_SHARE_READ, FILE_SHARE_WRITE, FILE_SHARE_READ|FILE_SHARE_WRITE }; + int a1, s1, a2, s2; + + for (a1 = 0; a1 < 4; a1++) + { + for (s1 = 0; s1 < 4; s1++) + { + HANDLE h = CreateFileA( filename, access_modes[a1], sharing_modes[s1], + NULL, CREATE_ALWAYS, 0, 0 ); + if (h == INVALID_HANDLE_VALUE) + { + ok(0,"couldn't create file \"%s\" (err=%ld)\n",filename,GetLastError()); + return; + } + for (a2 = 0; a2 < 4; a2++) + { + for (s2 = 0; s2 < 4; s2++) + { + HANDLE h2 = CreateFileA( filename, access_modes[a2], sharing_modes[s2], + NULL, CREATE_ALWAYS, 0, 0 ); + if (is_sharing_compatible( access_modes[a1], sharing_modes[s1], + access_modes[a2], sharing_modes[s2] )) + { + ok( h2 != INVALID_HANDLE_VALUE, + "open failed for modes %lx/%lx/%lx/%lx err %ld\n", + access_modes[a1], sharing_modes[s1], + access_modes[a2], sharing_modes[s2], GetLastError() ); + } + else + { + ok( h2 == INVALID_HANDLE_VALUE, + "open succeeded for modes %lx/%lx/%lx/%lx\n", + access_modes[a1], sharing_modes[s1], + access_modes[a2], sharing_modes[s2] ); + if (h2 == INVALID_HANDLE_VALUE) + ok( GetLastError() == ERROR_SHARING_VIOLATION, + "wrong error code %ld\n", GetLastError() ); + } + if (h2 != INVALID_HANDLE_VALUE) CloseHandle( h2 ); + } + } + CloseHandle( h ); + } + } + DeleteFileA( filename ); +} + static void test_FindFirstFileA() { HANDLE handle; @@ -1000,6 +1060,7 @@ START_TEST(file) test_FindFirstFileA(); test_FindNextFileA(); test_LockFile(); + test_file_sharing(); test_offset_in_overlapped_structure(); test_MapFile(); } diff --git a/server/fd.c b/server/fd.c index c41c4edb4dd..f9491c390f2 100644 --- a/server/fd.c +++ b/server/fd.c @@ -66,6 +66,8 @@ struct fd struct closed_fd *closed; /* structure to store the unix fd at destroy time */ struct object *user; /* object using this file descriptor */ struct list locks; /* list of locks on this fd */ + unsigned int access; /* file access (GENERIC_READ/WRITE) */ + unsigned int sharing; /* file sharing mode */ int unix_fd; /* unix file descriptor */ int fs_locks; /* can we use filesystem locks for this fd? */ int poll_index; /* index of fd in poll array */ @@ -817,6 +819,8 @@ struct fd *alloc_fd( const struct fd_ops *fd_user_ops, struct object *user ) fd->user = user; fd->inode = NULL; fd->closed = NULL; + fd->access = 0; + fd->sharing = 0; fd->unix_fd = -1; fd->fs_locks = 1; fd->poll_index = -1; @@ -831,10 +835,41 @@ struct fd *alloc_fd( const struct fd_ops *fd_user_ops, struct object *user ) return fd; } +/* check if the desired access is possible without violating */ +/* the sharing mode of other opens of the same file */ +static int check_sharing( struct fd *fd, unsigned int access, unsigned int sharing ) +{ + unsigned int existing_sharing = FILE_SHARE_READ | FILE_SHARE_WRITE; + unsigned int existing_access = 0; + struct list *ptr; + + /* if access mode is 0, sharing mode is ignored */ + if (!access) sharing = FILE_SHARE_READ|FILE_SHARE_WRITE; + fd->access = access; + fd->sharing = sharing; + + LIST_FOR_EACH( ptr, &fd->inode->open ) + { + struct fd *fd_ptr = LIST_ENTRY( ptr, struct fd, inode_entry ); + if (fd_ptr != fd) + { + existing_sharing &= fd_ptr->sharing; + existing_access |= fd_ptr->access; + } + } + + if ((access & GENERIC_READ) && !(existing_sharing & FILE_SHARE_READ)) return 0; + if ((access & GENERIC_WRITE) && !(existing_sharing & FILE_SHARE_WRITE)) return 0; + if ((existing_access & GENERIC_READ) && !(sharing & FILE_SHARE_READ)) return 0; + if ((existing_access & GENERIC_WRITE) && !(sharing & FILE_SHARE_WRITE)) return 0; + return 1; +} + /* open() wrapper using a struct fd */ /* the fd must have been created with alloc_fd */ /* on error the fd object is released */ -struct fd *open_fd( struct fd *fd, const char *name, int flags, mode_t *mode ) +struct fd *open_fd( struct fd *fd, const char *name, int flags, mode_t *mode, + unsigned int access, unsigned int sharing ) { struct stat st; struct closed_fd *closed_fd; @@ -873,6 +908,12 @@ struct fd *open_fd( struct fd *fd, const char *name, int flags, mode_t *mode ) fd->inode = inode; fd->closed = closed_fd; list_add_head( &inode->open, &fd->inode_entry ); + if (!check_sharing( fd, access, sharing )) + { + release_object( fd ); + set_error( STATUS_SHARING_VIOLATION ); + return NULL; + } } else { @@ -908,6 +949,12 @@ int get_unix_fd( struct fd *fd ) return fd->unix_fd; } +/* check if two file descriptors point to the same file */ +int is_same_file_fd( struct fd *fd1, struct fd *fd2 ) +{ + return fd1->inode == fd2->inode; +} + /* callback for event happening in the main poll() loop */ void fd_poll_event( struct fd *fd, int event ) { diff --git a/server/file.c b/server/file.c index 6b803f4cff1..1b2193019fe 100644 --- a/server/file.c +++ b/server/file.c @@ -56,20 +56,14 @@ struct file { struct object obj; /* object header */ struct fd *fd; /* file descriptor for this file */ - struct file *next; /* next file in hashing list */ char *name; /* file name */ unsigned int access; /* file access (GENERIC_READ/WRITE) */ unsigned int options; /* file options (FILE_DELETE_ON_CLOSE, FILE_SYNCHRONOUS...) */ - unsigned int sharing; /* file sharing mode */ int removable; /* is file on removable media? */ struct async_queue read_q; struct async_queue write_q; }; -#define NAME_HASH_SIZE 37 - -static struct file *file_hash[NAME_HASH_SIZE]; - static void file_dump( struct object *obj, int verbose ); static struct fd *file_get_fd( struct object *obj ); static void file_destroy( struct object *obj ); @@ -106,38 +100,6 @@ static inline int is_overlapped( const struct file *file ) return !(file->options & (FILE_SYNCHRONOUS_IO_ALERT | FILE_SYNCHRONOUS_IO_NONALERT)); } -static int get_name_hash( const char *name ) -{ - int hash = 0; - while (*name) hash ^= (unsigned char)*name++; - return hash % NAME_HASH_SIZE; -} - -/* check if the desired access is possible without violating */ -/* the sharing mode of other opens of the same file */ -static int check_sharing( const char *name, int hash, unsigned int access, - unsigned int sharing ) -{ - struct file *file; - unsigned int existing_sharing = FILE_SHARE_READ | FILE_SHARE_WRITE; - unsigned int existing_access = 0; - - for (file = file_hash[hash]; file; file = file->next) - { - if (strcmp( file->name, name )) continue; - existing_sharing &= file->sharing; - existing_access |= file->access; - } - if ((access & GENERIC_READ) && !(existing_sharing & FILE_SHARE_READ)) goto error; - if ((access & GENERIC_WRITE) && !(existing_sharing & FILE_SHARE_WRITE)) goto error; - if ((existing_access & GENERIC_READ) && !(sharing & FILE_SHARE_READ)) goto error; - if ((existing_access & GENERIC_WRITE) && !(sharing & FILE_SHARE_WRITE)) goto error; - return 1; - error: - set_error( STATUS_SHARING_VIOLATION ); - return 0; -} - /* create a file from a file descriptor */ /* if the function fails the fd is closed */ static struct file *create_file_for_fd( int fd, unsigned int access, unsigned int sharing ) @@ -147,10 +109,8 @@ static struct file *create_file_for_fd( int fd, unsigned int access, unsigned in if ((file = alloc_object( &file_ops ))) { file->name = NULL; - file->next = NULL; file->access = access; file->options = FILE_SYNCHRONOUS_IO_NONALERT; - file->sharing = sharing; file->removable = 0; if (!(file->fd = create_anonymous_fd( &file_fd_ops, fd, &file->obj ))) { @@ -167,7 +127,7 @@ static struct object *create_file( const char *nameptr, size_t len, unsigned int unsigned int attrs, int removable ) { struct file *file; - int hash, flags; + int flags; char *name; mode_t mode; @@ -175,10 +135,6 @@ static struct object *create_file( const char *nameptr, size_t len, unsigned int memcpy( name, nameptr, len ); name[len] = 0; - /* check sharing mode */ - hash = get_name_hash( name ); - if (!check_sharing( name, hash, access, sharing )) goto error; - switch(create) { case FILE_CREATE: flags = O_CREAT | O_EXCL; break; @@ -206,11 +162,8 @@ static struct object *create_file( const char *nameptr, size_t len, unsigned int file->access = access; file->options = options; - file->sharing = sharing; file->removable = removable; file->name = name; - file->next = file_hash[hash]; - file_hash[hash] = file; if (is_overlapped( file )) { init_async_queue (&file->read_q); @@ -219,7 +172,8 @@ static struct object *create_file( const char *nameptr, size_t len, unsigned int /* FIXME: should set error to STATUS_OBJECT_NAME_COLLISION if file existed before */ if (!(file->fd = alloc_fd( &file_fd_ops, &file->obj )) || - !(file->fd = open_fd( file->fd, name, flags | O_NONBLOCK | O_LARGEFILE, &mode ))) + !(file->fd = open_fd( file->fd, name, flags | O_NONBLOCK | O_LARGEFILE, + &mode, access, sharing))) { release_object( file ); return NULL; @@ -249,7 +203,7 @@ static struct object *create_file( const char *nameptr, size_t len, unsigned int /* check if two file objects point to the same file */ int is_same_file( struct file *file1, struct file *file2 ) { - return !strcmp( file1->name, file2->name ); + return is_same_file_fd( file1->fd, file2->fd ); } /* check if the file is on removable media */ @@ -434,11 +388,6 @@ static void file_destroy( struct object *obj ) if (file->name) { - /* remove it from the hashing list */ - struct file **pptr = &file_hash[get_name_hash( file->name )]; - while (*pptr && *pptr != file) pptr = &(*pptr)->next; - assert( *pptr ); - *pptr = (*pptr)->next; if (file->options & FILE_DELETE_ON_CLOSE) unlink( file->name ); free( file->name ); } diff --git a/server/file.h b/server/file.h index 85b47ae9177..8376e1b495d 100644 --- a/server/file.h +++ b/server/file.h @@ -45,11 +45,13 @@ struct fd_ops /* file descriptor functions */ extern struct fd *alloc_fd( const struct fd_ops *fd_user_ops, struct object *user ); -extern struct fd *open_fd( struct fd *fd, const char *name, int flags, mode_t *mode ); +extern struct fd *open_fd( struct fd *fd, const char *name, int flags, mode_t *mode, + unsigned int access, unsigned int sharing ); extern struct fd *create_anonymous_fd( const struct fd_ops *fd_user_ops, int unix_fd, struct object *user ); extern void *get_fd_user( struct fd *fd ); extern int get_unix_fd( struct fd *fd ); +extern int is_same_file_fd( struct fd *fd1, struct fd *fd2 ); extern void fd_poll_event( struct fd *fd, int event ); extern int check_fd_events( struct fd *fd, int events ); extern void set_fd_events( struct fd *fd, int events );