From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4001:c06::244; helo=mail-io0-x244.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-io0-x244.google.com (mail-io0-x244.google.com [IPv6:2607:f8b0:4001:c06::244]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id BF349210F4BC6 for ; Fri, 24 Aug 2018 07:55:30 -0700 (PDT) Received: by mail-io0-x244.google.com with SMTP id q4-v6so7346574iob.8 for ; Fri, 24 Aug 2018 07:55:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=RJxMcv0KeZATHJkmEZhSLe0pFwfGfMIHVMySPjF7XX0=; b=Ize5r+PDOnfP64v9Usf/RikAMUAV4s8nAg/b2fquc/6dmHzs3zXcSDMkLcaNq0+H1+ xuM4w70b31Er2F2T4N4layUGe4gdIOq9OvqbrfmK1o9MeLxNf6rdTfTa5xuhLkjx8nNk xb+0M0psPa7v8lG+GnoqtINOKhQOp13H7Xevk= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=RJxMcv0KeZATHJkmEZhSLe0pFwfGfMIHVMySPjF7XX0=; b=LcAirTZa1VLCk5p+JuxvB5uWxDyresI7hOCA3WwpCbmfr3AZDe8CxC7DGNvkntOpKh rH+JAKnKJL7SatazyjW6pZZmFkJfRUyY13hoy+a3SV7LV/mJwjVkPp5kEqHbGT5+FAO0 F5uU9nYxON/jMClRntBXc7vwGLVtEvvOnWvKW8JyNHrmn7318wpr6IOEcoDNvqE3OKG7 Cq8/D+gLwSaw9p2L6y8T7IDp/CWQB5PgdJvqA+JvvR7Ps6YoRLJbjAW3GghR+Lm/3tMP qNo0l6bmq4CL+p5ZszZ+F3U01nPrXS5nKK/7OKu3bHSKWYuMfT9Z2yZ0GPxpI6LxptYV i+TQ== X-Gm-Message-State: APzg51Cldq5CW6g+9/HL6Zxe5/eXWYN0CmIZaR1pngVltEsoD+UoAtq1 RETxsaa+IIm5hYfLkl+3tYVANDOZvAw122f5ivyizZfM3Ao= X-Google-Smtp-Source: ANB0VdbO9niT+kTH4dU0d7E8+Igie5Z+SV+zwbuDqO9XMP9c51ymM4BVQ70x91Zf3brTRpCbKXlDJqRC/lzyuuijkkQ= X-Received: by 2002:a6b:450f:: with SMTP id s15-v6mr1611772ioa.60.1535122529916; Fri, 24 Aug 2018 07:55:29 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a6b:ac05:0:0:0:0:0 with HTTP; Fri, 24 Aug 2018 07:55:29 -0700 (PDT) In-Reply-To: <20180821065047.GA17216@arm.com> References: <1532090300-5250-1-git-send-email-sughosh.ganu@arm.com> <1532090300-5250-8-git-send-email-sughosh.ganu@arm.com> <1532367194.3302.36.camel@arm.com> <20180821065047.GA17216@arm.com> From: Ard Biesheuvel Date: Fri, 24 Aug 2018 15:55:29 +0100 Message-ID: To: Sughosh Ganu Cc: Leif Lindholm , Supreeth Venkatesh , "edk2-devel@lists.01.org" Subject: Re: [PATCH v2 7/7] ArmPkg: Extra action to update permissions for S-ELO MM Image X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 24 Aug 2018 14:55:31 -0000 Content-Type: text/plain; charset="UTF-8" On 21 August 2018 at 07:50, Sughosh Ganu wrote: > hi Ard, > > On Tue July 23, 2018 at 11:03PM +0530, Supreeth Venkatesh wrote: >> >> On Sat, 2018-07-21 at 20:06 +0900, Ard Biesheuvel wrote: >> > On 20 July 2018 at 21:38, Sughosh Ganu wrote: >> > > >> > > From: Achin Gupta >> > > >> > > The Standalone MM drivers runs in S-EL0 in AArch64 on ARM Standard >> > > Platforms and is deployed during SEC phase. The memory allocated to >> > > the Standalone MM drivers should be marked as RO+X. >> > > >> > > During PE/COFF Image section parsing, this patch implements extra >> > > action "UpdatePeCoffPermissions" to request the privileged firmware >> > > in >> > > EL3 to update the permissions. >> > > >> > > Contributed-under: TianoCore Contribution Agreement 1.1 >> > > Signed-off-by: Sughosh Ganu >> > Apologies for bringing this up only now, but I don't think I was ever >> > cc'ed on these patches. >> > >> Apologies if you have missed it. But I am pretty sure it was part of >> earlier large patch-set on which you and leif were copied, as it was >> part of ArmPkg. >> > >> > We are relying on a debug hook in the PE/COFF loader to ensure that >> > we >> > don't end up with memory that is both writable and executable in the >> > secure world. Do we really think that is a good idea? >> > >> > (I know this code was derived from a proof of concept that I did >> > years >> > ago, but that was just a PoC) >> I think we need a little bit more details on what is your suggestion? >> >> A little bit background here: This code runs in S-EL0 and Request gets >> sent to secure world SPM to ensure that the region permissions are >> updated correctly via the "ArmMmuStandaloneMmCoreLib" SVC - >> ARM_SVC_ID_SP_SET_MEM_ATTRIBUTES_AARCH64. >> >> DebugPeCoffExtraActionLib is just used to extract image region >> information, but the region permission >> update request is sent to secure world for validation. >> >> With the above explanation, can you provide an insight into what was >> your thinking? >> Do you want us to create a separate library and call it >> as PeCoffExtraActionLib to avoid the "Debug" word though it is a hook >> to PeCoffExtraActionLib in MdePkg or do we want to create this library >> in a separate package (may be in MdePkg?) or something totally >> different. > > Supreeth had replied to your comments on the patch. Can you please > check this. If you feel that this needs to be implemented differently, > can you please suggest it to us. Thanks. > My point is that such a fundamental action that needs to occur while loading the PE/COFF image should not be hooked into the loader this way.