From patchwork Wed Apr 1 00:34:16 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Richter X-Patchwork-Id: 1264978 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.ozlabs.org (lists.ozlabs.org [203.11.71.2]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 48sS2X6K9dz9sT6 for ; Wed, 1 Apr 2020 11:37:24 +1100 (AEDT) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=linux.ibm.com Received: from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) by lists.ozlabs.org (Postfix) with ESMTP id 48sS2X43vFzDqRh for ; Wed, 1 Apr 2020 11:37:24 +1100 (AEDT) X-Original-To: skiboot@lists.ozlabs.org Delivered-To: skiboot@lists.ozlabs.org Authentication-Results: lists.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=linux.ibm.com (client-ip=148.163.158.5; helo=mx0a-001b2d01.pphosted.com; envelope-from=erichte@linux.ibm.com; receiver=) Authentication-Results: lists.ozlabs.org; dmarc=none (p=none dis=none) header.from=linux.ibm.com Received: from mx0a-001b2d01.pphosted.com (mx0b-001b2d01.pphosted.com [148.163.158.5]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 48sRzN2HpbzDqnM for ; Wed, 1 Apr 2020 11:34:40 +1100 (AEDT) Received: from pps.filterd (m0098417.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.42/8.16.0.42) with SMTP id 0310Y2i5047403 for ; Tue, 31 Mar 2020 20:34:38 -0400 Received: from e06smtp02.uk.ibm.com (e06smtp02.uk.ibm.com [195.75.94.98]) by mx0a-001b2d01.pphosted.com with ESMTP id 304g85rdbv-1 (version=TLSv1.2 cipher=AES256-GCM-SHA384 bits=256 verify=NOT) for ; Tue, 31 Mar 2020 20:34:38 -0400 Received: from localhost by e06smtp02.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 1 Apr 2020 01:34:26 +0100 Received: from b06avi18878370.portsmouth.uk.ibm.com (9.149.26.194) by e06smtp02.uk.ibm.com (192.168.101.132) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; (version=TLSv1/SSLv3 cipher=AES256-GCM-SHA384 bits=256/256) Wed, 1 Apr 2020 01:34:25 +0100 Received: from d06av26.portsmouth.uk.ibm.com (d06av26.portsmouth.uk.ibm.com [9.149.105.62]) by b06avi18878370.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 0310YX0m31981916 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 1 Apr 2020 00:34:33 GMT Received: from d06av26.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id B81C2AE055; Wed, 1 Apr 2020 00:34:33 +0000 (GMT) Received: from d06av26.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 11B1EAE051; Wed, 1 Apr 2020 00:34:33 +0000 (GMT) Received: from ceres.ibmuc.com (unknown [9.80.234.145]) by d06av26.portsmouth.uk.ibm.com (Postfix) with ESMTP; Wed, 1 Apr 2020 00:34:32 +0000 (GMT) From: Eric Richter To: skiboot@lists.ozlabs.org Date: Tue, 31 Mar 2020 19:34:16 -0500 X-Mailer: git-send-email 2.21.1 In-Reply-To: <20200401003426.7198-1-erichte@linux.ibm.com> References: <20200401003426.7198-1-erichte@linux.ibm.com> MIME-Version: 1.0 X-TM-AS-GCONF: 00 x-cbid: 20040100-0008-0000-0000-000003682143 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 20040100-0009-0000-0000-00004A89A76D Message-Id: <20200401003426.7198-6-erichte@linux.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.138, 18.0.676 definitions=2020-03-31_07:2020-03-31, 2020-03-31 signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 malwarescore=0 spamscore=0 impostorscore=0 phishscore=0 priorityscore=1501 lowpriorityscore=0 mlxlogscore=248 bulkscore=0 suspectscore=1 adultscore=0 mlxscore=0 clxscore=1015 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2003020000 definitions=main-2003310195 Subject: [Skiboot] [PATCH v3 05/15] secvar_main: rework secvar_main error flow, make storage locking explicit X-BeenThere: skiboot@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Mailing list for skiboot development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: nayna@linux.ibm.com Errors-To: skiboot-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org Sender: "Skiboot" This patch adjusts the behavior of secvar_main to actually halt the boot in some form if there is an issue initializing secure variables. We halt in skiboot if the secvar storage driver fails to initialize, as this should only happen due to unresolvable hardware issues, and contains our secure boot state. For all other cases we enforce secure boot in the bootloader (secure mode flag is set, but there will be no keys). Previously, the storage driver was expected to handle any locking procedures implicitly as part of the write operation. This patch makes the locking behavior now explicit, and part of the secvar_main flow. The storage driver is now locked unconditionally when exiting secvar_main, and the lock() call should halt the boot if it encounters any sign of struggle. Signed-off-by: Eric Richter --- include/secvar.h | 7 ++++--- libstb/secvar/secvar_main.c | 32 +++++++++++++++++++++++++++++--- 2 files changed, 33 insertions(+), 6 deletions(-) diff --git a/include/secvar.h b/include/secvar.h index c41fb739..cabde036 100644 --- a/include/secvar.h +++ b/include/secvar.h @@ -9,9 +9,10 @@ struct secvar; struct secvar_storage_driver { - int (*load_bank)(struct list_head *bank, int section); - int (*write_bank)(struct list_head *bank, int section); - int (*store_init)(void); + int (*load_bank)(struct list_head *bank, int section); + int (*write_bank)(struct list_head *bank, int section); + int (*store_init)(void); + void (*lock)(void); uint64_t max_var_size; }; diff --git a/libstb/secvar/secvar_main.c b/libstb/secvar/secvar_main.c index 787e4473..536b1643 100644 --- a/libstb/secvar/secvar_main.c +++ b/libstb/secvar/secvar_main.c @@ -39,11 +39,14 @@ int secvar_main(struct secvar_storage_driver storage_driver, list_head_init(&variable_bank); list_head_init(&update_bank); + /* Failures here should indicate some kind of hardware problem, + * therefore we don't even attempt to continue */ rc = secvar_storage.store_init(); if (rc) - goto fail; + abort(); - /* Failures here should indicate some kind of hardware problem */ + /* Failures here may be recoverable (PNOR was corrupted, restore from backup) + * so we want to boot up to skiroot to give the user a chance to fix */ rc = secvar_storage.load_bank(&variable_bank, SECVAR_VARIABLE_BANK); if (rc) goto fail; @@ -60,6 +63,8 @@ int secvar_main(struct secvar_storage_driver storage_driver, rc = secvar_backend.pre_process(); if (rc) { prlog(PR_ERR, "Error in backend pre_process = %d\n", rc); + /* Early failure state, lock the storage */ + secvar_storage.lock(); goto out; } } @@ -81,11 +86,18 @@ int secvar_main(struct secvar_storage_driver storage_driver, rc = secvar_storage.write_bank(&variable_bank, SECVAR_VARIABLE_BANK); if (rc) goto out; - + } + /* Write (and probably clear) the update bank if .process() actually detected + * and handled updates in the update bank. Unlike above, this includes error + * cases, where the backend should probably be clearing the bank. + */ + if (rc != OPAL_EMPTY) { rc = secvar_storage.write_bank(&update_bank, SECVAR_UPDATE_BANK); if (rc) goto out; } + /* Unconditionally lock the storage at this point */ + secvar_storage.lock(); if (secvar_backend.post_process) { rc = secvar_backend.post_process(); @@ -98,9 +110,23 @@ int secvar_main(struct secvar_storage_driver storage_driver, prlog(PR_INFO, "secvar initialized successfully\n"); return OPAL_SUCCESS; + fail: + /* Early failure, base secvar support failed to initialize */ secvar_set_status("fail"); + secvar_storage.lock(); + secvar_set_secure_mode(); + + prerror("secvar failed to initialize, rc = %04x\n", rc); + return rc; + out: + /* Soft-failure, enforce secure boot in bootloader for debug/recovery */ + clear_bank_list(&variable_bank); + clear_bank_list(&update_bank); + secvar_storage.lock(); + secvar_set_secure_mode(); + prerror("secvar failed to initialize, rc = %04x\n", rc); return rc; }