From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753021AbcJCML2 (ORCPT ); Mon, 3 Oct 2016 08:11:28 -0400 Received: from mail-io0-f196.google.com ([209.85.223.196]:34717 "EHLO mail-io0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751229AbcJCMLU (ORCPT ); Mon, 3 Oct 2016 08:11:20 -0400 MIME-Version: 1.0 In-Reply-To: References: <82b84c9c-38a4-4d17-910f-312668dbae01@users.sourceforge.net> From: Geert Uytterhoeven Date: Mon, 3 Oct 2016 14:11:18 +0200 X-Google-Sender-Auth: xZQMs7DDvSDYdpiV_nGPAsTOul4 Message-ID: Subject: Re: [PATCH v2 2/2] KVM: s390: Use memdup_user() rather than duplicating code 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 Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Markus, On Wed, Aug 24, 2016 at 8:40 PM, SF Markus Elfring wrote: > From: Markus Elfring > Date: Wed, 24 Aug 2016 20:10:09 +0200 > > * Reuse existing functionality from memdup_user() instead of keeping > duplicate source code. > > This issue was detected by using the Coccinelle software. > > * Return directly if this copy operation failed. > > Reviewed-by: David Hildenbrand > Signed-off-by: Markus Elfring > --- > > v2: Rebased on source files from "Linux next-20160824". > > arch/s390/kvm/guestdbg.c | 18 ++++-------------- > 1 file changed, 4 insertions(+), 14 deletions(-) > > diff --git a/arch/s390/kvm/guestdbg.c b/arch/s390/kvm/guestdbg.c > index 70b71ac..d7c6a7f 100644 > --- a/arch/s390/kvm/guestdbg.c > +++ b/arch/s390/kvm/guestdbg.c > @@ -216,20 +216,10 @@ int kvm_s390_import_bp_data(struct kvm_vcpu *vcpu, > else if (dbg->arch.nr_hw_bp > MAX_BP_COUNT) > return -EINVAL; > > - bp_data = kmalloc_array(dbg->arch.nr_hw_bp, > - sizeof(*bp_data), > - GFP_KERNEL); Probably not an issue here, but if "sizeof(*bp_data) * dbg->arch.nr_hw_bp" overflows, kmalloc_array() would have returned NULL here... > - if (!bp_data) { > - ret = -ENOMEM; > - goto error; > - } > - > - if (copy_from_user(bp_data, > - dbg->arch.hw_bp, > - sizeof(*bp_data) * dbg->arch.nr_hw_bp)) { > - ret = -EFAULT; > - goto error; > - } > + bp_data = memdup_user(dbg->arch.hw_bp, > + sizeof(*bp_data) * dbg->arch.nr_hw_bp); ... while this would continue silently, and corrupt memory. > + if (IS_ERR(bp_data)) > + return PTR_ERR(bp_data); 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 From mboxrd@z Thu Jan 1 00:00:00 1970 From: Geert Uytterhoeven Date: Mon, 03 Oct 2016 12:11:18 +0000 Subject: Re: [PATCH v2 2/2] KVM: s390: Use memdup_user() rather than duplicating code Message-Id: List-Id: References: <82b84c9c-38a4-4d17-910f-312668dbae01@users.sourceforge.net> In-Reply-To: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit 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 Hi Markus, On Wed, Aug 24, 2016 at 8:40 PM, SF Markus Elfring wrote: > From: Markus Elfring > Date: Wed, 24 Aug 2016 20:10:09 +0200 > > * Reuse existing functionality from memdup_user() instead of keeping > duplicate source code. > > This issue was detected by using the Coccinelle software. > > * Return directly if this copy operation failed. > > Reviewed-by: David Hildenbrand > Signed-off-by: Markus Elfring > --- > > v2: Rebased on source files from "Linux next-20160824". > > arch/s390/kvm/guestdbg.c | 18 ++++-------------- > 1 file changed, 4 insertions(+), 14 deletions(-) > > diff --git a/arch/s390/kvm/guestdbg.c b/arch/s390/kvm/guestdbg.c > index 70b71ac..d7c6a7f 100644 > --- a/arch/s390/kvm/guestdbg.c > +++ b/arch/s390/kvm/guestdbg.c > @@ -216,20 +216,10 @@ int kvm_s390_import_bp_data(struct kvm_vcpu *vcpu, > else if (dbg->arch.nr_hw_bp > MAX_BP_COUNT) > return -EINVAL; > > - bp_data = kmalloc_array(dbg->arch.nr_hw_bp, > - sizeof(*bp_data), > - GFP_KERNEL); Probably not an issue here, but if "sizeof(*bp_data) * dbg->arch.nr_hw_bp" overflows, kmalloc_array() would have returned NULL here... > - if (!bp_data) { > - ret = -ENOMEM; > - goto error; > - } > - > - if (copy_from_user(bp_data, > - dbg->arch.hw_bp, > - sizeof(*bp_data) * dbg->arch.nr_hw_bp)) { > - ret = -EFAULT; > - goto error; > - } > + bp_data = memdup_user(dbg->arch.hw_bp, > + sizeof(*bp_data) * dbg->arch.nr_hw_bp); ... while this would continue silently, and corrupt memory. > + if (IS_ERR(bp_data)) > + return PTR_ERR(bp_data); 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