From 131d7d0b04d5b843a43746a2dcea6504208a4de3 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Mon, 5 Oct 2009 14:22:05 -0700 Subject: [PATCH] Fix bug 6776 - Running overlapping Byte Lock test will core dump Samba daemon. Re-write core of POSIX locking logic. Jeremy. --- source3/locking/brlock.c | 367 +++++++++++++++++++++++++++------------------- 1 files changed, 219 insertions(+), 148 deletions(-) diff --git a/source3/locking/brlock.c b/source3/locking/brlock.c index be2948c..f22030c 100644 --- a/source3/locking/brlock.c +++ b/source3/locking/brlock.c @@ -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; @@ -944,9 +1037,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; @@ -973,11 +1066,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. */ @@ -988,68 +1079,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) { @@ -1082,10 +1155,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); -- 1.5.4.3