Re: [PATCH 7/7] Guard bvec iteration logic v2

[Ming Lei]

On Tue, Apr 4, 2017 at 11:19 PM, Dmitry Monakhov <dmonakhov@xxxxxxxxxx> wrote:
> Ming Lei <tom.leiming@xxxxxxxxx> writes:
>
>> On Mon, Apr 3, 2017 at 3:23 PM, Dmitry Monakhov <dmonakhov@xxxxxxxxxx> wrote:
>>> Currently if some one try to advance bvec beyond it's size we simply
>>> dump WARN_ONCE and continue to iterate beyond bvec array boundaries.
>>> This simply means that we endup dereferencing/corrupting random memory
>>> region.
>>>
>>> Sane reaction would be to propagate error back to calling context
>>> But bvec_iter_advance's calling context is not always good for error
>>> handling. For safity reason let truncate iterator size to zero which
>>
>> IMO, we can avoid continuing to iterate by checking the return value,
>> and looks it is rude to just set iterator size as 0.
> But situation itself is horrible already. IMHO this is BUG_ON situation,

Not sure it is a real BUG_ON() since corrupt bvec array shouldn't happen
because we usually don't modify bvec array directly and just copy to local
variable for further access, but dereferencing random memory might happen.

> but since Linus hate bugons, I try to replace with something loud, but
> very safe. Since there is no guarantee that caller will ignore an error
> and try to dereference bvec the only safe thing we can to is to clamp
> iterator to zero to prevent any possible usage in future.

Since you prevent the case from happening, no dereference invalid bvec
can happen any more, but may cause dead loop. Setting iterator size as
zero can break the dead loop, but the driver still may not know this error
and continue to do its following work.

Don't have a better idea now, and looks it is fine to set iter.bi_size as
zero at the beginning of guarding bvec iteration.



Thanks,
Ming Lei