r3189: improved the share_conflict() logic (both in terms of readability and
authorAndrew Tridgell <tridge@samba.org>
Mon, 25 Oct 2004 04:24:58 +0000 (04:24 +0000)
committerGerald (Jerry) Carter <jerry@samba.org>
Wed, 10 Oct 2007 18:04:38 +0000 (13:04 -0500)
correctness). pvfs now passes the BASE-RENAME test.
(This used to be commit 4cf3f65a5c19fdad62a0bdef225b2d9002cf8c8b)

source4/ntvfs/common/opendb.c
source4/ntvfs/posix/pvfs_open.c
source4/ntvfs/posix/pvfs_rename.c
source4/ntvfs/posix/pvfs_unlink.c
source4/script/tests/test_posix.sh

index 5c962fbad006f3c0ed53f81e370c48e9fed6bd53..a3924daf8eef2fc7fd41d8edb0e501d4f2979a9e 100644 (file)
@@ -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;i<count;i++) {
+               if (share_conflict(elist+i, &e)) {
+                       if (dbuf.dptr) free(dbuf.dptr);
+                       return NT_STATUS_SHARING_VIOLATION;
+               }
+       }
+
        free(dbuf.dptr);
-       return True;
+       return NT_STATUS_OK;
 }
index badd18d370e6296950ff53b9d2ff28b3b885101c..b66b3725db7263e8d97c164b89cdefc2947d4673 100644 (file)
@@ -723,17 +723,24 @@ NTSTATUS pvfs_change_create_options(struct pvfs_state *pvfs,
 
 
 /*
-  determine if a file is open - used to prevent some operations on open files
+  determine if a file can be deleted, or if it is prevented by an
+  already open file
 */
-BOOL pvfs_is_open(struct pvfs_state *pvfs, struct pvfs_filename *name)
+NTSTATUS pvfs_can_delete(struct pvfs_state *pvfs, struct pvfs_filename *name)
 {
        NTSTATUS status;
        DATA_BLOB key;
 
        status = pvfs_locking_key(name, name, &key);
        if (!NT_STATUS_IS_OK(status)) {
-               return False;
+               return NT_STATUS_NO_MEMORY;
        }
 
-       return odb_is_open(pvfs->odb_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;
 }
index 89d6a0da14cecf3843d720128d1d7fc6e8426b25..6116f6c7bfbe2198f3c9e53cfc3e2d49a5a84bfb 100644 (file)
@@ -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) {
index 5733722ad5ff2f4663532b710889644444eec5d4..10a27a5de7d545388353fc47030ec37e35b576e5 100644 (file)
@@ -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;
index d17d428642248c4af6a95c4b8ee610b9d8cf6027..414cf30a676a7a2c4c3ee4d226fc62232e5193d3 100755 (executable)
@@ -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