From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eu-smtp-delivery-151.mimecast.com (eu-smtp-delivery-151.mimecast.com [185.58.85.151]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 8D5AF70 for ; Wed, 12 May 2021 08:04:39 +0000 (UTC) Received: from AcuMS.aculab.com (156.67.243.121 [156.67.243.121]) (Using TLS) by relay.mimecast.com with ESMTP id uk-mta-226-GJ3a8MFKOOuK1O-e4s34MQ-1; Wed, 12 May 2021 09:04:35 +0100 X-MC-Unique: GJ3a8MFKOOuK1O-e4s34MQ-1 Received: from AcuMS.Aculab.com (fd9f:af1c:a25b:0:994c:f5c2:35d6:9b65) by AcuMS.aculab.com (fd9f:af1c:a25b:0:994c:f5c2:35d6:9b65) with Microsoft SMTP Server (TLS) id 15.0.1497.2; Wed, 12 May 2021 09:04:33 +0100 Received: from AcuMS.Aculab.com ([fe80::994c:f5c2:35d6:9b65]) by AcuMS.aculab.com ([fe80::994c:f5c2:35d6:9b65%12]) with mapi id 15.00.1497.015; Wed, 12 May 2021 09:04:33 +0100 From: David Laight To: 'Joerg Roedel' , "x86@kernel.org" , Hyunwook Baek CC: Joerg Roedel , "stable@vger.kernel.org" , "hpa@zytor.com" , Andy Lutomirski , Dave Hansen , Peter Zijlstra , Jiri Slaby , Dan Williams , Tom Lendacky , "Juergen Gross" , Kees Cook , David Rientjes , Cfir Cohen , Erdem Aktas , Masami Hiramatsu , Mike Stunes , Sean Christopherson , Martin Radev , Arvind Sankar , "linux-coco@lists.linux.dev" , "linux-kernel@vger.kernel.org" , "kvm@vger.kernel.org" , "virtualization@lists.linux-foundation.org" Subject: RE: [PATCH 3/6] x86/sev-es: Use __put_user()/__get_user Thread-Topic: [PATCH 3/6] x86/sev-es: Use __put_user()/__get_user Thread-Index: AQHXRwQswpicLJM6a0eoGXhfP+3kQqrfe29g Date: Wed, 12 May 2021 08:04:33 +0000 Message-ID: <0496626f018d4d27a8034a4822170222@AcuMS.aculab.com> References: <20210512075445.18935-1-joro@8bytes.org> <20210512075445.18935-4-joro@8bytes.org> In-Reply-To: <20210512075445.18935-4-joro@8bytes.org> Accept-Language: en-GB, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ms-exchange-transport-fromentityheader: Hosted x-originating-ip: [10.202.205.107] X-Mailing-List: linux-coco@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=C51A453 smtp.mailfrom=david.laight@aculab.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: aculab.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable From: Joerg > Sent: 12 May 2021 08:55 >=20 > From: Joerg Roedel >=20 > The put_user() and get_user() functions do checks on the address which is > passed to them. They check whether the address is actually a user-space > address and whether its fine to access it. They also call might_fault() > to indicate that they could fault and possibly sleep. >=20 > All of these checks are neither wanted nor required in the #VC exception > handler, which can be invoked from almost any context and also for MMIO > instructions from kernel space on kernel memory. All the #VC handler > wants to know is whether a fault happened when the access was tried. >=20 > This is provided by __put_user()/__get_user(), which just do the access > no matter what. That can't be right at all. __put/get_user() are only valid on user addresses and will try to fault in a missing page - so can sleep. At best this is abused the calls. =09David > Fixes: f980f9c31a92 ("x86/sev-es: Compile early handler code into kernel = image") > Cc: stable@vger.kernel.org # v5.10+ > Signed-off-by: Joerg Roedel > --- > arch/x86/kernel/sev.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) >=20 > diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c > index 6530a844eb61..110b39345b40 100644 > --- a/arch/x86/kernel/sev.c > +++ b/arch/x86/kernel/sev.c > @@ -342,22 +342,22 @@ static enum es_result vc_write_mem(struct es_em_ctx= t *ctxt, > =09switch (size) { > =09case 1: > =09=09memcpy(&d1, buf, 1); > -=09=09if (put_user(d1, target)) > +=09=09if (__put_user(d1, target)) > =09=09=09goto fault; > =09=09break; > =09case 2: > =09=09memcpy(&d2, buf, 2); > -=09=09if (put_user(d2, target)) > +=09=09if (__put_user(d2, target)) > =09=09=09goto fault; > =09=09break; > =09case 4: > =09=09memcpy(&d4, buf, 4); > -=09=09if (put_user(d4, target)) > +=09=09if (__put_user(d4, target)) > =09=09=09goto fault; > =09=09break; > =09case 8: > =09=09memcpy(&d8, buf, 8); > -=09=09if (put_user(d8, target)) > +=09=09if (__put_user(d8, target)) > =09=09=09goto fault; > =09=09break; > =09default: > @@ -396,22 +396,22 @@ static enum es_result vc_read_mem(struct es_em_ctxt= *ctxt, >=20 > =09switch (size) { > =09case 1: > -=09=09if (get_user(d1, s)) > +=09=09if (__get_user(d1, s)) > =09=09=09goto fault; > =09=09memcpy(buf, &d1, 1); > =09=09break; > =09case 2: > -=09=09if (get_user(d2, s)) > +=09=09if (__get_user(d2, s)) > =09=09=09goto fault; > =09=09memcpy(buf, &d2, 2); > =09=09break; > =09case 4: > -=09=09if (get_user(d4, s)) > +=09=09if (__get_user(d4, s)) > =09=09=09goto fault; > =09=09memcpy(buf, &d4, 4); > =09=09break; > =09case 8: > -=09=09if (get_user(d8, s)) > +=09=09if (__get_user(d8, s)) > =09=09=09goto fault; > =09=09memcpy(buf, &d8, 8); > =09=09break; > -- > 2.31.1 - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1= PT, UK Registration No: 1397386 (Wales)