Re: [PATCH] blk-map: add kernel address validation in blk_rq_map_kern func

[renxudong <renxudong1@huawei.com>]

When we issued scsi cmd, oops occurred. The call stack was as follows.
Call trace:
  __memcpy+0x110/0x180
  bio_endio+0x118/0x190
  blk_update_request+0x94/0x378
  scsi_end_request+0x48/0x2a8
  scsi_io_completion+0xa4/0x6d0
  scsi_finish_command+0xd4/0x138
  scsi_softirq_done+0x13c/0x198
  blk_done_softirq+0xc4/0x108
  __do_softirq+0x120/0x324
  run_ksoftirqd+0x44/0x60
  smpboot_thread_fn+0x1ac/0x1e8
  kthread+0x134/0x138
  ret_from_fork+0x10/0x18
  Since oops is in the process of scsi cmd done, we have not added oops 
info to the commit log.

On 2020/1/7 12:05, Bob Liu wrote:
> On 1/7/20 10:38 AM, Zhiqiang Liu wrote:
>> Friendly ping...
>>
>> On 2019/12/30 20:17, Zhiqiang Liu wrote:
>>> From: renxudong <renxudong1@huawei.com>
>>>
>>> Blk_rq_map_kern func is used to map kernel data to a request,
>>> in which kbuf par should be a valid kernel buffer. However,
>>> kbuf par is only checked whether it is null in blk_rq_map_kern func.
>>>
>>> If users pass a non kernel address to blk_rq_map_kern func in the
>>> non-aligned scenario, the invalid kbuf will be set to bio->bi_private.
>>> When the request is completed, bio_copy_kern_endio_read will be called
>>> to copy data to the kernel address in bio->bi_private. If the bi_private
>>> is not a valid kernel address, the system will oops. In this case, we
> 
> This patch looks fine to me, but curious did you trigger the real oops?
> If yes, it's better add the oops info into commit log.
> 
>>> cannot judge whether the bio structure is damaged or the kernel address is
>>> invalid.
>>>
>>> Here, we add kernel address validation by calling virt_addr_valid.
>>>
>>> Signed-off-by: renxudong <renxudong1@huawei.com>
>>> Reviewed-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
>>> ---
>>>   block/blk-map.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/block/blk-map.c b/block/blk-map.c
>>> index 3a62e471d81b..7deb1b44d1e3 100644
>>> --- a/block/blk-map.c
>>> +++ b/block/blk-map.c
>>> @@ -229,7 +229,7 @@ int blk_rq_map_kern(struct request_queue *q, struct request *rq, void *kbuf,
>>>
>>>   	if (len > (queue_max_hw_sectors(q) << 9))
>>>   		return -EINVAL;
>>> -	if (!len || !kbuf)
>>> +	if (!len || !virt_addr_valid(kbuf))
>>>   		return -EINVAL;
>>>
>>>   	do_copy = !blk_rq_aligned(q, addr, len) || object_is_on_stack(kbuf);
>>>
>>
> 
> 
> .
>