linux-kernel - Re: [RFC PATCH] 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: <52D97B12.8090208@linux.vnet.ibm.com>
Date:	Sat, 18 Jan 2014 00:18:50 +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>,
	linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
	Sumanth <sumantk2@...ux.vnet.ibm.com>
Subject: Re: [RFC PATCH]  fs null_blk: Null pointer deference problem in alloc_page_buffers

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.


--
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/