linux-kernel - Re: [RFC PATCH V2] fs null_blk: Null pointer deference problem in alloc_page

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <52DE1578.4010905@linux.vnet.ibm.com>
Date:	Tue, 21 Jan 2014 12:06:40 +0530
From:	Raghavendra K T <raghavendra.kt@...ux.vnet.ibm.com>
To:	Matias Bjorling <m@...rling.me>
CC:	Alexander Viro <viro@...iv.linux.org.uk>,
	Jens Axboe <axboe@...nel.dk>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Yuanhan Liu <yuanhan.liu@...ux.intel.com>,
	"Darrick J. Wong" <darrick.wong@...cle.com>,
	Jan Kara <jack@...e.cz>, Johannes Weiner <hannes@...xchg.org>,
	Zhang Yanfei <zhangyanfei@...fujitsu.com>,
	Jeff <jmoyer@...hat.com>, linux-kernel@...r.kernel.org,
	linux-fsdevel@...r.kernel.org,
	Sumanth <sumantk2@...ux.vnet.ibm.com>
Subject: Re: [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.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/