* [PATCH 0/1] Fix NULL pointer dereference in register_disk
@ 2019-02-20 13:27 zhengbin
2019-02-20 13:27 ` [PATCH 1/1] " zhengbin
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: zhengbin @ 2019-02-20 13:27 UTC (permalink / raw)
To: axboe, linux-block; +Cc: houtao1, yi.zhang
When I use syzkaller test kernel, there is a NULL pointer dereference
in register_disk. The better solution is add return code to
__device_add_disk. There is a patchset(http://lists.infradead.org/pipermail
/linux-nvme/2016-August/005860.html) from Fam Zheng, int this patchset, the
modify is as follows:
int device_add_disk() {
...alloc A...
retval = alloc B
if (retval)
goto fail;
...
fail:
return retval; --->did not free A
}
There are many callers, ie:
loop_add-->add_disk-->device_add_disk-->__device_add_disk
loop_remove-->del_gendisk
----->This will free all resources, inclue B(free fail)
Maybe the better way is that if device_add_disk return fail, it should
free all resources? This needs to modify all the callers(Otherwise the
callers will double free), unfortunately, I am not very familiar with it.
zhengbin (1):
Fix NULL pointer dereference in register_disk
block/genhd.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH 1/1] Fix NULL pointer dereference in register_disk
2019-02-20 13:27 [PATCH 0/1] Fix NULL pointer dereference in register_disk zhengbin
@ 2019-02-20 13:27 ` zhengbin
2019-02-25 1:23 ` [PATCH 0/1] " zhengbin (A)
2019-02-28 21:02 ` Jens Axboe
2 siblings, 0 replies; 5+ messages in thread
From: zhengbin @ 2019-02-20 13:27 UTC (permalink / raw)
To: axboe, linux-block; +Cc: houtao1, yi.zhang
If __device_add_disk-->bdi_register_owner-->bdi_register-->
bdi_register_va-->device_create_vargs fails, bdi->dev is still
NULL, __device_add_disk-->register_disk will visit bdi->dev->kobj.
This patch fixes that.
Signed-off-by: zhengbin <zhengbin13@huawei.com>
---
block/genhd.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/block/genhd.c b/block/genhd.c
index 1dd8fd6..78b82d2 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -655,10 +655,12 @@ static void register_disk(struct device *parent, struct gendisk *disk,
kobject_uevent(&part_to_dev(part)->kobj, KOBJ_ADD);
disk_part_iter_exit(&piter);
- err = sysfs_create_link(&ddev->kobj,
- &disk->queue->backing_dev_info->dev->kobj,
- "bdi");
- WARN_ON(err);
+ if (disk->queue->backing_dev_info->dev) {
+ err = sysfs_create_link(&ddev->kobj,
+ &disk->queue->backing_dev_info->dev->kobj,
+ "bdi");
+ WARN_ON(err);
+ }
}
/**
--
2.7.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH 0/1] Fix NULL pointer dereference in register_disk
2019-02-20 13:27 [PATCH 0/1] Fix NULL pointer dereference in register_disk zhengbin
2019-02-20 13:27 ` [PATCH 1/1] " zhengbin
@ 2019-02-25 1:23 ` zhengbin (A)
2019-02-28 21:02 ` Jens Axboe
2 siblings, 0 replies; 5+ messages in thread
From: zhengbin (A) @ 2019-02-25 1:23 UTC (permalink / raw)
To: axboe, linux-block; +Cc: houtao1, yi.zhang, huawei.libin
ping
On 2019/2/20 21:27, zhengbin wrote:
> When I use syzkaller test kernel, there is a NULL pointer dereference
> in register_disk. The better solution is add return code to
> __device_add_disk. There is a patchset(http://lists.infradead.org/pipermail
> /linux-nvme/2016-August/005860.html) from Fam Zheng, int this patchset, the
> modify is as follows:
> int device_add_disk() {
> ...alloc A...
> retval = alloc B
> if (retval)
> goto fail;
> ...
> fail:
> return retval; --->did not free A
> }
> There are many callers, ie:
> loop_add-->add_disk-->device_add_disk-->__device_add_disk
> loop_remove-->del_gendisk
> ----->This will free all resources, inclue B(free fail)
>
> Maybe the better way is that if device_add_disk return fail, it should
> free all resources? This needs to modify all the callers(Otherwise the
> callers will double free), unfortunately, I am not very familiar with it.
>
> zhengbin (1):
> Fix NULL pointer dereference in register_disk
>
> block/genhd.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> --
> 2.7.4
>
>
> .
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH 0/1] Fix NULL pointer dereference in register_disk
2019-02-20 13:27 [PATCH 0/1] Fix NULL pointer dereference in register_disk zhengbin
2019-02-20 13:27 ` [PATCH 1/1] " zhengbin
2019-02-25 1:23 ` [PATCH 0/1] " zhengbin (A)
@ 2019-02-28 21:02 ` Jens Axboe
2019-03-01 1:27 ` zhengbin (A)
2 siblings, 1 reply; 5+ messages in thread
From: Jens Axboe @ 2019-02-28 21:02 UTC (permalink / raw)
To: zhengbin, linux-block; +Cc: houtao1, yi.zhang
On 2/20/19 6:27 AM, zhengbin wrote:
> When I use syzkaller test kernel, there is a NULL pointer dereference
> in register_disk. The better solution is add return code to
> __device_add_disk. There is a patchset(http://lists.infradead.org/pipermail
> /linux-nvme/2016-August/005860.html) from Fam Zheng, int this patchset, the
> modify is as follows:
> int device_add_disk() {
> ...alloc A...
> retval = alloc B
> if (retval)
> goto fail;
> ...
> fail:
> return retval; --->did not free A
> }
> There are many callers, ie:
> loop_add-->add_disk-->device_add_disk-->__device_add_disk
> loop_remove-->del_gendisk
> ----->This will free all resources, inclue B(free fail)
>
> Maybe the better way is that if device_add_disk return fail, it should
> free all resources? This needs to modify all the callers(Otherwise the
> callers will double free), unfortunately, I am not very familiar with it.
That can/should be done when we actually deal with error handling properly
in this path. I have applied your patch.
--
Jens Axboe
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH 0/1] Fix NULL pointer dereference in register_disk
2019-02-28 21:02 ` Jens Axboe
@ 2019-03-01 1:27 ` zhengbin (A)
0 siblings, 0 replies; 5+ messages in thread
From: zhengbin (A) @ 2019-03-01 1:27 UTC (permalink / raw)
To: Jens Axboe, linux-block; +Cc: houtao1, yi.zhang, Jason Yan
Thanks
> That can/should be done when we actually deal with error handling properly
> in this path. I have applied your patch.
-->So when we will actually deal with error handling properly?
On 2019/3/1 5:02, Jens Axboe wrote:
> On 2/20/19 6:27 AM, zhengbin wrote:
>> When I use syzkaller test kernel, there is a NULL pointer dereference
>> in register_disk. The better solution is add return code to
>> __device_add_disk. There is a patchset(http://lists.infradead.org/pipermail
>> /linux-nvme/2016-August/005860.html) from Fam Zheng, int this patchset, the
>> modify is as follows:
>> int device_add_disk() {
>> ...alloc A...
>> retval = alloc B
>> if (retval)
>> goto fail;
>> ...
>> fail:
>> return retval; --->did not free A
>> }
>> There are many callers, ie:
>> loop_add-->add_disk-->device_add_disk-->__device_add_disk
>> loop_remove-->del_gendisk
>> ----->This will free all resources, inclue B(free fail)
>>
>> Maybe the better way is that if device_add_disk return fail, it should
>> free all resources? This needs to modify all the callers(Otherwise the
>> callers will double free), unfortunately, I am not very familiar with it.
>
> That can/should be done when we actually deal with error handling properly
> in this path. I have applied your patch.
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-03-01 1:27 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-20 13:27 [PATCH 0/1] Fix NULL pointer dereference in register_disk zhengbin
2019-02-20 13:27 ` [PATCH 1/1] " zhengbin
2019-02-25 1:23 ` [PATCH 0/1] " zhengbin (A)
2019-02-28 21:02 ` Jens Axboe
2019-03-01 1:27 ` zhengbin (A)
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.