smbd: Fix flawed share_mode_stale_pid API

The comment for this routine said:

> Modifies d->num_share_modes, watch out in routines iterating over
> that array.

Well, it turns out that *every* caller of this API got it wrong. So I
think it's better to change the routine.

This leaves the array untouched while iterating but filters out the
deleted ones while saving them back to disk.

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Michael Adam <obnox@samba.org>
(cherry picked from commit 7d91ffc6fdc3b371564e14f09822a96264ea372a)

diff --git a/source3/librpc/idl/open_files.idl b/source3/librpc/idl/open_files.idl
index a2e386f68c4262605276df116ab76aa53903b9e0..686bc02548a7e6811188c723a4d815723f57ef1f 100644 (file)
--- a/source3/librpc/idl/open_files.idl
+++ b/source3/librpc/idl/open_files.idl
@@ -23,6 +23,12 @@ interface open_files
                uint32          uid;
                uint16          flags;
                uint32          name_hash;
+
+               /*
+                * In-memory flag indicating a non-existing pid. We don't want
+                * to store this share_mode_entry on disk.
+                */
+               [skip] boolean8 stale;
        } share_mode_entry;
 
        typedef [public] struct {

diff --git a/source3/locking/locking.c b/source3/locking/locking.c
index c0092109e8d6b9b9d5df9799bbea16bb97fdf19b..be2c92dfbd1ddb711bfc5d26e1f4916f041e499d 100644 (file)
--- a/source3/locking/locking.c
+++ b/source3/locking/locking.c
@@ -617,6 +617,10 @@ bool is_valid_share_mode_entry(const struct share_mode_entry *e)
 {
        int num_props = 0;
 
+       if (e->stale) {
+               return false;
+       }
+
        num_props += ((e->op_type == NO_OPLOCK) ? 1 : 0);
        num_props += (EXCLUSIVE_OPLOCK_TYPE(e->op_type) ? 1 : 0);
        num_props += (LEVEL_II_OPLOCK_TYPE(e->op_type) ? 1 : 0);
@@ -635,9 +639,7 @@ bool is_deferred_open_entry(const struct share_mode_entry *e)
 /*
  * In case d->share_modes[i] conflicts with something or otherwise is
  * being used, we need to make sure the corresponding process still
- * exists. This routine checks it and potentially removes the entry
- * from d->share_modes. Modifies d->num_share_modes, watch out in
- * routines iterating over that array.
+ * exists.
  */
 bool share_mode_stale_pid(struct share_mode_data *d, unsigned idx)
 {
@@ -658,17 +660,32 @@ bool share_mode_stale_pid(struct share_mode_data *d, unsigned idx)
        DEBUG(10, ("PID %s (index %u out of %u) does not exist anymore\n",
                   procid_str_static(&e->pid), idx,
                   (unsigned)d->num_share_modes));
-       *e = d->share_modes[d->num_share_modes-1];
-       d->num_share_modes -= 1;
 
-       if (d->num_share_modes == 0 &&
-           d->num_delete_tokens) {
+       e->stale = true;
+
+       if (d->num_delete_tokens != 0) {
+               uint32_t i, num_stale;
+
                /*
                 * We cannot have any delete tokens
                 * if there are no valid share modes.
                 */
-               TALLOC_FREE(d->delete_tokens);
-               d->num_delete_tokens = 0;
+
+               num_stale = 0;
+
+               for (i=0; i<d->num_share_modes; i++) {
+                       if (d->share_modes[i].stale) {
+                               num_stale += 1;
+                       }
+               }
+
+               if (num_stale == d->num_share_modes) {
+                       /*
+                        * No non-stale share mode found
+                        */
+                       TALLOC_FREE(d->delete_tokens);
+                       d->num_delete_tokens = 0;
+               }
        }
 
        d->modified = true;

diff --git a/source3/locking/share_mode_lock.c b/source3/locking/share_mode_lock.c
index 266be65a44587697e7ee57fb83de9e70abc3f67e..6782f592ff87259778a24915f87f862ac8675c4d 100644 (file)
--- a/source3/locking/share_mode_lock.c
+++ b/source3/locking/share_mode_lock.c
@@ -118,6 +118,7 @@ static struct share_mode_data *parse_share_modes(TALLOC_CTX *mem_ctx,
 {
        struct share_mode_data *d;
        enum ndr_err_code ndr_err;
+       uint32_t i;
        DATA_BLOB blob;
 
        d = talloc(mem_ctx, struct share_mode_data);
@@ -137,6 +138,14 @@ static struct share_mode_data *parse_share_modes(TALLOC_CTX *mem_ctx,
                goto fail;
        }
 
+       /*
+        * Initialize the values that are [skip] in the idl. The NDR code does
+        * not initialize them.
+        */
+
+       for (i=0; i<d->num_share_modes; i++) {
+               d->share_modes[i].stale = false;
+       }
        d->modified = false;
        d->fresh = false;
 
@@ -159,12 +168,27 @@ static TDB_DATA unparse_share_modes(struct share_mode_data *d)
 {
        DATA_BLOB blob;
        enum ndr_err_code ndr_err;
+       uint32_t i;
 
        if (DEBUGLEVEL >= 10) {
                DEBUG(10, ("unparse_share_modes:\n"));
                NDR_PRINT_DEBUG(share_mode_data, d);
        }
 
+       i = 0;
+       while (i < d->num_share_modes) {
+               if (d->share_modes[i].stale) {
+                       /*
+                        * Remove the stale entries before storing
+                        */
+                       struct share_mode_entry *m = d->share_modes;
+                       m[i] = m[d->num_share_modes-1];
+                       d->num_share_modes -= 1;
+               } else {
+                       i += 1;
+               }
+       }
+
        if (d->num_share_modes == 0) {
                DEBUG(10, ("No used share mode found\n"));
                return make_tdb_data(NULL, 0);