]> git.ipfire.org Git - thirdparty/samba.git/commitdiff
s3:smbd: allow reset_share_mode_entry() to handle more than one durable handle
authorStefan Metzmacher <metze@samba.org>
Thu, 29 Aug 2024 16:43:14 +0000 (18:43 +0200)
committerStefan Metzmacher <metze@samba.org>
Thu, 10 Oct 2024 12:47:33 +0000 (12:47 +0000)
This means that multiple durable handles with RH leases can
co-exist now... Before only the last remaining durable handle
was able to pass the SMB_VFS_DURABLE_DISCONNECT() step.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15649
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15651

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>
selftest/knownfail.d/smb2.durable-v2-open.bug15651 [deleted file]
source3/locking/share_mode_lock.c

diff --git a/selftest/knownfail.d/smb2.durable-v2-open.bug15651 b/selftest/knownfail.d/smb2.durable-v2-open.bug15651
deleted file mode 100644 (file)
index 1702a3a..0000000
+++ /dev/null
@@ -1,5 +0,0 @@
-^samba3.smb2.durable-v2-open.statRH-and-lease
-^samba3.smb2.durable-v2-open.two-same-lease
-^samba3.smb2.durable-v2-open.two-different-lease
-^samba3.smb2.durable-v2-open.keep-disconnected-rh-with-stat-open
-^samba3.smb2.durable-v2-open.keep-disconnected-rwh-with-stat-open
index 3fc7d56562a479762c11815a07bb2d35171eb89b..4bbccdcd3bd142111b5afc74d612d235e69ec189 100644 (file)
@@ -2703,16 +2703,25 @@ bool reset_share_mode_entry(
        struct share_mode_data *d = NULL;
        TDB_DATA key = locking_key(&id);
        struct locking_tdb_data *ltdb = NULL;
-       struct share_mode_entry e;
+       struct share_mode_entry e = { .pid.pid = 0 };
        struct share_mode_entry_buf e_buf;
+       size_t old_idx;
+       size_t new_idx;
+       bool found;
        NTSTATUS status;
-       int cmp;
        bool ret = false;
        bool ok;
+       struct file_id_buf id_buf;
+       struct server_id_buf pid_buf1;
+       struct server_id_buf pid_buf2;
+       size_t low_idx1, low_idx2, low_num;
+       size_t mid_idx1, mid_idx2, mid_num;
+       size_t high_idx1, high_idx2, high_num;
+       TDB_DATA dbufs[4];
+       size_t num_dbufs = 0;
 
        status = share_mode_lock_access_private_data(lck, &d);
        if (!NT_STATUS_IS_OK(status)) {
-               struct file_id_buf id_buf;
                /* Any error recovery possible here ? */
                DBG_ERR("share_mode_lock_access_private_data() failed for "
                        "%s - %s\n",
@@ -2728,29 +2737,54 @@ bool reset_share_mode_entry(
                return false;
        }
 
-       if (ltdb->num_share_entries != 1) {
-               DBG_DEBUG("num_share_modes=%zu\n", ltdb->num_share_entries);
-               goto done;
-       }
+       DBG_DEBUG("%s - num_share_modes=%zu\n",
+                 file_id_str_buf(id, &id_buf),
+                 ltdb->num_share_entries);
 
-       ok = share_mode_entry_get(ltdb->share_entries, &e);
-       if (!ok) {
-               DBG_WARNING("share_mode_entry_get failed\n");
+       new_idx = share_mode_entry_find(
+               ltdb->share_entries,
+               ltdb->num_share_entries,
+               new_pid,
+               new_share_file_id,
+               &e,
+               &found);
+       if (found) {
+               DBG_ERR("%s - num_share_modes=%zu "
+                       "found NEW[%s][%"PRIu64"]\n",
+                       file_id_str_buf(id, &id_buf),
+                       ltdb->num_share_entries,
+                       server_id_str_buf(new_pid, &pid_buf2),
+                       new_share_file_id);
                goto done;
        }
 
-       cmp = share_mode_entry_cmp(
-               old_pid, old_share_file_id, e.pid, e.share_file_id);
-       if (cmp != 0) {
-               struct server_id_buf tmp1, tmp2;
-               DBG_WARNING("Expected pid=%s, file_id=%"PRIu64", "
-                           "got pid=%s, file_id=%"PRIu64"\n",
-                           server_id_str_buf(old_pid, &tmp1),
-                           old_share_file_id,
-                           server_id_str_buf(e.pid, &tmp2),
-                           e.share_file_id);
+       old_idx = share_mode_entry_find(
+               ltdb->share_entries,
+               ltdb->num_share_entries,
+               old_pid,
+               old_share_file_id,
+               &e,
+               &found);
+       if (!found) {
+               DBG_WARNING("%s - num_share_modes=%zu "
+                           "OLD[%s][%"PRIu64"] not found\n",
+                           file_id_str_buf(id, &id_buf),
+                           ltdb->num_share_entries,
+                           server_id_str_buf(old_pid, &pid_buf1),
+                           old_share_file_id);
                goto done;
        }
+       DBG_DEBUG("%s - num_share_modes=%zu "
+                 "OLD[%s][%"PRIu64"] => idx=%zu "
+                 "NEW[%s][%"PRIu64"] => idx=%zu\n",
+                 file_id_str_buf(id, &id_buf),
+                 ltdb->num_share_entries,
+                 server_id_str_buf(old_pid, &pid_buf1),
+                 old_share_file_id,
+                 old_idx,
+                 server_id_str_buf(new_pid, &pid_buf2),
+                 new_share_file_id,
+                 new_idx);
 
        e.pid = new_pid;
        if (new_mid != UINT64_MAX) {
@@ -2764,11 +2798,248 @@ bool reset_share_mode_entry(
                goto done;
        }
 
-       ltdb->share_entries = e_buf.buf;
+       /*
+        * The logic to remove the existing
+        * entry and add the new one at the
+        * same time is a bit complex because
+        * we need to keep the entries sorted.
+        *
+        * The following examples should catch
+        * the corner cases and show that
+        * the {low,mid,high}_{idx1,num} are
+        * correctly calculated and the new
+        * entry is put before or after the mid
+        * elements...
+        *
+        * 1.
+        *    0
+        *    1
+        *    2  <- old_idx
+        *          new_idx -> 3
+        *    3
+        *    4
+        *
+        *    low_idx1 = 0;
+        *    low_idx2 = MIN(old_idx, new_idx);  => 2
+        *    low_num = low_idx2 - low_idx1; => 2
+        *
+        *    if (new < old) => new; => no
+        *
+        *    mid_idx1 = MIN(old_idx+1, new_idx); => 3
+        *    mid_idx2 = MAX(old_idx, new_idx); => 3
+        *    mid_num = mid_idx2 - mid_idx1; => 0
+        *
+        *    if (new >= old) => new; => yes
+        *
+        *    high_idx1 = MAX(old_idx+1, new_idx); => 3
+        *    high_idx2 = num_share_entries; => 5
+        *    high_num = high_idx2 - high_idx1 = 2
+        *
+        * 2.
+        *    0
+        *    1
+        *          new_idx -> 2
+        *    2  <- old_idx
+        *    3
+        *    4
+        *
+        *    low_idx1 = 0;
+        *    low_idx2 = MIN(old_idx, new_idx);  => 2
+        *    low_num = low_idx2 - low_idx1; => 2
+        *
+        *    if (new < old) => new; => no
+        *
+        *    mid_idx1 = MIN(old_idx+1, new_idx); => 2
+        *    mid_idx2 = MAX(old_idx, new_idx); => 2
+        *    mid_num = mid_idx2 - mid_idx1; => 0
+        *
+        *    if (new >= old) => new; => yes
+        *
+        *    high_idx1 = MAX(old_idx+1, new_idx); => 3
+        *    high_idx2 = num_share_entries; => 5
+        *    high_num = high_idx2 - high_idx1 = 2
+        *
+        * 3.
+        *    0
+        *    1  <- old_idx
+        *    2
+        *          new_idx -> 3
+        *    3
+        *    4
+        *
+        *    low_idx1 = 0;
+        *    low_idx2 = MIN(old_idx, new_idx);  => 1
+        *    low_num = low_idx2 - low_idx1; => 1
+        *
+        *    if (new < old) => new; => no
+        *
+        *    mid_idx1 = MIN(old_idx+1, new_idx); => 2
+        *    mid_idx2 = MAX(old_idx, new_idx); => 3
+        *    mid_num = mid_idx2 - mid_idx1; => 1
+        *
+        *    if (new >= old) => new; => yes
+        *
+        *    high_idx1 = MAX(old_idx+1, new_idx); => 3
+        *    high_idx2 = num_share_entries; => 5
+        *    high_num = high_idx2 - high_idx1 = 2
+        *
+        * 4.
+        *    0
+        *          new_idx -> 1
+        *    1
+        *    2
+        *    3  <- old_idx
+        *    4
+        *
+        *    low_idx1 = 0;
+        *    low_idx2 = MIN(old_idx, new_idx);  => 1
+        *    low_num = low_idx2 - low_idx1; => 1
+        *
+        *    if (new < old) => new; => yes
+        *
+        *    mid_idx1 = MIN(old_idx+1, new_idx); => 1
+        *    mid_idx2 = MAX(old_idx, new_idx); => 3
+        *    mid_num = mid_idx2 - mid_idx1; => 2
+        *
+        *    if (new >= old) => new; => no
+        *
+        *    high_idx1 = MAX(old_idx+1, new_idx); => 4
+        *    high_idx2 = num_share_entries; => 5
+        *    high_num = high_idx2 - high_idx1 = 1
+        *
+        * 5.
+        *          new_idx -> 0
+        *    0
+        *    1
+        *    2
+        *    3
+        *    4  <- old_idx
+        *
+        *    low_idx1 = 0;
+        *    low_idx2 = MIN(old_idx, new_idx);  => 0
+        *    low_num = low_idx2 - low_idx1; => 0
+        *
+        *    if (new < old) => new; => yes
+        *
+        *    mid_idx1 = MIN(old_idx+1, new_idx); => 0
+        *    mid_idx2 = MAX(old_idx, new_idx); => 4
+        *    mid_num = mid_idx2 - mid_idx1; => 4
+        *
+        *    if (new >= old) => new; => no
+        *
+        *    high_idx1 = MAX(old_idx+1, new_idx); => 5
+        *    high_idx2 = num_share_entries; => 5
+        *    high_num = high_idx2 - high_idx1 = 0
+        *
+        * 6.
+        *          new_idx -> 0
+        *    0 <- old_idx
+        *
+        *    low_idx1 = 0;
+        *    low_idx2 = MIN(old_idx, new_idx);  => 0
+        *    low_num = low_idx2 - low_idx1; => 0
+        *
+        *    if (new < old) => new; => no
+        *
+        *    mid_idx1 = MIN(old_idx+1, new_idx); => 0
+        *    mid_idx2 = MAX(old_idx, new_idx); => 0
+        *    mid_num = mid_idx2 - mid_idx1; => 0
+        *
+        *    if (new >= old) => new; => yes
+        *
+        *    high_idx1 = MAX(old_idx+1, new_idx); => 1
+        *    high_idx2 = num_share_entries; => 1
+        *    high_num = high_idx2 - high_idx1 = 0
+        *
+        * 7.
+        *    0 <- old_idx
+        *          new_idx -> 1
+        *
+        *    low_idx1 = 0;
+        *    low_idx2 = MIN(old_idx, new_idx);  => 0
+        *    low_num = low_idx2 - low_idx1; => 0
+        *
+        *    if (new < old) => new; => no
+        *
+        *    mid_idx1 = MIN(old_idx+1, new_idx); => 1
+        *    mid_idx2 = MAX(old_idx, new_idx); => 1
+        *    mid_num = mid_idx2 - mid_idx1; => 0
+        *
+        *    if (new >= old) => new; => yes
+        *
+        *    high_idx1 = MAX(old_idx+1, new_idx); => 1
+        *    high_idx2 = num_share_entries; => 1
+        *    high_num = high_idx2 - high_idx1 = 0
+        */
+       low_idx1 = 0;
+       low_idx2 = MIN(old_idx, new_idx);
+       low_num = low_idx2 - low_idx1;
+       mid_idx1 = MIN(old_idx+1, new_idx);
+       mid_idx2 = MAX(old_idx, new_idx);
+       mid_num = mid_idx2 - mid_idx1;
+       high_idx1 = MAX(old_idx+1, new_idx);
+       high_idx2 = ltdb->num_share_entries;
+       high_num = high_idx2 - high_idx1;
+
+       if (low_num != 0) {
+               dbufs[num_dbufs] = (TDB_DATA) {
+                       .dptr = discard_const_p(uint8_t, ltdb->share_entries) +
+                               low_idx1 * SHARE_MODE_ENTRY_SIZE,
+                       .dsize = low_num * SHARE_MODE_ENTRY_SIZE,
+               };
+               num_dbufs += 1;
+       }
+
+       if (new_idx < old_idx) {
+               dbufs[num_dbufs] = (TDB_DATA) {
+                       .dptr = e_buf.buf, .dsize = SHARE_MODE_ENTRY_SIZE,
+               };
+               num_dbufs += 1;
+       }
+
+       if (mid_num != 0) {
+               dbufs[num_dbufs] = (TDB_DATA) {
+                       .dptr = discard_const_p(uint8_t, ltdb->share_entries) +
+                               mid_idx1 * SHARE_MODE_ENTRY_SIZE,
+                       .dsize = mid_num * SHARE_MODE_ENTRY_SIZE,
+               };
+               num_dbufs += 1;
+       }
+
+       if (new_idx >= old_idx) {
+               dbufs[num_dbufs] = (TDB_DATA) {
+                       .dptr = e_buf.buf, .dsize = SHARE_MODE_ENTRY_SIZE,
+               };
+               num_dbufs += 1;
+       }
+
+       if (high_num != 0) {
+               dbufs[num_dbufs] = (TDB_DATA) {
+                       .dptr = discard_const_p(uint8_t, ltdb->share_entries) +
+                               high_idx1 * SHARE_MODE_ENTRY_SIZE,
+                       .dsize = high_num * SHARE_MODE_ENTRY_SIZE,
+               };
+               num_dbufs += 1;
+       }
 
+       {
+               size_t i;
+               for (i=0; i<num_dbufs; i++) {
+                       DBG_DEBUG("dbufs[%zu]=(%p, %zu)\n",
+                                 i,
+                                 dbufs[i].dptr,
+                                 dbufs[i].dsize);
+               }
+       }
+
+       /*
+        * We completely rewrite the entries...
+        */
+       ltdb->share_entries = NULL;
+       ltdb->num_share_entries = 0;
        d->modified = true;
 
-       status = share_mode_data_ltdb_store(d, key, ltdb, NULL, 0);
+       status = share_mode_data_ltdb_store(d, key, ltdb, dbufs, num_dbufs);
        if (!NT_STATUS_IS_OK(status)) {
                DBG_ERR("share_mode_data_ltdb_store failed: %s\n",
                        nt_errstr(status));