From mboxrd@z Thu Jan 1 00:00:00 1970 From: Geert Uytterhoeven Subject: Re: [PATCH v2 2/2] KVM: s390: Use memdup_user() rather than duplicating code Date: Mon, 3 Oct 2016 16:00:03 +0200 Message-ID: References: <82b84c9c-38a4-4d17-910f-312668dbae01@users.sourceforge.net> <73d5a586-2178-a311-f19c-c16c6e8cbb22@users.sourceforge.net> <9ee60162-110b-1305-5a97-624de425d072@users.sourceforge.net> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Return-path: In-Reply-To: <9ee60162-110b-1305-5a97-624de425d072@users.sourceforge.net> Sender: kvm-owner@vger.kernel.org List-Archive: List-Post: To: SF Markus Elfring Cc: kvm@vger.kernel.org, linux-s390 , =?UTF-8?Q?Christian_Borntr=C3=A4ger?= , Cornelia Huck , David Hildenbrand , Heiko Carstens , Martin Schwidefsky , Paolo Bonzini , =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= , LKML , "kernel-janitors@vger.kernel.org" , Julia Lawall , Walter Harms List-ID: Hi Markus, On Mon, Oct 3, 2016 at 3:47 PM, SF Markus Elfring 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@linux-m68k.org 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