Re: [PATCH v2 2/2] KVM: s390: Use memdup_user() rather than duplicating code

[Geert Uytterhoeven]

Hi Markus,

On Mon, Oct 3, 2016 at 3:47 PM, SF Markus Elfring
<elfring@xxxxxxxxxxxxxxxxxxxxx> wrote:
>>> Did you notice the check "IS_ERR(bp_data)" and the corresponding reaction
>>> in this update suggestion?
>>
>> Yes, but bp_data may still be a valid (as in "not an error") value.
>
> Thanks for your constructive feedback.
>
>> Your commit a1708a2eaded836b ("KVM: s390: Improve determination of sizes in
>> kvm_s390_import_bp_data()") made the code more robust, as kmalloc_array() ha
>>  a builtin overflow check, and will return NULL if overflow is detected.
>> However, commit 0624a8eb82efd58e ("KVM: s390: Use memdup_user() rather than
>> duplicating code") dropped that safety net again.
>
> * How much are you concerned about the shown software evolution around
>   multiplications for memory allocations?

Enough to write my reply ;-)

> * Does there really a probability remain that an inappropriate product
>   would be calculated here (as the situation was before my two update steps
>   for this software module)?

Perhaps not. Hence my "Probably not an issue here, ...".

> * Can it be that you are looking for a variant of a function like "memdup_user"
>   where values can be passed as separate parameters "count" and "size" so that
>   the needed multiplication and corresponding overflow check would be performed
>   together as desired?

If there are enough uses, and people like it, adding memdup_user_array()
may be a good idea...

P.S. Why do your questions make me think of a scientific paper? ;-)

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds