From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-it0-x22d.google.com (mail-it0-x22d.google.com [IPv6:2607:f8b0:4001:c0b::22d]) (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 C1C8280431 for ; Fri, 17 Mar 2017 06:23:24 -0700 (PDT) Received: by mail-it0-x22d.google.com with SMTP id g138so25350830itb.0 for ; Fri, 17 Mar 2017 06:23:24 -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=nRcrseFZhPLFRpEu0jy9wMbij2xFdhB37ZpHcNtxgt4=; b=TEa1qFsj7q1RpbWEDTnJYYwN67slwUJrGaWHXfIYeybkz1EBaBMaUvMa0KmnZsYJT2 hEDxADzS78qnPbRsNsV1x/N+kTzTCs5F9YdN2ohfnk62h5kBrJuBq0BR7eeVmDCnNo88 oXcvqwSbYdCcP+HYxPYeZ9Ip1x9EjDSWXyZ30= 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=nRcrseFZhPLFRpEu0jy9wMbij2xFdhB37ZpHcNtxgt4=; b=BoR+C7YwiVq67DWZN5Fhog+81pBDE1GsKKLeMR9KZEUXkB2lSkmzmXMoDoQo0z4pPa P80beQFO/We4Utf6NEXp54LXeXbDYfUME+LajIRh++5oBByP9NxX6EqUsfQKiCxSQus5 PuawrDp/TF5HV5G7lN1s+Qt/mWrFfzgPcjvHNCbUguW5LgZMUtjKJ+qBlMy73rqMeU7m 0acf0lRwcQhK0WC1UIdtr8/g4vvy73iRUBpAfEfTL3lNcP9arLghqKck8DxQKmF7tjDE 12sT3KdNlg0e/yMSj9FsJZ4Hhp/7O55gcA2l9DSa4nhdnr9wPUEfd0uKVxiIhUVxYs2a aBrg== X-Gm-Message-State: AFeK/H3pv03uOt1Hx3JT7Q20dURBT7vXzN1Ryap2EGd/DrMFkSFhdIaS7e0EY9BFnXnZc1dnISSk1q43R5w9Agsm X-Received: by 10.36.77.10 with SMTP id l10mr2577338itb.59.1489757002644; Fri, 17 Mar 2017 06:23:22 -0700 (PDT) MIME-Version: 1.0 Received: by 10.107.10.27 with HTTP; Fri, 17 Mar 2017 06:23:22 -0700 (PDT) In-Reply-To: <48f431ec-483e-4896-3fca-c5fedcdc1029@redhat.com> References: <1487958664-10707-1-git-send-email-ard.biesheuvel@linaro.org> <74D8A39837DF1E4DA445A8C0B3885C503A8F5385@shsmsx102.ccr.corp.intel.com> <262ebc4d-629c-3437-2146-f4c0e5723f1a@redhat.com> <48f431ec-483e-4896-3fca-c5fedcdc1029@redhat.com> From: Ard Biesheuvel Date: Fri, 17 Mar 2017 13:23:22 +0000 Message-ID: To: Laszlo Ersek Cc: "Yao, Jiewen" , "edk2-devel@lists.01.org" , "Gao, Liming" Subject: Re: [PATCH] MdeModulePkg/DxeCore: base code protection on permission attributes X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 17 Mar 2017 13:23:25 -0000 Content-Type: text/plain; charset=UTF-8 On 17 March 2017 at 12:35, Laszlo Ersek wrote: > On 03/17/17 13:11, Ard Biesheuvel wrote: >> On 17 March 2017 at 12:07, Laszlo Ersek wrote: >>> On 02/26/17 15:00, Ard Biesheuvel wrote: >>>> On 25 February 2017 at 04:04, Yao, Jiewen wrote: >>>>> Hi Ard >>>>> I agree with you on this enhancement. >>>>> >>>>> I prefer to adding the description as comment in the code, so that people >>>>> can get clear picture when he/she reads the code. >>>>> >>>>> // >>>>> // Instead of assuming that a PE/COFF section of type EFI_IMAGE_SCN_CNT_CODE >>>>> // can always be mapped read-only, classify a section as a code section only >>>>> // if it has the executable attribute set and the writable attribute >>>>> cleared. >>>>> // >>>>> // This adheres more closely to the PE/COFF spec, and avoids issues with >>>>> // Linux OS loaders that consists of a single read/write/execute section. >>>>> // >>>>> if ((Section[Index].Characteristics & (EFI_IMAGE_SCN_MEM_WRITE | >>>>> EFI_IMAGE_SCN_MEM_EXECUTE)) == EFI_IMAGE_SCN_MEM_EXECUTE) { >>>>> >>>>> With comment update, reviewed-by: Jiewen.yao@intel.com >>>>> >>>> >>>> Thanks Jiewen >>>> >>>> Pushed as a2ed40c02bf2 >>> >>> Is it possible that (recent?) Linux EFI stubs (aarch64) don't pass the >>> above check? I got a report from a colleague: >>> >>> !!!!!!!! ProtectUefiImageCommon - CodeSegmentCount is 0 !!!!!!!! >>> EFI stub: Booting Linux Kernel... >>> EFI stub: Using DTB from configuration table >>> EFI stub: Exiting boot services and installing virtual address map... >>> >>> I tried to reproduce it with "4.5.0-15.el7.aarch64", and with >>> "4.8.7-300.fc25.aarch64", and I'm not seeing the message with either. >>> >>> I asked him about the exact kernel version (no answer yet, his workday >>> hasn't started yet). >>> >> >> Well, the warning may be a bit loud, but this is expected behavior. I >> expect to be able to queue the linux/arm64 changes that split the >> PE/COFF sections and align them to 4 KB for v4.12, and so current >> kernels all consists of a single rwx section, which does not qualify >> as a code section. > > Okay, thanks. > > In that case, does it make sense to downgrade the messages from DEBUG_ERROR to DEBUG_WARN? > > if (ImageRecord->CodeSegmentCount == 0) { > DEBUG ((DEBUG_ERROR, "!!!!!!!! ProtectUefiImageCommon - CodeSegmentCount is 0 !!!!!!!!\n")); > PdbPointer = PeCoffLoaderGetPdbPointer ((VOID*) (UINTN) ImageAddress); > if (PdbPointer != NULL) { > DEBUG ((DEBUG_ERROR, "!!!!!!!! Image - %a !!!!!!!!\n", PdbPointer)); > } > goto Finish; > } > Yes, that sounds reasonable.