lkml.org 
[lkml]   [2014]   [Jan]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC PATCH V2] fs null_blk: Null pointer deference problem in alloc_page_buffers
On 01/20/2014 10:02 PM, Matias Bjorling wrote:
> On 01/20/2014 04:58 AM, Raghavendra K T wrote:
>> If we load the null_blk module with bs=8k we get following oops:
>> [ 3819.812190] BUG: unable to handle kernel NULL pointer dereference at 0000000000000008
>> [ 3819.812387] IP: [<ffffffff81170aa5>] create_empty_buffers+0x28/0xaf
>> [ 3819.812527] PGD 219244067 PUD 215a06067 PMD 0
>> [ 3819.812640] Oops: 0000 [#1] SMP
>> [ 3819.812772] Modules linked in: null_blk(+)
>>
>> Fix that by resetting block size to PAGE_SIZE if it is greater than PAGE_SIZE
>> Also add sanity checks for block size > PAGE_SIZE.
>
> We should probably split the patch into two. Giving a better description
> to each of the changes.
>

Agreed.

[...]
>> diff --git a/drivers/block/null_blk.c b/drivers/block/null_blk.c
>> index a2e69d2..bcae726 100644
>> --- a/drivers/block/null_blk.c
>> +++ b/drivers/block/null_blk.c
>> @@ -622,6 +622,10 @@ static int __init null_init(void)
>> irqmode = NULL_IRQ_NONE;
>> }
>> #endif
>> + if (bs > PAGE_SIZE) {
>> + pr_warn("Invalid block size. Setting it to %lu\n", PAGE_SIZE);
>> + bs = PAGE_SIZE;
>> + }
>
> Could the warning say something like:
>
> pr_warn("null_blk: invalid block size\n");
> pr_warn("null_blk: defaults block size to &lu\n");
>
> then it follows the same patterns as the other errors.

Agree.

>
>>
>> if (queue_mode == NULL_Q_MQ && use_per_node_hctx) {
>> if (submit_queues < nr_online_nodes) {
>> diff --git a/fs/block_dev.c b/fs/block_dev.c
>> index 1e86823..2481d42 100644
>> --- a/fs/block_dev.c
>> +++ b/fs/block_dev.c
>> @@ -1027,6 +1027,7 @@ void bd_set_size(struct block_device *bdev, loff_t size)
>> break;
>> bsize <<= 1;
>> }
>> + BUG_ON(bsize > PAGE_SIZE);
>> bdev->bd_block_size = bsize;
>> bdev->bd_inode->i_blkbits = blksize_bits(bsize);
>> }
>> diff --git a/fs/buffer.c b/fs/buffer.c
>> index 6024877..8b7ada1 100644
>> --- a/fs/buffer.c
>> +++ b/fs/buffer.c
>> @@ -1571,6 +1571,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;
>>
>
> For the check patch, the description could mention some text on why we
> hit this error case and why we check for it.
>

Thanks for the suggestions. Will post V3 with the changes.



\
 
 \ /
  Last update: 2014-01-21 08:21    [W:0.034 / U:0.784 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site