Re: [RFC PATCH] fs null_blk: Null pointer deference problem in alloc_page_buffers

[Raghavendra K T <raghavendra.kt@linux.vnet.ibm.com>]

On 01/17/2014 11:56 PM, Matias Bjorling wrote:
> On 01/17/2014 01:22 AM, Raghavendra K T wrote:
>>
>> diff --git a/drivers/block/null_blk.c b/drivers/block/null_blk.c
>> index a2e69d2..6b0e049 100644
>> --- a/drivers/block/null_blk.c
>> +++ b/drivers/block/null_blk.c
>> @@ -535,6 +535,11 @@ static int null_add_dev(void)
>>       if (!nullb)
>>           return -ENOMEM;
>>
>> +    if (bs > PAGE_SIZE) {
>> +        WARN(1, "Invalid block size. Setting it to 4096\n");
>> +        bs = 4096;
>> +    }
>> +
>
> Use PAGESIZE instead, move it to null_init with the other checks and use
> pr_warn for issuing the warning.

Thanks. will do that change.

>
>>       spin_lock_init(&nullb->lock);
>>
>>       if (queue_mode == NULL_Q_MQ && use_per_node_hctx)
>> diff --git a/fs/buffer.c b/fs/buffer.c
>> index 6024877..029c698 100644
>> --- a/fs/buffer.c
>> +++ b/fs/buffer.c
>> @@ -883,6 +883,7 @@ struct buffer_head *alloc_page_buffers(struct page
>> *page, unsigned long size,
>>       struct buffer_head *bh, *head;
>>       long offset;
>>
>> +    BUG_ON(size > PAGE_SIZE);
>>   try_again:
>>       head = NULL;
>>       offset = PAGE_SIZE;
>> @@ -1571,6 +1572,7 @@ void create_empty_buffers(struct page *page,
>>       struct buffer_head *bh, *head, *tail;
>>
>>       head = alloc_page_buffers(page, blocksize, 1);
>> +    BUG_ON(!head);
>>       bh = head;
>>       do {
>>           bh->b_state |= b_state;
>>
>
> It seems? that the physical sector size is always limited to the system
> page size.
>
> Why not do the check in add_disk (or __blkdev_get) and fail there,
> instead of failing on the first partition check?
>

I believe you meant checking BUG_ON(block size >  PAGE_SIZE) could be 
done earlier in add_disk/__blkdev_get. I 'll check on that.

For null pointer from alloc_page_buffers part (low in memory condition)
I think we need to retain that.