Bug 10685 - Byte range locking - the checking for overlapping pending locks code has an off-by-one error
Summary: Byte range locking - the checking for overlapping pending locks code has an o...
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: File services (show other bugs)
Version: unspecified
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-07-01 20:25 UTC by Jeremy Allison
Modified: 2014-07-17 18:40 UTC (History)
1 user (show)

See Also:


Attachments
git-am fix for master. (1.34 KB, patch)
2014-07-01 22:39 UTC, Jeremy Allison
vl: review+
obnox: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jeremy Allison 2014-07-01 20:25:48 UTC
In source3/locking/brlock.c we have:

/****************************************************************************
 Check if an unlock overlaps a pending lock.
****************************************************************************/

static bool brl_pending_overlap(const struct lock_struct *lock, const struct lock_struct *pend_lock)
{
        if ((lock->start <= pend_lock->start) && (lock->start + lock->size > pend_lock->start))
                return True;
        if ((lock->start >= pend_lock->start) && (lock->start <= pend_lock->start + pend_lock->size))
                return True;
        return False;
}

Consider:

lock = start=110,size=10
pend_lock = 100, size=10

Should not overlap. However,

(lock->start <= pend_lock->start + pend_lock->size)
     110             100                10

is true, so it returns true (overlap).

lock->start <= pend_lock->start + pend_lock->size

should be:

lock->start < pend_lock->start + pend_lock->size

instead. Patch to follow. This wasn't noticed before as it only causes spurious wakeups which do no harm. Found when developing the torture test for
bug  10684 - SMB1 blocking locks can fail notification on unlock, causing client timeout.
Comment 1 Jeremy Allison 2014-07-01 22:39:03 UTC
Created attachment 10062 [details]
git-am fix for master.

Applies cleanly to 4.0.next and 4.1.next also.

Jeremy.
Comment 2 Michael Adam 2014-07-02 06:03:17 UTC
Comment on attachment 10062 [details]
git-am fix for master.

pushed to autobuild
Comment 3 Jeremy Allison 2014-07-02 16:07:19 UTC
Re-assigning to Karolin for inclusion in 4.1.next, 4.0.next. Patch applies cleanly to both branches.

Jeremy.
Comment 4 Karolin Seeger 2014-07-11 07:26:31 UTC
(In reply to comment #3)
> Re-assigning to Karolin for inclusion in 4.1.next, 4.0.next. Patch applies
> cleanly to both branches.
> 
> Jeremy.

Pushed to autobuild-v4-[0|1]-test.
Comment 5 Karolin Seeger 2014-07-17 18:40:06 UTC
(In reply to comment #4)
> (In reply to comment #3)
> > Re-assigning to Karolin for inclusion in 4.1.next, 4.0.next. Patch applies
> > cleanly to both branches.
> > 
> > Jeremy.
> 
> Pushed to autobuild-v4-[0|1]-test.

Pushed to both branches.
Closing out bug report.

Thanks!