On 11/22/2019 03:58 AM, Alexander Potapenko wrote:
> I'm using the following patch in KMSAN tree to suppress these bugs:
>
> diff --git a/drivers/block/null_blk_main.c b/drivers/block/null_blk_main.c
> index 0e7da5015ccd5..9e8e99bdc0db3 100644
> --- a/drivers/block/null_blk_main.c
> +++ b/drivers/block/null_blk_main.c
> @@ -1235,8 +1235,16 @@ static blk_status_t null_handle_cmd(struct
> nullb_cmd *cmd, sector_t sector,
> if (dev->memory_backed)
> cmd->error = null_handle_memory_backed(cmd, op);
>
> - if (!cmd->error && dev->zoned)
> + if (!cmd->error && dev->zoned) {
> cmd->error = null_handle_zoned(cmd, op, sector, nr_sectors);
> + } else if (dev->queue_mode != NULL_Q_BIO) {
> + /*
> + * TODO(glider): this is a hack to suppress bugs in nullb
> + * driver. I have no idea what I'm doing, better wait for a
> + * proper fix from Jens Axboe.
> + */
> + cmd->error = null_handle_rq(cmd);
> + }
This is just based on what I read in the code, I don't have
the kmsan support.
null_handle_rq() should not be called without checking the
dev->memory_backed value since it handles memory backed operation,
(it may be working because of the correct error handling done
in the copy_from_nullb() when t_page == NULL).
Possible explanation could be for the above code fixing the problem
is:-
null_handle_cmd()
None of the ifs are handled (since it is default behavior)
calls null_handle_rq()
Processes each bvec in this request :-
rq_for_each_segment()
calls null_transfer()
calls copy_from_null()
Now here in copy_from_null(), null_lookup_page() returns
NULL since it is not present in the radix tree(dev and cache
since its !membacked && !cache_size I assume since I don't have
the command line of modprobe null_blk),
and then does the dest memset():-
(from linux-block for-next)
1009 if (!t_page) {
1010 memset(dst + off + count, 0, temp);
1011 goto next;
1012 }
which initializes the destination page to 0. This is what Jens
is referring to I guess that we need to memset() it for the
read when !membacked.
It will be great if you can verify the above code path on your
setup with above memset().
The above patch does fix the problem but it is very confusing.
What we need is to traverse the rq bvec and actually initialize
the bvec.page and repeat the pattern present in the copy_from_null()
in such a way this common code is shared between membacked and
non-membacked mode for REQ_OP_READ.
Jens can correct me if I'm wrong.
I'll be happy to look into this if Jens is okay.
>
> It appears to fix the problem, but I'm not really sure this is the
> right thing to do (e.g. whether it covers all invocations of
> null_handle_cmd(), probably not)
> I don't see an easy way to memset() the pages being read, as there're
> no pages at this point.
>
>> >--
