From mboxrd@z Thu Jan  1 00:00:00 1970
From: Raghavendra K T <raghavendra.kt@linux.vnet.ibm.com>
Subject: Re: [RFC PATCH]  fs null_blk: Null pointer deference problem in alloc_page_buffers
Date: Sat, 18 Jan 2014 00:18:50 +0530
Message-ID: <52D97B12.8090208@linux.vnet.ibm.com>
References: <1389950530-8903-1-git-send-email-raghavendra.kt@linux.vnet.ibm.com> <52D975D3.2010009@bjorling.me>
Mime-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Cc: Alexander Viro <viro@zeniv.linux.org.uk>,
	Jens Axboe <axboe@kernel.dk>,
	Andrew Morton <akpm@linux-foundation.org>,
	Yuanhan Liu <yuanhan.liu@linux.intel.com>,
	"Darrick J. Wong" <darrick.wong@oracle.com>,
	Jan Kara <jack@suse.cz>, Johannes Weiner <hannes@cmpxchg.org>,
	Zhang Yanfei <zhangyanfei@cn.fujitsu.com>,
	linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	Sumanth <sumantk2@linux.vnet.ibm.com>
To: Matias Bjorling <m@bjorling.me>
Return-path: <linux-kernel-owner@vger.kernel.org>
In-Reply-To: <52D975D3.2010009@bjorling.me>
Sender: linux-kernel-owner@vger.kernel.org
List-Id: linux-fsdevel.vger.kernel.org

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.