From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id EB0EDC433EF for ; Sat, 2 Oct 2021 20:11:16 +0000 (UTC) Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id F23DA61B3C for ; Sat, 2 Oct 2021 20:11:15 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org F23DA61B3C Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linaro.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.denx.de Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 2198082903; Sat, 2 Oct 2021 22:11:13 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=linaro.org header.i=@linaro.org header.b="UrltTjqi"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 4360C829FC; Sat, 2 Oct 2021 22:11:11 +0200 (CEST) Received: from mail-wr1-x429.google.com (mail-wr1-x429.google.com [IPv6:2a00:1450:4864:20::429]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id F05CA822D7 for ; Sat, 2 Oct 2021 22:11:07 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=ilias.apalodimas@linaro.org Received: by mail-wr1-x429.google.com with SMTP id s21so21455788wra.7 for ; Sat, 02 Oct 2021 13:11:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=JfIIawcsSezw1CzDd12QErQORXNcA8AOBSVRrNUYbIU=; b=UrltTjqi4hSxAu7vGQH+qIrioylmbNRdJOIq35dt5VYHFFpxpuNfaBhweHcLKHC/Ni KXOQh29TG6vPHlFHTOBXVPa59A5JJ9UjsSpnMHyKjsnI342HP1hJSY7FaHMehbUcx14m hts2plDgOag4+FfrF5J4SrGMlX8DCdfUQuPcmdhBmPLGwH+RWcuLqiVssRnJx+ndvvt7 fX9WCTOLinde9Wgsz52UgGYxGY6GARhbp6G9C5nTQvoHYi07QU9/ShKdTZDUcWM5ij7E mv09YwwmxKWtotR9g9gU2+quYN0PtNpFZNgjvZqtgNEuqzPvqVu2sM4gVu1NMxr0jRe5 XXqg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=JfIIawcsSezw1CzDd12QErQORXNcA8AOBSVRrNUYbIU=; b=A83DhksSRcYmP2sEc1JL0tanBQsyX6Bh/dnIEr2GsOx10OSAleTt5SPJ9jn2AHWKnW ttW96iez3O2/UImcC4wEm6boeqLZJhQzGwVVZLbiHFBvGS3S8sGG+zUP39dlQyWL+xV+ 9Xgjljoyyj3zEL07rWnGGqgwMbnwcCy9cPgFQmWyzTuhtIxgv5IESbXoCUj+prWee0dq Q1ixE19k+XB448EnRNjQ9sPntf2x9z7owrv3hQhJjPBjfL2zsElxdrqmPYJI4CEDWzAz UTiktWXPc91ZvQgvRQIucHEOGMqwKaHR0751FU6cKzz8tTkqgBAfpcNk41q3GVrIsZoe jGtA== X-Gm-Message-State: AOAM531v4PYYISTZkbkp+TbV1lBt0OlQcYn5Cgox3mMh4EejYA0P3YJp RsZ2lTHHjN/9NYHFmLb/dwMGtydafx5pRjya X-Google-Smtp-Source: ABdhPJzdrr4difGyNxR+wVEqh7KwuH/iS2bwr60yorAMVDOW62stCXDcLgyIwmA4iQeAqKeorba6Gg== X-Received: by 2002:adf:ab06:: with SMTP id q6mr4867740wrc.433.1633205467480; Sat, 02 Oct 2021 13:11:07 -0700 (PDT) Received: from apalos.home (ppp-94-66-220-209.home.otenet.gr. [94.66.220.209]) by smtp.gmail.com with ESMTPSA id s3sm10504856wrt.54.2021.10.02.13.11.06 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 02 Oct 2021 13:11:06 -0700 (PDT) Date: Sat, 2 Oct 2021 23:11:04 +0300 From: Ilias Apalodimas To: Heinrich Schuchardt Cc: u-boot@lists.denx.de, Alexander Graf , Masahisa Kojima Subject: Re: [PATCH v2 3/4] efi_loader: simplify efi_sigstore_parse_sigdb() Message-ID: References: <20211002094227.119870-1-heinrich.schuchardt@canonical.com> <20211002094227.119870-4-heinrich.schuchardt@canonical.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20211002094227.119870-4-heinrich.schuchardt@canonical.com> X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.34 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.2 at phobos.denx.de X-Virus-Status: Clean Hi Heinrich, [...] > @@ -740,44 +741,15 @@ err: > */ > struct efi_signature_store *efi_sigstore_parse_sigdb(u16 *name) > { > - struct efi_signature_store *sigstore = NULL; > const efi_guid_t *vendor; > void *db; > efi_uintn_t db_size; > - efi_status_t ret; > - > - if (!u16_strcmp(name, L"PK") || !u16_strcmp(name, L"KEK")) { > - vendor = &efi_global_variable_guid; > - } else if (!u16_strcmp(name, L"db") || !u16_strcmp(name, L"dbx")) { > - vendor = &efi_guid_image_security_database; > - } else { > - EFI_PRINT("unknown signature database, %ls\n", name); > - return NULL; > - } > - > - /* retrieve variable data */ > - db_size = 0; > - ret = EFI_CALL(efi_get_variable(name, vendor, NULL, &db_size, NULL)); > - if (ret == EFI_NOT_FOUND) { > - EFI_PRINT("variable, %ls, not found\n", name); > - sigstore = calloc(sizeof(*sigstore), 1); > - return sigstore; > - } else if (ret != EFI_BUFFER_TOO_SMALL) { > - EFI_PRINT("Getting variable, %ls, failed\n", name); > - return NULL; > - } > > - db = malloc(db_size); > + vendor = efi_auth_var_get_guid(name); > + db = efi_get_var(name, vendor, &db_size); > if (!db) { > - EFI_PRINT("Out of memory\n"); > - return NULL; > - } > - > - ret = EFI_CALL(efi_get_variable(name, vendor, NULL, &db_size, db)); > - if (ret != EFI_SUCCESS) { > - EFI_PRINT("Getting variable, %ls, failed\n", name); > - free(db); > - return NULL; > + EFI_PRINT("variable, %ls, not found\n", name); > + return calloc(sizeof(struct efi_signature_store), 1); We are creating a security problem here. The previous code, before returning a calloced buffer, was specifically checking for EFI_NOT_FOUND. The way this is restructured might lead to an weird situation. There's a chance efi_get_var() will return NULL, even if the variable is there (e.g an alloc failed). That's problematic if we want to check 'dbx' though. The new fucntion will return that empty buffer and we'll end up ignoring the 'dbx' entries. Regards /Ilias > } > > return efi_build_signature_store(db, db_size); > -- > 2.32.0 >