Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

post-process-pe: add tests to validate NX compliance #705

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

post-process-pe: add tests to validate NX compliance #705

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
46524b6
CI: use checkout@v4
vathpela Aug 2, 2024
d850dff
CI: work around ownership issue on github
vathpela Aug 2, 2024
97fd3a0
Update fedora CI targets
vathpela Dec 12, 2024
98804ee
post-process-pe: add tests to validate NX compliance
dennis-tseng99 Apr 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 25 additions & 25 deletions .github/workflows/pullrequest.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,66 +22,70 @@ jobs:
efiarch: aa64
gccarch: aarch64
makearch: aarch64
distro: f36
distro: f41
- arch: amd64
efiarch: aa64
gccarch: aarch64
makearch: aarch64
distro: f35
distro: f40
- arch: amd64
efiarch: arm
gccarch: arm
makearch: arm
distro: f36
distro: f41
- arch: amd64
efiarch: arm
gccarch: arm
makearch: arm
distro: f35
distro: f40
- arch: amd64
efiarch: arm
gccarch: arm
makearch: arm
distro: f34
distro: f39
- arch: amd64
efiarch: x64
gccarch: x86_64
makearch: x86_64
distro: f36
distro: f41
- arch: amd64
efiarch: x64
gccarch: x86_64
makearch: x86_64
distro: f35
distro: f40
- arch: amd64
efiarch: x64
gccarch: x86_64
makearch: x86_64
distro: f34
distro: f39
- arch: amd64
efiarch: ia32
gccarch: x86_64
makearch: ia32
distro: f36
distro: f41
- arch: amd64
efiarch: ia32
gccarch: x86_64
makearch: ia32
distro: f35
distro: f40
- arch: amd64
efiarch: ia32
gccarch: x86_64
makearch: ia32
distro: f34
distro: f39

steps:
- name: Checkout
uses: actions/checkout@v2
uses: actions/checkout@v4
with:
# otherwise we are testing target branch instead of the PR branch (see pull_request_target trigger)
ref: ${{ github.event.pull_request.head.sha }}
fetch-depth: 0
submodules: recursive
- name: Work around directory ownership issue
id: ignore-ownership
run: |
git config --global --add safe.directory /__w/shim/shim
- name: Update submodules on ${{ matrix.distro }} for ${{ matrix.efiarch }}
id: update-submodules
run: |
Expand Down Expand Up @@ -118,15 +122,15 @@ jobs:
- arch: amd64
efiarch: x64
makearch: x86_64
distro: f36
distro: f41
- arch: amd64
efiarch: x64
makearch: x86_64
distro: f35
distro: f40
- arch: amd64
efiarch: x64
makearch: x86_64
distro: f34
distro: f39
- arch: amd64
efiarch: x64
makearch: x86_64
Expand All @@ -138,28 +142,24 @@ jobs:
- arch: amd64
efiarch: ia32
makearch: ia32
distro: f36
- arch: amd64
efiarch: ia32
makearch: ia32
distro: f35
- arch: amd64
efiarch: ia32
makearch: ia32
distro: f34
distro: f39
- arch: amd64
efiarch: ia32
makearch: ia32
distro: centos8

steps:
- name: Checkout
uses: actions/checkout@v2
uses: actions/checkout@v4
with:
# otherwise we are testing target branch instead of the PR branch (see pull_request_target trigger)
ref: ${{ github.event.pull_request.head.sha }}
fetch-depth: 0
submodules: recursive
- name: Work around directory ownership issue
id: ignore-ownership
run: |
git config --global --add safe.directory /__w/shim/shim
- name: Update submodules on ${{ matrix.distro }} for ${{ matrix.efiarch }}
id: update-submodules
run: |
Expand Down
58 changes: 57 additions & 1 deletion post-process-pe.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ static int verbosity;
})

static bool set_nx_compat = false;
static bool require_nx_compat = false;

typedef uint8_t UINT8;
typedef uint16_t UINT16;
Expand Down Expand Up @@ -360,6 +361,50 @@ set_dll_characteristics(PE_COFF_LOADER_IMAGE_CONTEXT *ctx)
}
}

static int
validate_nx_compat(PE_COFF_LOADER_IMAGE_CONTEXT *ctx)
{
EFI_IMAGE_SECTION_HEADER *Section;
int i;
bool nx_compat_flag;
const int level = require_nx_compat ? ERROR : WARNING;
int ret = 0;

nx_compat_flag = EFI_IMAGE_DLLCHARACTERISTICS_NX_COMPAT
& ctx->PEHdr->Pe32.OptionalHeader.DllCharacteristics;
debug(NOISE, "NX-Compat-Flag: %s\n", nx_compat_flag ? "set" : "unset");
if (!nx_compat_flag) {
debug(level, "NX Compatibility flag is not set\n");
if (require_nx_compat)
ret = -1;
}

debug(NOISE, "Section alignment is 0x%x, page size is 0x%x\n",
ctx->PEHdr->Pe32Plus.OptionalHeader.SectionAlignment,
PAGE_SIZE);
if (ctx->PEHdr->Pe32Plus.OptionalHeader.SectionAlignment != PAGE_SIZE) {
debug(level, "Section alignment is not page aligned\n");
if (require_nx_compat)
ret = -1;
}

Section = ctx->FirstSection;
for (i=0, Section = ctx->FirstSection; i < ctx->NumberOfSections; i++, Section++) {
debug(NOISE, "Section %d has WRITE=%d and EXECUTE=%d\n", i,
(Section->Characteristics & EFI_IMAGE_SCN_MEM_WRITE) ? 1 : 0,
(Section->Characteristics & EFI_IMAGE_SCN_MEM_EXECUTE) ? 1 : 0);

if ((Section->Characteristics & EFI_IMAGE_SCN_MEM_WRITE) &&
(Section->Characteristics & EFI_IMAGE_SCN_MEM_EXECUTE)) {
debug(level, "Section %d is writable and executable\n", i);
if (require_nx_compat)
ret = -1;
}
}

return ret;
}

static void
fix_timestamp(PE_COFF_LOADER_IMAGE_CONTEXT *ctx)
{
Expand Down Expand Up @@ -449,6 +494,10 @@ handle_one(char *f)

set_dll_characteristics(&ctx);

rc = validate_nx_compat(&ctx);
if (rc < 0)
err(2, "NX compatibility check failed\n");

fix_timestamp(&ctx);

fix_checksum(&ctx, map, sz);
Expand Down Expand Up @@ -483,6 +532,7 @@ static void __attribute__((__noreturn__)) usage(int status)
fprintf(out, " -v Be more verbose\n");
fprintf(out, " -N Disable the NX compatibility flag\n");
fprintf(out, " -n Enable the NX compatibility flag\n");
fprintf(out, " -x Error on NX compatibility\n");
fprintf(out, " -h Print this help text and exit\n");

exit(status);
Expand Down Expand Up @@ -510,11 +560,14 @@ int main(int argc, char **argv)
{.name = "verbose",
.val = 'v',
},
{.name = "error-nx-compat",
.val = 'x',
},
{.name = ""}
};
int longindex = -1;

while ((i = getopt_long(argc, argv, "hNnqv", options, &longindex)) != -1) {
while ((i = getopt_long(argc, argv, "hNnqvx", options, &longindex)) != -1) {
switch (i) {
case 'h':
case '?':
Expand All @@ -532,6 +585,9 @@ int main(int argc, char **argv)
case 'v':
verbosity = MIN(verbosity + 1, MAX_VERBOSITY);
break;
case 'x':
require_nx_compat = true;
break;
}
}

Expand Down