From 5c95896499dd6f72c8fc9be84b0da880571731da Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Mon, 25 Oct 2004 04:24:58 +0000 Subject: r3189: improved the share_conflict() logic (both in terms of readability and correctness). pvfs now passes the BASE-RENAME test. (This used to be commit 4cf3f65a5c19fdad62a0bdef225b2d9002cf8c8b) --- source4/ntvfs/common/opendb.c | 74 +++++++++++++++++++++++++------------- source4/ntvfs/posix/pvfs_open.c | 15 +++++--- source4/ntvfs/posix/pvfs_rename.c | 11 ++++-- source4/ntvfs/posix/pvfs_unlink.c | 9 ++--- source4/script/tests/test_posix.sh | 8 ++--- 5 files changed, 78 insertions(+), 39 deletions(-) diff --git a/source4/ntvfs/common/opendb.c b/source4/ntvfs/common/opendb.c index 5c962fbad0..a3924daf8e 100644 --- a/source4/ntvfs/common/opendb.c +++ b/source4/ntvfs/common/opendb.c @@ -145,41 +145,38 @@ struct odb_lock *odb_lock(TALLOC_CTX *mem_ctx, return lck; } - /* determine if two odb_entry structures conflict */ static BOOL share_conflict(struct odb_entry *e1, struct odb_entry *e2) { - uint32_t m1, m2; - - m1 = e1->access_mask & (SA_RIGHT_FILE_WRITE_DATA | SA_RIGHT_FILE_READ_DATA); - m2 = e2->share_access & - (NTCREATEX_SHARE_ACCESS_WRITE | NTCREATEX_SHARE_ACCESS_READ); +#define CHECK_MASK(am, sa, right, share) if (((am) & (right)) && !((sa) & (share))) return True - if ((m1 & m2) != m1) { - return True; + /* if either open involves no read.write or delete access then + it can't conflict */ + if (!(e1->access_mask & (SA_RIGHT_FILE_WRITE_DATA | SA_RIGHT_FILE_READ_DATA | STD_RIGHT_DELETE_ACCESS))) { + return False; + } + if (!(e2->access_mask & (SA_RIGHT_FILE_WRITE_DATA | SA_RIGHT_FILE_READ_DATA | STD_RIGHT_DELETE_ACCESS))) { + return False; } - m1 = e2->access_mask & (SA_RIGHT_FILE_WRITE_DATA | SA_RIGHT_FILE_READ_DATA); - m2 = e1->share_access & - (NTCREATEX_SHARE_ACCESS_WRITE | NTCREATEX_SHARE_ACCESS_READ); + /* check the basic share access */ + CHECK_MASK(e1->access_mask, e2->share_access, SA_RIGHT_FILE_WRITE_DATA, NTCREATEX_SHARE_ACCESS_WRITE); + CHECK_MASK(e2->access_mask, e1->share_access, SA_RIGHT_FILE_WRITE_DATA, NTCREATEX_SHARE_ACCESS_WRITE); - if ((m1 & m2) != m1) { - return True; - } + CHECK_MASK(e1->access_mask, e2->share_access, SA_RIGHT_FILE_READ_DATA, NTCREATEX_SHARE_ACCESS_READ); + CHECK_MASK(e2->access_mask, e1->share_access, SA_RIGHT_FILE_READ_DATA, NTCREATEX_SHARE_ACCESS_READ); + CHECK_MASK(e1->access_mask, e2->share_access, STD_RIGHT_DELETE_ACCESS, NTCREATEX_SHARE_ACCESS_DELETE); + CHECK_MASK(e2->access_mask, e1->share_access, STD_RIGHT_DELETE_ACCESS, NTCREATEX_SHARE_ACCESS_DELETE); + + /* if a delete is pending then a second open is not allowed */ if ((e1->create_options & NTCREATEX_OPTIONS_DELETE_ON_CLOSE) || (e2->create_options & NTCREATEX_OPTIONS_DELETE_ON_CLOSE)) { return True; } - if ((e1->access_mask & STD_RIGHT_DELETE_ACCESS) && - !(e2->share_access & NTCREATEX_SHARE_ACCESS_DELETE)) { - return True; - } - - return False; } @@ -338,20 +335,49 @@ NTSTATUS odb_set_create_options(struct odb_lock *lck, /* - determine if a file is open + determine if a file can be opened with the given share_access, + create_options and access_mask */ -BOOL odb_is_open(struct odb_context *odb, DATA_BLOB *key) +NTSTATUS odb_can_open(struct odb_context *odb, DATA_BLOB *key, + uint32_t share_access, uint32_t create_options, + uint32_t access_mask) { TDB_DATA dbuf; TDB_DATA kbuf; + struct odb_entry *elist; + int i, count; + struct odb_entry e; kbuf.dptr = key->data; kbuf.dsize = key->length; dbuf = tdb_fetch(odb->w->tdb, kbuf); if (dbuf.dptr == NULL) { - return False; + return NT_STATUS_OK; } + + elist = (struct odb_entry *)dbuf.dptr; + count = dbuf.dsize / sizeof(struct odb_entry); + + if (count == 0) { + free(dbuf.dptr); + return NT_STATUS_OK; + } + + e.server = odb->server; + e.tid = odb->tid; + e.fnum = -1; + e.share_access = share_access; + e.create_options = create_options; + e.access_mask = access_mask; + + for (i=0;iodb_context, &key); + status = odb_can_open(pvfs->odb_context, &key, + NTCREATEX_SHARE_ACCESS_READ | + NTCREATEX_SHARE_ACCESS_WRITE | + NTCREATEX_SHARE_ACCESS_DELETE, + 0, STD_RIGHT_DELETE_ACCESS); + + return status; } diff --git a/source4/ntvfs/posix/pvfs_rename.c b/source4/ntvfs/posix/pvfs_rename.c index 89d6a0da14..6116f6c7bf 100644 --- a/source4/ntvfs/posix/pvfs_rename.c +++ b/source4/ntvfs/posix/pvfs_rename.c @@ -48,9 +48,14 @@ NTSTATUS pvfs_rename(struct ntvfs_module_context *ntvfs, return status; } - if (pvfs_is_open(pvfs, name1) || - pvfs_is_open(pvfs, name2)) { - return NT_STATUS_SHARING_VIOLATION; + status = pvfs_can_delete(pvfs, name1); + if (!NT_STATUS_IS_OK(status)) { + return status; + } + + status = pvfs_can_delete(pvfs, name2); + if (!NT_STATUS_IS_OK(status)) { + return status; } if (name1->has_wildcard || name2->has_wildcard) { diff --git a/source4/ntvfs/posix/pvfs_unlink.c b/source4/ntvfs/posix/pvfs_unlink.c index 5733722ad5..10a27a5de7 100644 --- a/source4/ntvfs/posix/pvfs_unlink.c +++ b/source4/ntvfs/posix/pvfs_unlink.c @@ -47,6 +47,11 @@ static NTSTATUS pvfs_unlink_one(struct pvfs_state *pvfs, TALLOC_CTX *mem_ctx, return NT_STATUS_OBJECT_NAME_NOT_FOUND; } + status = pvfs_can_delete(pvfs, name); + if (!NT_STATUS_IS_OK(status)) { + return status; + } + /* finally try the actual unlink */ if (unlink(name->full_name) == -1) { status = pvfs_map_errno(pvfs, errno); @@ -80,10 +85,6 @@ NTSTATUS pvfs_unlink(struct ntvfs_module_context *ntvfs, return NT_STATUS_OBJECT_NAME_NOT_FOUND; } - if (pvfs_is_open(pvfs, name)) { - return NT_STATUS_SHARING_VIOLATION; - } - dir = talloc_p(req, struct pvfs_dir); if (dir == NULL) { return NT_STATUS_NO_MEMORY; diff --git a/source4/script/tests/test_posix.sh b/source4/script/tests/test_posix.sh index d17d428642..414cf30a67 100755 --- a/source4/script/tests/test_posix.sh +++ b/source4/script/tests/test_posix.sh @@ -30,18 +30,18 @@ testit() { tests="BASE-FDPASS BASE-LOCK1 BASE-LOCK2 BASE-LOCK3 BASE-LOCK4" tests="$tests BASE-LOCK5 BASE-LOCK6 BASE-LOCK7 BASE-UNLINK BASE-ATTR" -tests="$tests BASE-TRANS2 BASE-NEGNOWAIT BASE-DIR" +tests="$tests BASE-NEGNOWAIT BASE-DIR" tests="$tests BASE-DENY2 BASE-TCON BASE-TCONDEV BASE-RW1" tests="$tests BASE-DENY3 BASE-XCOPY" tests="$tests BASE-DELETE BASE-PROPERTIES BASE-MANGLE" tests="$tests BASE-CHKPATH BASE-SECLEAK" tests="$tests RAW-QFSINFO RAW-QFILEINFO RAW-SFILEINFO-BUG" -tests="$tests RAW-LOCK RAW-SEEK RAW-CONTEXT" +tests="$tests RAW-LOCK RAW-SEEK RAW-CONTEXT BASE-RENAME" -soon="BASE-DIR1 BASE-DENY1 BASE-VUID BASE-OPEN BASE-DEFER_OPEN BASE-RENAME BASE-OPENATTR BASE-CHARSET" +soon="BASE-DIR1 BASE-DENY1 BASE-VUID BASE-OPEN BASE-DEFER_OPEN BASE-OPENATTR BASE-CHARSET" soon="$soon RAW-SFILEINFO RAW-SEARCH RAW-OPEN RAW-MKDIR RAW-OPLOCK RAW-NOTIFY RAW-MUX RAW-IOCTL" -soon="$soon RAW-CHKPATH RAW-UNLINK RAW-READ RAW-WRITE RAW-RENAME RAW-CLOSE" +soon="$soon RAW-CHKPATH RAW-UNLINK RAW-READ RAW-WRITE RAW-RENAME RAW-CLOSE BASE-TRANS2" for t in $tests; do if [ ! -z "$start" -a "$start" != $t ]; then -- cgit