From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from lindbergh.monkeyblade.net ([23.128.96.19]:41862 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1345773AbiCHKLt (ORCPT ); Tue, 8 Mar 2022 05:11:49 -0500 Received: from mx0b-00069f02.pphosted.com (mx0b-00069f02.pphosted.com [205.220.177.32]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0835C3CA7D for ; Tue, 8 Mar 2022 02:10:32 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oracle.com; h=date : from : to : cc : subject : message-id : references : content-type : in-reply-to : mime-version; s=corp-2021-07-09; bh=P1r1K+4YuH63KR0+oHZedrlKfjWA3twiHCAs24pT9qY=; b=s8QJ4o1W6Uw7cq+lLdYDmmrmeozmd6A9d2nPTNBuU5+/UfJ4Fgrk9QCvQeNN93FRC5ii X5z5Aw1uUkMo9wuPsSzxaN8p8ZUpdTki9PmOxysNYOWWzGINE9Xs6gM1RKh/B/5TyPes nO7nxRJF+ORbL2y1yy8owT2nagBUC+wkhPHjWCtbJ+j2pCIikuFSKVwUk3rydOcPrqOt KecG5iBKLepMOKVJnnESNeM6SWGzJr0mt5n+T/vTnARJa+IS3rPwvZgulyRhAjGOgpuh WvKbLr3TJj+qvg6EnEqpo9bi2ip4kqRwut5y393P4elcWQ/LB8rX7p1YwsHLvvgQxtz6 3A== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oracle.onmicrosoft.com; s=selector2-oracle-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=P1r1K+4YuH63KR0+oHZedrlKfjWA3twiHCAs24pT9qY=; b=rooRX5B4deIJbAbJcYICd4OhKx5LbNd5zYCloL/Si9he326m1ZYhWzlNU9mSC47UJT6xTOs1ZacPjO/3rr42jYl/0lW01v3/SUhJoK1rrmTjrt2Ji83FudOW53T9FH4LFf19rXQNZBQxVtKdvkXTw9sFMbbgZpqmdj3fUtYaDzk= Date: Tue, 8 Mar 2022 13:10:09 +0300 From: Dan Carpenter Subject: Re: [PATCH 1/1] check_host_input: add a pattern Message-ID: <20220308101009.GF3315@kadam> References: <20220308082513.549442-1-elena.reshetova@intel.com> <20220308082513.549442-2-elena.reshetova@intel.com> Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220308082513.549442-2-elena.reshetova@intel.com> MIME-Version: 1.0 List-ID: To: Elena Reshetova Cc: smatch@vger.kernel.org Hi Elena, I'm super happy to help you with anything you want. We could talk on zoom if that's a useful thing. I can merge this patch but it feels like maybe it should be turned off by default. There should be some command line argument for it. On Tue, Mar 08, 2022 at 10:25:13AM +0200, Elena Reshetova wrote: > check_host_input pattern helps identifying locations > where a guest kernel can take a potentially malicious > input from the untrusted host in a confidential computing > threat model. The output of the pattern helps to > facilitate manual code audit and cross verify fuzzing > coverage. > > Signed-off-by: Elena Reshetova > --- > check_host_input.c | 886 +++++++++++++++++++++++++++++++++++++++++++++ > check_list.h | 2 + > 2 files changed, 888 insertions(+) > create mode 100644 check_host_input.c > > diff --git a/check_host_input.c b/check_host_input.c > new file mode 100644 > index 00000000..dc18ab34 > --- /dev/null > +++ b/check_host_input.c > @@ -0,0 +1,886 @@ > +/* > + * Smatch pattern to facilitate the hardening of the Linux guest kernel > + * for Confidential Cloud Computing threat model. > + * In this model the Linux guest kernel cannot trust the values > + * it obtains using low level IO functions because they can be provided > + * by a potentially malicious host or VMM. Instead it needs to make > + * sure the code that handles processing of such values is hardened, > + * free of memory safety issues and other potential security issues. > + * > + * This smatch pattern helps to indentify such places. > + * Currently it covers most of MSR, portIO, MMIO and cpuid reading primitives. > + * The full list of covered functions is stored in host_input_funcs array. > + * The output of the pattern can be used to facilitate code audit, as > + * well as to verify that utilized fuzzing strategy can reach all the > + * code paths that can take a low-level input from a potentially malicious host. > + * > + * When ran, the pattern produces two types of findings: errors and warnings. > + * This is done to help prioritizing the issues for the manual code audit. > + * However if time permits all locations reported by the pattern should be checked. > + * > + * Written based on existing smatch patterns. > + * > + * Author: Elena Reshetova > + * Copyright (c) 2021-22, Intel Corporation > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2, as > + * published by the Free Software Foundation. > + * > + */ > +#include "smatch.h" > +#include "smatch_slist.h" > +#include "smatch_extra.h" > +#include > + > +static int my_id; > + > +STATE(local_variables); > +STATE(local_from_host); > +STATE(tainted_from_host); > +STATE(called_funcs); My instinct is that these are too many states. Try to limit it to one or two states per file and the code becomes a lot simpler. The "local_variables" state is not required. static bool is_local(struct expression *expr) { expr = strip_expr(expr); if (!expr) return false; if (expr->type != EXPR_SYMBOL) return false; /* FIXME: why not just check if MOD_AUTO is set instead? */ if (expr->symbol->ctype.modifiers & (MOD_NONLOCAL | MOD_STATIC | MOD_ADDRESSABLE)) return false; return true; } called_funcs is not required either. You can use the "cur_func_sym" global variable for that. static int get_func_start_lineno(void) { if (!cur_func_sym) return -1; return cur_func_sym->pos.line; } I've looked through this file and there are ways that it could probably be updated to newer better APIs but it sounds like the other in progress patch is the more interesting problem. regards, dan carpenter