Fix bug 6776 - Running overlapping Byte Lock test will core dump Samba daemon.
authorJeremy Allison <jra@samba.org>
Mon, 5 Oct 2009 17:27:48 +0000 (10:27 -0700)
committerJeremy Allison <jra@samba.org>
Mon, 5 Oct 2009 17:27:48 +0000 (10:27 -0700)
Re-write core of POSIX locking logic.
Jeremy.

source3/locking/brlock.c

index 05d6c7f95d09fa9eb21f949f3fb0d73691d93e51..18a798182b957048310287bfc275701b9e310e6f 100644 (file)
@@ -390,10 +390,9 @@ NTSTATUS brl_lock_windows_default(struct byte_range_lock *br_lck,
  Cope with POSIX range splits and merges.
 ****************************************************************************/
 
-static unsigned int brlock_posix_split_merge(struct lock_struct *lck_arr,              /* Output array. */
-                                               const struct lock_struct *ex,           /* existing lock. */
-                                               const struct lock_struct *plock,        /* proposed lock. */
-                                               bool *lock_was_added)
+static unsigned int brlock_posix_split_merge(struct lock_struct *lck_arr,      /* Output array. */
+                                               struct lock_struct *ex,         /* existing lock. */
+                                               struct lock_struct *plock)      /* proposed lock. */
 {
        bool lock_types_differ = (ex->lock_type != plock->lock_type);
 
@@ -410,21 +409,23 @@ static unsigned int brlock_posix_split_merge(struct lock_struct *lck_arr,         /* Ou
        /* Did we overlap ? */
 
 /*********************************************
-                                             +---------+
-                                             | ex      |
-                                             +---------+
-                              +-------+
-                              | plock |
-                              +-------+
+                                        +---------+
+                                        | ex      |
+                                        +---------+
+                         +-------+
+                         | plock |
+                         +-------+
 OR....
-             +---------+
-             |  ex     |
-             +---------+
+        +---------+
+        |  ex     |
+        +---------+
 **********************************************/
 
        if ( (ex->start > (plock->start + plock->size)) ||
-                       (plock->start > (ex->start + ex->size))) {
+               (plock->start > (ex->start + ex->size))) {
+
                /* No overlap with this lock - copy existing. */
+
                memcpy(&lck_arr[0], ex, sizeof(struct lock_struct));
                return 1;
        }
@@ -436,26 +437,109 @@ OR....
         +---------------------------+
         |       plock               | -> replace with plock.
         +---------------------------+
+OR
+             +---------------+
+             |       ex      |
+             +---------------+
+        +---------------------------+
+        |       plock               | -> replace with plock.
+        +---------------------------+
+
 **********************************************/
 
        if ( (ex->start >= plock->start) &&
-                       (ex->start + ex->size <= plock->start + plock->size) ) {
-               memcpy(&lck_arr[0], plock, sizeof(struct lock_struct));
-               *lock_was_added = True;
-               return 1;
+               (ex->start + ex->size <= plock->start + plock->size) ) {
+
+               /* Replace - discard existing lock. */
+
+               return 0;
        }
 
 /*********************************************
+Adjacent after.
+                        +-------+
+                        |  ex   |
+                        +-------+
+        +---------------+
+        |   plock       |
+        +---------------+
+
+BECOMES....
+        +---------------+-------+
+        |   plock       | ex    | - different lock types.
+        +---------------+-------+
+OR.... (merge)
+        +-----------------------+
+        |   plock               | - same lock type.
+        +-----------------------+
+**********************************************/
+
+       if (plock->start + plock->size == ex->start) {
+
+               /* If the lock types are the same, we merge, if different, we
+                  add the remainder of the old lock. */
+
+               if (lock_types_differ) {
+                       /* Add existing. */
+                       memcpy(&lck_arr[0], ex, sizeof(struct lock_struct));
+                       return 1;
+               } else {
+                       /* Merge - adjust incoming lock as we may have more
+                        * merging to come. */
+                       plock->size += ex->size;
+                       return 0;
+               }
+       }
+
+/*********************************************
+Adjacent before.
+        +-------+
+        |  ex   |
+        +-------+
+                +---------------+
+                |   plock       |
+                +---------------+
+BECOMES....
+        +-------+---------------+
+        | ex    |   plock       | - different lock types
+        +-------+---------------+
+
+OR.... (merge)
+        +-----------------------+
+        |      plock            | - same lock type.
+        +-----------------------+
+
+**********************************************/
+
+       if (ex->start + ex->size == plock->start) {
+
+               /* If the lock types are the same, we merge, if different, we
+                  add the existing lock. */
+
+               if (lock_types_differ) {
+                       memcpy(&lck_arr[0], ex, sizeof(struct lock_struct));
+                       return 1;
+               } else {
+                       /* Merge - adjust incoming lock as we may have more
+                        * merging to come. */
+                       plock->start = ex->start;
+                       plock->size += ex->size;
+                       return 0;
+               }
+       }
+
+/*********************************************
+Overlap after.
         +-----------------------+
         |          ex           |
         +-----------------------+
         +---------------+
         |   plock       |
         +---------------+
-OR....
-                        +-------+
-                        |  ex   |
-                        +-------+
+OR
+               +----------------+
+               |       ex       |
+               +----------------+
         +---------------+
         |   plock       |
         +---------------+
@@ -466,60 +550,57 @@ BECOMES....
         +---------------+-------+
 OR.... (merge)
         +-----------------------+
-        |   ex                  | - same lock type.
+        |   plock               | - same lock type.
         +-----------------------+
 **********************************************/
 
        if ( (ex->start >= plock->start) &&
-                               (ex->start <= plock->start + plock->size) &&
-                               (ex->start + ex->size > plock->start + plock->size) ) {
-
-               *lock_was_added = True;
+               (ex->start <= plock->start + plock->size) &&
+               (ex->start + ex->size > plock->start + plock->size) ) {
 
                /* If the lock types are the same, we merge, if different, we
-                  add the new lock before the old. */
+                  add the remainder of the old lock. */
 
                if (lock_types_differ) {
-                       /* Add new. */
-                       memcpy(&lck_arr[0], plock, sizeof(struct lock_struct));
-                       memcpy(&lck_arr[1], ex, sizeof(struct lock_struct));
+                       /* Add remaining existing. */
+                       memcpy(&lck_arr[0], ex, sizeof(struct lock_struct));
                        /* Adjust existing start and size. */
-                       lck_arr[1].start = plock->start + plock->size;
-                       lck_arr[1].size = (ex->start + ex->size) - (plock->start + plock->size);
-                       return 2;
-               } else {
-                       /* Merge. */
-                       memcpy(&lck_arr[0], plock, sizeof(struct lock_struct));
-                       /* Set new start and size. */
-                       lck_arr[0].start = plock->start;
-                       lck_arr[0].size = (ex->start + ex->size) - plock->start;
+                       lck_arr[0].start = plock->start + plock->size;
+                       lck_arr[0].size = (ex->start + ex->size) - (plock->start + plock->size);
                        return 1;
+               } else {
+                       /* Merge - adjust incoming lock as we may have more
+                        * merging to come. */
+                       plock->size += (ex->start + ex->size) - (plock->start + plock->size);
+                       return 0;
                }
        }
 
 /*********************************************
-   +-----------------------+
-   |  ex                   |
-   +-----------------------+
-           +---------------+
-           |   plock       |
-           +---------------+
-OR....
-   +-------+        
-   |  ex   |
-   +-------+
-           +---------------+
-           |   plock       |
-           +---------------+
+Overlap before.
+        +-----------------------+
+        |  ex                   |
+        +-----------------------+
+                +---------------+
+                |   plock       |
+                +---------------+
+OR
+        +-------------+
+        |  ex         |
+        +-------------+
+                +---------------+
+                |   plock       |
+                +---------------+
+
 BECOMES....
-   +-------+---------------+
-   | ex    |   plock       | - different lock types
-   +-------+---------------+
+        +-------+---------------+
+        | ex    |   plock       | - different lock types
+        +-------+---------------+
 
 OR.... (merge)
-   +-----------------------+
-   | ex                    | - same lock type.
-   +-----------------------+
+        +-----------------------+
+        |      plock            | - same lock type.
+        +-----------------------+
 
 **********************************************/
 
@@ -527,27 +608,25 @@ OR.... (merge)
                        (ex->start + ex->size >= plock->start) &&
                        (ex->start + ex->size <= plock->start + plock->size) ) {
 
-               *lock_was_added = True;
-
                /* If the lock types are the same, we merge, if different, we
-                  add the new lock after the old. */
+                  add the truncated old lock. */
 
                if (lock_types_differ) {
                        memcpy(&lck_arr[0], ex, sizeof(struct lock_struct));
-                       memcpy(&lck_arr[1], plock, sizeof(struct lock_struct));
                        /* Adjust existing size. */
                        lck_arr[0].size = plock->start - ex->start;
-                       return 2;
-               } else {
-                       /* Merge. */
-                       memcpy(&lck_arr[0], ex, sizeof(struct lock_struct));
-                       /* Adjust existing size. */
-                       lck_arr[0].size = (plock->start + plock->size) - ex->start;
                        return 1;
+               } else {
+                       /* Merge - adjust incoming lock as we may have more
+                        * merging to come. MUST ADJUST plock SIZE FIRST ! */
+                       plock->size += (plock->start - ex->start);
+                       plock->start = ex->start;
+                       return 0;
                }
        }
 
 /*********************************************
+Complete overlap.
         +---------------------------+
         |        ex                 |
         +---------------------------+
@@ -560,32 +639,31 @@ BECOMES.....
         +-------+---------+---------+
 OR
         +---------------------------+
-        |        ex                 | - same lock type.
+        |        plock              | - same lock type.
         +---------------------------+
 **********************************************/
 
        if ( (ex->start < plock->start) && (ex->start + ex->size > plock->start + plock->size) ) {
-               *lock_was_added = True;
 
                if (lock_types_differ) {
 
                        /* We have to split ex into two locks here. */
 
                        memcpy(&lck_arr[0], ex, sizeof(struct lock_struct));
-                       memcpy(&lck_arr[1], plock, sizeof(struct lock_struct));
-                       memcpy(&lck_arr[2], ex, sizeof(struct lock_struct));
+                       memcpy(&lck_arr[1], ex, sizeof(struct lock_struct));
 
                        /* Adjust first existing size. */
                        lck_arr[0].size = plock->start - ex->start;
 
                        /* Adjust second existing start and size. */
-                       lck_arr[2].start = plock->start + plock->size;
-                       lck_arr[2].size = (ex->start + ex->size) - (plock->start + plock->size);
-                       return 3;
+                       lck_arr[1].start = plock->start + plock->size;
+                       lck_arr[1].size = (ex->start + ex->size) - (plock->start + plock->size);
+                       return 2;
                } else {
-                       /* Just eat plock. */
-                       memcpy(&lck_arr[0], ex, sizeof(struct lock_struct));
-                       return 1;
+                       /* Just eat the existing locks, merge them into plock. */
+                       plock->start = ex->start;
+                       plock->size = ex->size;
+                       return 0;
                }
        }
 
@@ -609,7 +687,6 @@ static NTSTATUS brl_lock_posix(struct messaging_context *msg_ctx,
        unsigned int i, count, posix_count;
        struct lock_struct *locks = br_lck->lock_data;
        struct lock_struct *tp;
-       bool lock_was_added = False;
        bool signal_pending_read = False;
        bool break_oplocks = false;
        NTSTATUS status;
@@ -633,8 +710,9 @@ static NTSTATUS brl_lock_posix(struct messaging_context *msg_ctx,
        if (!tp) {
                return NT_STATUS_NO_MEMORY;
        }
-       
+
        count = posix_count = 0;
+
        for (i=0; i < br_lck->num_locks; i++) {
                struct lock_struct *curr_lock = &locks[i];
 
@@ -671,7 +749,7 @@ static NTSTATUS brl_lock_posix(struct messaging_context *msg_ctx,
                        }
 
                        /* Work out overlaps. */
-                       tmp_count += brlock_posix_split_merge(&tp[count], curr_lock, plock, &lock_was_added);
+                       tmp_count += brlock_posix_split_merge(&tp[count], curr_lock, plock);
                        posix_count += tmp_count;
                        count += tmp_count;
                }
@@ -692,10 +770,21 @@ static NTSTATUS brl_lock_posix(struct messaging_context *msg_ctx,
                                             LEVEL2_CONTEND_POSIX_BRL);
        }
 
-       if (!lock_was_added) {
-               memcpy(&tp[count], plock, sizeof(struct lock_struct));
-               count++;
+       /* Try and add the lock in order, sorted by lock start. */
+       for (i=0; i < count; i++) {
+               struct lock_struct *curr_lock = &tp[i];
+
+               if (curr_lock->start <= plock->start) {
+                       continue;
+               }
+       }
+
+       if (i < count) {
+               memmove(&tp[i+1], &tp[i],
+                       (count - i)*sizeof(struct lock_struct));
        }
+       memcpy(&tp[i], plock, sizeof(struct lock_struct));
+       count++;
 
        /* We can get the POSIX lock, now see if it needs to
           be mapped into a lower level POSIX one, and if so can
@@ -729,12 +818,16 @@ static NTSTATUS brl_lock_posix(struct messaging_context *msg_ctx,
                }
        }
 
-       /* Realloc so we don't leak entries per lock call. */
-       tp = (struct lock_struct *)SMB_REALLOC(tp, count * sizeof(*locks));
-       if (!tp) {
-               status = NT_STATUS_NO_MEMORY;
-               goto fail;
+       /* If we didn't use all the allocated size,
+        * Realloc so we don't leak entries per lock call. */
+       if (count < br_lck->num_locks + 2) {
+               tp = (struct lock_struct *)SMB_REALLOC(tp, count * sizeof(*locks));
+               if (!tp) {
+                       status = NT_STATUS_NO_MEMORY;
+                       goto fail;
+               }
        }
+
        br_lck->num_locks = count;
        SAFE_FREE(br_lck->lock_data);
        br_lck->lock_data = tp;
@@ -955,9 +1048,9 @@ bool brl_unlock_windows_default(struct messaging_context *msg_ctx,
 
 static bool brl_unlock_posix(struct messaging_context *msg_ctx,
                             struct byte_range_lock *br_lck,
-                            const struct lock_struct *plock)
+                            struct lock_struct *plock)
 {
-       unsigned int i, j, count, posix_count;
+       unsigned int i, j, count;
        struct lock_struct *tp;
        struct lock_struct *locks = br_lck->lock_data;
        bool overlap_found = False;
@@ -984,11 +1077,9 @@ static bool brl_unlock_posix(struct messaging_context *msg_ctx,
                return False;
        }
 
-       count = posix_count = 0;
+       count = 0;
        for (i = 0; i < br_lck->num_locks; i++) {
                struct lock_struct *lock = &locks[i];
-               struct lock_struct tmp_lock[3];
-               bool lock_was_added = False;
                unsigned int tmp_count;
 
                /* Only remove our own locks - ignore fnum. */
@@ -999,68 +1090,50 @@ static bool brl_unlock_posix(struct messaging_context *msg_ctx,
                        continue;
                }
 
-               /* Work out overlaps. */
-               tmp_count = brlock_posix_split_merge(&tmp_lock[0], &locks[i], plock, &lock_was_added);
-
-               if (tmp_count == 1) {
-                       /* Ether the locks didn't overlap, or the unlock completely
-                          overlapped this lock. If it didn't overlap, then there's
-                          no change in the locks. */
-                       if (tmp_lock[0].lock_type != UNLOCK_LOCK) {
-                               SMB_ASSERT(tmp_lock[0].lock_type == locks[i].lock_type);
-                               /* No change in this lock. */
-                               memcpy(&tp[count], &tmp_lock[0], sizeof(struct lock_struct));
-                               count++;
-                               posix_count++;
-                       } else {
-                               SMB_ASSERT(tmp_lock[0].lock_type == UNLOCK_LOCK);
-                               overlap_found = True;
-                       }
-                       continue;
-               } else if (tmp_count == 2) {
-                       /* The unlock overlapped an existing lock. Copy the truncated
-                          lock into the lock array. */
-                       if (tmp_lock[0].lock_type != UNLOCK_LOCK) {
-                               SMB_ASSERT(tmp_lock[0].lock_type == locks[i].lock_type);
-                               SMB_ASSERT(tmp_lock[1].lock_type == UNLOCK_LOCK);
-                               memcpy(&tp[count], &tmp_lock[0], sizeof(struct lock_struct));
-                               if (tmp_lock[0].size != locks[i].size) {
-                                       overlap_found = True;
-                               }
-                       } else {
-                               SMB_ASSERT(tmp_lock[0].lock_type == UNLOCK_LOCK);
-                               SMB_ASSERT(tmp_lock[1].lock_type == locks[i].lock_type);
-                               memcpy(&tp[count], &tmp_lock[1], sizeof(struct lock_struct));
-                               if (tmp_lock[1].start != locks[i].start) {
-                                       overlap_found = True;
-                               }
+               if (lock->lock_flav == WINDOWS_LOCK) {
+                       /* Do any Windows flavour locks conflict ? */
+                       if (brl_conflict(lock, plock)) {
+                               SAFE_FREE(tp);
+                               return false;
                        }
+                       /* Just copy the Windows lock into the new array. */
+                       memcpy(&tp[count], lock, sizeof(struct lock_struct));
                        count++;
-                       posix_count++;
                        continue;
-               } else {
-                       /* tmp_count == 3 - (we split a lock range in two). */
-                       SMB_ASSERT(tmp_lock[0].lock_type == locks[i].lock_type);
-                       SMB_ASSERT(tmp_lock[1].lock_type == UNLOCK_LOCK);
-                       SMB_ASSERT(tmp_lock[2].lock_type == locks[i].lock_type);
+               }
+
+               /* Work out overlaps. */
+               tmp_count = brlock_posix_split_merge(&tp[count], lock, plock);
+
+               if (tmp_count == 0) {
+                       /* plock overlapped the existing lock completely,
+                          or replaced it. Don't copy the existing lock. */
+                       overlap_found = true;
+               } else if (tmp_count == 1) {
+                       /* Either no overlap, (simple copy of existing lock) or
+                        * an overlap of an existing lock. */
+                       /* If the lock changed size, we had an overlap. */
+                       if (tp[count].size != lock->size) {
+                               overlap_found = true;
+                       }
+                       count += tmp_count;
+               } else if (tmp_count == 2) {
+                       /* We split a lock range in two. */
+                       overlap_found = true;
+                       count += tmp_count;
 
-                       memcpy(&tp[count], &tmp_lock[0], sizeof(struct lock_struct));
-                       count++;
-                       posix_count++;
-                       memcpy(&tp[count], &tmp_lock[2], sizeof(struct lock_struct));
-                       count++;
-                       posix_count++;
-                       overlap_found = True;
                        /* Optimisation... */
                        /* We know we're finished here as we can't overlap any
                           more POSIX locks. Copy the rest of the lock array. */
+
                        if (i < br_lck->num_locks - 1) {
-                               memcpy(&tp[count], &locks[i+1], 
+                               memcpy(&tp[count], &locks[i+1],
                                        sizeof(*locks)*((br_lck->num_locks-1) - i));
                                count += ((br_lck->num_locks-1) - i);
                        }
                        break;
                }
+
        }
 
        if (!overlap_found) {
@@ -1093,10 +1166,8 @@ static bool brl_unlock_posix(struct messaging_context *msg_ctx,
                tp = NULL;
        }
 
-       if (posix_count == 0) {
-               contend_level2_oplocks_end(br_lck->fsp,
-                                          LEVEL2_CONTEND_POSIX_BRL);
-       }
+       contend_level2_oplocks_end(br_lck->fsp,
+                                  LEVEL2_CONTEND_POSIX_BRL);
 
        br_lck->num_locks = count;
        SAFE_FREE(br_lck->lock_data);