From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=pass header.i=@linaro.org header.s=google header.b=qh3R3LcH; spf=pass (domain: linaro.org, ip: 209.85.166.68, mailfrom: ard.biesheuvel@linaro.org) Received: from mail-io1-f68.google.com (mail-io1-f68.google.com [209.85.166.68]) by groups.io with SMTP; Mon, 22 Apr 2019 15:40:37 -0700 Received: by mail-io1-f68.google.com with SMTP id n11so10902106ioh.1 for ; Mon, 22 Apr 2019 15:40:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=Qon2W2gXrfCI+bG2RPa2YaDgR9gPjKcI0Sv2f+ZB7Kc=; b=qh3R3LcHuc3o+qWlYVV57cJFd+bUlJxpAykfLpMtpaXWzQfmLZTXEzNopkvZGzc8O1 jrxFW8kqHDuAI2LSQlwq1C5mfsfN+KN+U9Otlk0jQpXphD4on4JB7YnPXHn5ds6eqpCX zG7jzwmQZZqABjnaTpQcjBX+kg4IoJZJAh2SKIYsTV+MlfssFkS5Uz8JBZAWlisYKW8N 7ZMNAOKv1vVPGWs/WbQ2jg+47gQ+j8h7WgnOldvyBTxpkZ+FPH5jVTEC8/Gn1dQCEVXj DUcQbunwgX0pbG4VPqMKa47KgeEV2Puhp6y7grdd+CaAhdZUqbLP6/A0geIjX8Guvn3N KauA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=Qon2W2gXrfCI+bG2RPa2YaDgR9gPjKcI0Sv2f+ZB7Kc=; b=uck4sGPoryMqY5HMIyktDhXsvXZdQbdAIYnVrbc0VJtMCnG0jq4VXII/COC/mATFW8 mUafeuemo9WHScfyXXhvLtgf6lcbmS9VK0w0zjnzdFoSRE45pzaaczzZxNR4lDTNeU1Y stkk8tWb+WdWTy04efBjFI2v9mCyMSWR4+4HxKwAel7hOsTcTLw2FGnamtSSnsm3nUot KS+zLaERMqSMeT0sBdhOdbvApA6XTd3JbugrWxPpn3vvzEvAr9dI4Uwr9FSvPzDlDdIM dlti0Q404IUhSDAfVkM9pUU0mL+WWBoI4MeY6HT7M5dnJceVY4wiDxKP0X8tp63Hz4CO dNuQ== X-Gm-Message-State: APjAAAVdow231FoEw8p/4cFBLvzSp7bvtAt79UWTRjWxe8Pr1zMEHjxc Y7r5riGkQ0MypDHXbiOrLsEW4LKFQoK3rkWv7Wk6+jwv650= X-Google-Smtp-Source: APXvYqxvXGJEsIPqsEmVjT0RcdRWzxa1IITTKlJOP7cQJiHwV3esTT4yai2lo25qbMhYUS3flnyqU57aKg8rIPApNTQ= X-Received: by 2002:a5e:9b17:: with SMTP id j23mr15260885iok.60.1555972836872; Mon, 22 Apr 2019 15:40:36 -0700 (PDT) MIME-Version: 1.0 References: <20190420103454.5148-1-ard.biesheuvel@linaro.org> In-Reply-To: From: "Ard Biesheuvel" Date: Tue, 23 Apr 2019 00:40:23 +0200 Message-ID: Subject: Re: [edk2-devel] [PATCH v2] MdeModulePkg/DxeCapsuleLibFmp: avoid ESRT accesses at runtime To: edk2-devel-groups-io , "Kinney, Michael D" Cc: "Wu, Hao A" , "Wang, Jian J" Content-Type: text/plain; charset="UTF-8" On Tue, 23 Apr 2019 at 00:37, Michael D Kinney wrote: > > Ard, > > I only see one potential issue. > > The size of the buffer copied is based on the FwResourceCount field. > The actual size of based on the FwResourceCountMax field. Your patch > does not set FwResourceCountMax to FwResourceCount, so this may confuse > the OS consumers that may think the buffer allocated for ESRT is larger > that is actually is. > > /// > /// The number of firmware resources in the table, must not be zero. > /// > UINT32 FwResourceCount; > /// > /// The maximum number of resource array entries that can be within the table > /// without reallocating the table, must not be zero. > /// > UINT32 FwResourceCountMax; > The OS never sees our copy of the table. That copy is for internal use only by the DxeCapsuleLibFmp implementation. > The simplest fix is to use FwResourceCountMax instead of FwResourceCount > for the copy size. The other option is to set FwResourceCountMax to > FwResourceCountMax after the copy. > Given the above, I'll go with the latter. It is unlikely to matter, but it is more correct, and prevents any potential confusion in the future. > The rest of the patch looks good. With one of the two changes above: > > Reviewed-by: Michael D Kinney > Thanks. > > > -----Original Message----- > > From: devel@edk2.groups.io > > [mailto:devel@edk2.groups.io] On Behalf Of Ard > > Biesheuvel > > Sent: Monday, April 22, 2019 3:03 PM > > To: Wu, Hao A > > Cc: devel@edk2.groups.io; Wang, Jian J > > ; Kinney, Michael D > > > > Subject: Re: [edk2-devel] [PATCH v2] > > MdeModulePkg/DxeCapsuleLibFmp: avoid ESRT accesses at > > runtime > > > > On Mon, 22 Apr 2019 at 09:14, Wu, Hao A > > wrote: > > > > > > > -----Original Message----- > > > > From: devel@edk2.groups.io > > [mailto:devel@edk2.groups.io] On Behalf Of Ard > > > > Biesheuvel > > > > Sent: Saturday, April 20, 2019 6:35 PM > > > > To: devel@edk2.groups.io > > > > Cc: Wu, Hao A; Wang, Jian J; Kinney, Michael D; Ard > > Biesheuvel > > > > Subject: [edk2-devel] [PATCH v2] > > MdeModulePkg/DxeCapsuleLibFmp: avoid > > > > ESRT accesses at runtime > > > > > > Hello Ard, > > > > > > It seems to me v2 patch makes a copy of the ESRT for > > runtime usage (rather > > > than avoid using ESRT), so the title of the commit > > may need update as > > > well. > > > > > > Could you help to update the patch subject when you > > push the change? > > > > > > Other than that, the patch looks good to me. But I > > would like to see if > > > Mike has additional comments on this: > > > > > > Acked-by: Hao Wu > > > > > > > Thanks Hao. I will modify the subject to 'clone ESRT > > for runtime access' > > > > Mike: any comments? > > > > > > > >