Corruption in "b_assoc_buffer" list of bufferhead structure.

Corruption in "b_assoc_buffer" list of bufferhead structure.

[srinivasa]

Hi
  I have got a oops  in which "b_assoc_buffer" list of bufferhead is 
getting corrupted with strange values. It looks like a race problem 
,which is not reproducable at everytime. 
When I looked in to the code,I found that  "b_assoc_buffer" list is 
protected by a spinlock on "private_lock" of struct address_space. But 
there is one situation,where I suspect the chance of corruption. that is 
in try_to_free_buffers() of fs/buffer.c
When mapping becomes NULL, there is no lock protection and if 2 or more 
processors passes this condition and executes drop_buffers() 
simultaneously, there may be a chance of list corruption.

So could somebody please explain whether this situation exists or not?
======================================================================

int try_to_free_buffers(struct page *page)
{
        struct address_space * const mapping = page->mapping;
        struct buffer_head *buffers_to_free = NULL;
        int ret = 0;

        BUG_ON(!PageLocked(page));
        if (PageWriteback(page))
                return 0;

        if (mapping == NULL) {      /* can this still happen? */ <<<<here is my doubt>>>>>>
                ret = drop_buffers(page, &buffers_to_free);
                goto out;
        }

        spin_lock(&mapping->private_lock);
        ret = drop_buffers(page, &buffers_to_free);
        if (ret) {
                /*
                 * If the filesystem writes its buffers by hand (eg ext3)
                 * then we can have clean buffers against a dirty page.  We
                 * clean the page here; otherwise later reattachment of
buffers
                 * could encounter a non-uptodate page, which is
unresolvable.
                 * This only applies in the rare case where
try_to_free_buffers
                 * succeeds but the page is not freed.
                 */
                clear_page_dirty(page);
        }
        spin_unlock(&mapping->private_lock);
=========================================================================================


Thanks
Srinivasa DS

Re: Corruption in "b_assoc_buffer" list of bufferhead structure.

[Mingming Cao]

On Tue, 2006-05-09 at 11:41 +0530, srinivasa wrote:
> Hi
>   I have got a oops  in which "b_assoc_buffer" list of bufferhead is 
> getting corrupted with strange values. It looks like a race problem 
> ,which is not reproducable at everytime. 
> When I looked in to the code,I found that  "b_assoc_buffer" list is 
> protected by a spinlock on "private_lock" of struct address_space. But 
> there is one situation,where I suspect the chance of corruption. that is 
> in try_to_free_buffers() of fs/buffer.c
> When mapping becomes NULL, there is no lock protection and if 2 or more 
> processors passes this condition and executes drop_buffers() 
> simultaneously, there may be a chance of list corruption.
> 
> So could somebody please explain whether this situation exists or not?

Yes, the situation exists.

Which kernel you are running now? It seems Badari has discovered the
same issue and the patch that fixed the deference already made into
mainline:

http://marc.theaimsgroup.com/?l=linux-kernel&m=111464710927691&w=2


Thanks,
Mingming
> ======================================================================
> 
> int try_to_free_buffers(struct page *page)
> {
>         struct address_space * const mapping = page->mapping;
>         struct buffer_head *buffers_to_free = NULL;
>         int ret = 0;
> 
>         BUG_ON(!PageLocked(page));
>         if (PageWriteback(page))
>                 return 0;
> 
>         if (mapping == NULL) {      /* can this still happen? */ <<<<here is my doubt>>>>>>
>                 ret = drop_buffers(page, &buffers_to_free);
>                 goto out;
>         }
> 
>         spin_lock(&mapping->private_lock);
>         ret = drop_buffers(page, &buffers_to_free);
>         if (ret) {
>                 /*
>                  * If the filesystem writes its buffers by hand (eg ext3)
>                  * then we can have clean buffers against a dirty page.  We
>                  * clean the page here; otherwise later reattachment of
> buffers
>                  * could encounter a non-uptodate page, which is
> unresolvable.
>                  * This only applies in the rare case where
> try_to_free_buffers
>                  * succeeds but the page is not freed.
>                  */
>                 clear_page_dirty(page);
>         }
>         spin_unlock(&mapping->private_lock);
> =========================================================================================
> 
> 
> Thanks
> Srinivasa DS
> 
> 
> 
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Corruption in "b_assoc_buffer" list of bufferhead structure.

[srinivasa]

Mingming Cao wrote:
> On Tue, 2006-05-09 at 11:41 +0530, srinivasa wrote:
>   
>> Hi
>>   I have got a oops  in which "b_assoc_buffer" list of bufferhead is 
>> getting corrupted with strange values. It looks like a race problem 
>> ,which is not reproducable at everytime. 
>> When I looked in to the code,I found that  "b_assoc_buffer" list is 
>> protected by a spinlock on "private_lock" of struct address_space. But 
>> there is one situation,where I suspect the chance of corruption. that is 
>> in try_to_free_buffers() of fs/buffer.c
>> When mapping becomes NULL, there is no lock protection and if 2 or more 
>> processors passes this condition and executes drop_buffers() 
>> simultaneously, there may be a chance of list corruption.
>>
>> So could somebody please explain whether this situation exists or not?
>>     
>
> Yes, the situation exists.
>
> Which kernel you are running now? It seems Badari has discovered the
> same issue and the patch that fixed the deference already made into
> mainline:
>
> http://marc.theaimsgroup.com/?l=linux-kernel&m=111464710927691&w=2
>   
Iam using 2.6.16 kernel which is having the mentioned patch.
Is there any way use a lock to avoid race condition ,when mapping 
becomes  NULL ?

>
> Thanks,
> Mingming
>   
>> ======================================================================
>>
>> int try_to_free_buffers(struct page *page)
>> {
>>         struct address_space * const mapping = page->mapping;
>>         struct buffer_head *buffers_to_free = NULL;
>>         int ret = 0;
>>
>>         BUG_ON(!PageLocked(page));
>>         if (PageWriteback(page))
>>                 return 0;
>>
>>         if (mapping == NULL) {      /* can this still happen? */ <<<<here is my doubt>>>>>>
>>                 ret = drop_buffers(page, &buffers_to_free);
>>                 goto out;
>>         }
>>
>>         spin_lock(&mapping->private_lock);
>>         ret = drop_buffers(page, &buffers_to_free);
>>         if (ret) {
>>                 /*
>>                  * If the filesystem writes its buffers by hand (eg ext3)
>>                  * then we can have clean buffers against a dirty page.  We
>>                  * clean the page here; otherwise later reattachment of
>> buffers
>>                  * could encounter a non-uptodate page, which is
>> unresolvable.
>>                  * This only applies in the rare case where
>> try_to_free_buffers
>>                  * succeeds but the page is not freed.
>>                  */
>>                 clear_page_dirty(page);
>>         }
>>         spin_unlock(&mapping->private_lock);
>> =========================================================================================
>>
>>
>> Thanks
>> Srinivasa DS
>>
>>
>>
>>
>> -
>> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>     
>
>