[PATCH] Fix messed hunks in generic_setlease

[PATCH] Fix messed hunks in generic_setlease

[Pavel Emelyanov]

I have noticed, that one hunk was lost and one duplicated 
during merging the fix-potential-oops-in-generic_setlease(-xxx) 
patches. One of the fixes is already in the hot-fixes, but the
second one is still lost.

The returned pointer was not the one allocated, but some temporary
used to scan through the inode's locks list. This caused and OOPS 
during Kamalesh's testing.

Signed-off-by: Pavel Emelyanov <xemul@openvz.org>

---

diff --git a/fs/locks.c b/fs/locks.c
index c0fe71a..c1198e3 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -1423,7 +1418,7 @@ int generic_setlease(struct file *filp, 
 	locks_copy_lock(new_fl, lease);
 	locks_insert_lock(before, new_fl);
 
-	*flp = fl;
+	*flp = new_fl;
 	return 0;
 
 out:

Re: [PATCH] Fix messed hunks in generic_setlease

[Andrew Morton]

On Tue, 25 Sep 2007 11:57:45 +0400 Pavel Emelyanov <xemul@openvz.org> wrote:

> I have noticed, that one hunk was lost and one duplicated 
> during merging the fix-potential-oops-in-generic_setlease(-xxx) 
> patches. One of the fixes is already in the hot-fixes, but the
> second one is still lost.
> 
> The returned pointer was not the one allocated, but some temporary
> used to scan through the inode's locks list. This caused and OOPS 
> during Kamalesh's testing.
> 
> Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
> 
> ---
> 
> diff --git a/fs/locks.c b/fs/locks.c
> index c0fe71a..c1198e3 100644
> --- a/fs/locks.c
> +++ b/fs/locks.c
> @@ -1423,7 +1418,7 @@ int generic_setlease(struct file *filp, 
>  	locks_copy_lock(new_fl, lease);
>  	locks_insert_lock(before, new_fl);
>  
> -	*flp = fl;
> +	*flp = new_fl;
>  	return 0;
>  
>  out:

argh, what a mess - there are way too many trees playing with fs/locks.c.

umm, I think this is not a mismerge and that the original patch
(http://lkml.org/lkml/2007/9/20/141) had this bug in it.

And I've just sent that buggy patch to Linus.  Do you agree?

Re: [PATCH] Fix messed hunks in generic_setlease

[Pavel Emelyanov]

Andrew Morton wrote:
> On Tue, 25 Sep 2007 11:57:45 +0400 Pavel Emelyanov <xemul@openvz.org> wrote:
> 
>> I have noticed, that one hunk was lost and one duplicated 
>> during merging the fix-potential-oops-in-generic_setlease(-xxx) 
>> patches. One of the fixes is already in the hot-fixes, but the
>> second one is still lost.
>>
>> The returned pointer was not the one allocated, but some temporary
>> used to scan through the inode's locks list. This caused and OOPS 
>> during Kamalesh's testing.
>>
>> Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
>>
>> ---
>>
>> diff --git a/fs/locks.c b/fs/locks.c
>> index c0fe71a..c1198e3 100644
>> --- a/fs/locks.c
>> +++ b/fs/locks.c
>> @@ -1423,7 +1418,7 @@ int generic_setlease(struct file *filp, 
>>  	locks_copy_lock(new_fl, lease);
>>  	locks_insert_lock(before, new_fl);
>>  
>> -	*flp = fl;
>> +	*flp = new_fl;
>>  	return 0;
>>  
>>  out:
> 
> argh, what a mess - there are way too many trees playing with fs/locks.c.
> 
> umm, I think this is not a mismerge and that the original patch
> (http://lkml.org/lkml/2007/9/20/141) had this bug in it.

Indeed... :(

> And I've just sent that buggy patch to Linus.  Do you agree?

Shame on me... Sorry :(

(going to the blackboard to write "I will check my patches twice before
 sending them to Andrew" for 100 times)

Re: [PATCH] Fix messed hunks in generic_setlease

[Kamalesh Babulal]

Pavel Emelyanov wrote:
> I have noticed, that one hunk was lost and one duplicated 
> during merging the fix-potential-oops-in-generic_setlease(-xxx) 
> patches. One of the fixes is already in the hot-fixes, but the
> second one is still lost.
> 
> The returned pointer was not the one allocated, but some temporary
> used to scan through the inode's locks list. This caused and OOPS 
> during Kamalesh's testing.
> 
> Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
> 
> ---
> 
> diff --git a/fs/locks.c b/fs/locks.c
> index c0fe71a..c1198e3 100644
> --- a/fs/locks.c
> +++ b/fs/locks.c
> @@ -1423,7 +1418,7 @@ int generic_setlease(struct file *filp, 
>  	locks_copy_lock(new_fl, lease);
>  	locks_insert_lock(before, new_fl);
> 
> -	*flp = fl;
> +	*flp = new_fl;
>  	return 0;
> 
>  out:
> 

Hi Pavel,

I tested your patch and NULL pointer dereference is not triggered.

-- 
Thanks & Regards,
Kamalesh Babulal,
Linux Technology Center,
IBM, ISTL.