From patchwork Wed Nov 23 20:51:17 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ondrej Kozina X-Patchwork-Id: 9444391 X-Patchwork-Delegate: snitzer@redhat.com Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id AED3260779 for ; Wed, 23 Nov 2016 20:52:46 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id A9339276AE for ; Wed, 23 Nov 2016 20:52:46 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 962D327D64; Wed, 23 Nov 2016 20:52:46 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from mx4-phx2.redhat.com (mx4-phx2.redhat.com [209.132.183.25]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id F14EC27C2D for ; Wed, 23 Nov 2016 20:52:44 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by mx4-phx2.redhat.com (8.13.8/8.13.8) with ESMTP id uANKpOjk015427; Wed, 23 Nov 2016 15:51:24 -0500 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id uANKpMWa004127 for ; Wed, 23 Nov 2016 15:51:22 -0500 Received: from dhcp131-147.brq.redhat.com (ovpn-204-39.brq.redhat.com [10.40.204.39]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id uANKpK3n031262; Wed, 23 Nov 2016 15:51:20 -0500 From: Ondrej Kozina To: dm-devel@redhat.com Date: Wed, 23 Nov 2016 21:51:17 +0100 Message-Id: <1479934277-14913-1-git-send-email-okozina@redhat.com> In-Reply-To: <20161121154049.GA13248@redhat.com> References: <20161121154049.GA13248@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.23 X-loop: dm-devel@redhat.com Cc: dhowells@redhat.com, aryabinin@virtuozzo.com, mpatocka@redhat.com, snitzer@redhat.com, mbroz@redhat.com Subject: [dm-devel] [PATCH] dm-crypt: check key payload pointer not null X-BeenThere: dm-devel@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: device-mapper development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: dm-devel-bounces@redhat.com Errors-To: dm-devel-bounces@redhat.com X-Virus-Scanned: ClamAV using ClamSMTP Hi Mike, those are fixes for latest version (including your cleanup patch). I've fixed one awkward bug the rest is based on David's sugestions he gave me earlier today. David, may I also kindly ask you for oficial 'Reviewed-by' stamp provided it's correct now (with regard to kernel keyring bits)? bugfix: harden checks for 'user' or 'logon' keywords in key_string (it allowed to pass with :logo:... or "use:...) bugfix: we have to check user_key_payload reference after rcu_read_lock(). Otherwise keys revoked between request_key() and rcu_read_lock() calls would return NULL pointer. improvement: calling key_validate() right after request_key() is useless. Exactly same check is performed inside request_key() routine. improvement: do not check the whole key_type string again. We already know it's either 'logon' or 'user'. Read just first char. Reviewed-by: David Howells --- drivers/md/dm-crypt.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c index c6ff083..78ab5e8 100644 --- a/drivers/md/dm-crypt.c +++ b/drivers/md/dm-crypt.c @@ -1501,31 +1501,31 @@ static int crypt_set_keyring_key(struct crypt_config *cc, const char *key_string if (!key_desc || key_desc == key_string || !strlen(key_desc + 1)) return -EINVAL; - if (strncmp(key_string, "logon", key_desc - key_string) && - strncmp(key_string, "user", key_desc - key_string)) + if (strncmp(key_string, "logon:", key_desc - key_string + 1) && + strncmp(key_string, "user:", key_desc - key_string + 1)) return -EINVAL; new_key_string = kstrdup(key_string, GFP_KERNEL); if (!new_key_string) return -ENOMEM; - key = request_key(strncmp(key_string, "user", 4) ? &key_type_logon : &key_type_user, + key = request_key(key_string[0] == 'l' ? &key_type_logon : &key_type_user, key_desc + 1, NULL); if (IS_ERR(key)) { kzfree(new_key_string); return PTR_ERR(key); } - ret = key_validate(key); - if (ret < 0) { + rcu_read_lock(); + + ukp = user_key_payload(key); + if (!ukp) { + rcu_read_unlock(); key_put(key); kzfree(new_key_string); - return ret; + return -EKEYREVOKED; } - rcu_read_lock(); - - ukp = user_key_payload(key); if (cc->key_size != ukp->datalen) { rcu_read_unlock(); key_put(key);