From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:40360 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750783AbdDBCYQ (ORCPT ); Sat, 1 Apr 2017 22:24:16 -0400 Received: from pps.filterd (m0098410.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.20/8.16.0.20) with SMTP id v322O6Lx144604 for ; Sat, 1 Apr 2017 22:24:16 -0400 Received: from e28smtp07.in.ibm.com (e28smtp07.in.ibm.com [125.16.236.7]) by mx0a-001b2d01.pphosted.com with ESMTP id 29j97f43c8-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Sat, 01 Apr 2017 22:24:15 -0400 Received: from localhost by e28smtp07.in.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Sun, 2 Apr 2017 07:54:12 +0530 Subject: Re: [PATCH] KEYS: encrypted: avoid encrypting/decrypting stack buffers From: Mimi Zohar To: Eric Biggers , keyrings@vger.kernel.org Cc: David Howells , Andy Lutomirski , Herbert Xu , Eric Biggers , linux-kernel@vger.kernel.org, stable@vger.kernel.org Date: Sat, 01 Apr 2017 22:23:57 -0400 In-Reply-To: <20170401191709.25170-1-ebiggers3@gmail.com> References: <20170401191709.25170-1-ebiggers3@gmail.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Message-Id: <1491099837.3499.163.camel@linux.vnet.ibm.com> Sender: stable-owner@vger.kernel.org List-ID: Hi Eric, On Sat, 2017-04-01 at 12:17 -0700, Eric Biggers wrote: > From: Eric Biggers > > Since v4.9, the crypto API cannot (normally) be used to encrypt/decrypt > stack buffers because the stack may be virtually mapped. Fix this for > the padding buffers in encrypted-keys by using ZERO_PAGE for the > encryption padding and by allocating a temporary heap buffer for the > decryption padding. > > Tested with CONFIG_DEBUG_SG=y: > keyctl new_session > keyctl add user master "abcdefghijklmnop" @s > keyid=$(keyctl add encrypted desc "new user:master 25" @s) > datablob="$(keyctl pipe $keyid)" > keyctl unlink $keyid > keyid=$(keyctl add encrypted desc "load $datablob" @s) > datablob2="$(keyctl pipe $keyid)" > [ "$datablob" = "$datablob2" ] && echo "Success!" Have you created an encrypted key on a kernel without this patch and attempted to load that key on a kernel with this patch?  Does it still work? Mimi > > Cc: Andy Lutomirski > Cc: Herbert Xu > Cc: Mimi Zohar > Cc: stable@vger.kernel.org # 4.9+ > Signed-off-by: Eric Biggers > --- > security/keys/encrypted-keys/encrypted.c | 17 +++++++++-------- > 1 file changed, 9 insertions(+), 8 deletions(-) > > diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c > index 0010955d7876..1845d47474a0 100644 > --- a/security/keys/encrypted-keys/encrypted.c > +++ b/security/keys/encrypted-keys/encrypted.c > @@ -480,12 +480,9 @@ static int derived_key_encrypt(struct encrypted_key_payload *epayload, > struct skcipher_request *req; > unsigned int encrypted_datalen; > u8 iv[AES_BLOCK_SIZE]; > - unsigned int padlen; > - char pad[16]; > int ret; > > encrypted_datalen = roundup(epayload->decrypted_datalen, blksize); > - padlen = encrypted_datalen - epayload->decrypted_datalen; > > req = init_skcipher_req(derived_key, derived_keylen); > ret = PTR_ERR(req); > @@ -493,11 +490,10 @@ static int derived_key_encrypt(struct encrypted_key_payload *epayload, > goto out; > dump_decrypted_data(epayload); > > - memset(pad, 0, sizeof pad); > sg_init_table(sg_in, 2); > sg_set_buf(&sg_in[0], epayload->decrypted_data, > epayload->decrypted_datalen); > - sg_set_buf(&sg_in[1], pad, padlen); > + sg_set_page(&sg_in[1], ZERO_PAGE(0), AES_BLOCK_SIZE, 0); > > sg_init_table(sg_out, 1); > sg_set_buf(sg_out, epayload->encrypted_data, encrypted_datalen); > @@ -584,9 +580,14 @@ static int derived_key_decrypt(struct encrypted_key_payload *epayload, > struct skcipher_request *req; > unsigned int encrypted_datalen; > u8 iv[AES_BLOCK_SIZE]; > - char pad[16]; > + u8 *pad; > int ret; > > + /* Throwaway buffer to hold the unused zero padding at the end */ > + pad = kmalloc(AES_BLOCK_SIZE, GFP_KERNEL); > + if (!pad) > + return -ENOMEM; > + > encrypted_datalen = roundup(epayload->decrypted_datalen, blksize); > req = init_skcipher_req(derived_key, derived_keylen); > ret = PTR_ERR(req); > @@ -594,13 +595,12 @@ static int derived_key_decrypt(struct encrypted_key_payload *epayload, > goto out; > dump_encrypted_data(epayload, encrypted_datalen); > > - memset(pad, 0, sizeof pad); > sg_init_table(sg_in, 1); > sg_init_table(sg_out, 2); > sg_set_buf(sg_in, epayload->encrypted_data, encrypted_datalen); > sg_set_buf(&sg_out[0], epayload->decrypted_data, > epayload->decrypted_datalen); > - sg_set_buf(&sg_out[1], pad, sizeof pad); > + sg_set_buf(&sg_out[1], pad, AES_BLOCK_SIZE); > > memcpy(iv, epayload->iv, sizeof(iv)); > skcipher_request_set_crypt(req, sg_in, sg_out, encrypted_datalen, iv); > @@ -612,6 +612,7 @@ static int derived_key_decrypt(struct encrypted_key_payload *epayload, > goto out; > dump_decrypted_data(epayload); > out: > + kfree(pad); > return ret; > } >