open-power / skiboot

OPAL boot and runtime firmware for POWER
Apache License 2.0
100 stars 134 forks source link

Properly check mmap error code #252

Closed hannob closed 4 years ago

hannob commented 4 years ago

Given that subscribing to you rmailing list does not work I'll send the pull request anyway, despite the warning that I shouldn't.

The check here in the code is wrong. mmap never returns zero, on failure it returns MAP_FAILED (or -1).

There are 2 more cases in the code, however it's less clear to me how to fix them. In libstb/create-container.c there is an mmap call that doesn't seem to check the return value at all.

In extract-gcov.c there's this:

    addr = mmap(NULL, sb.st_size, PROT_READ, MAP_PRIVATE, fd, 0);
    assert(addr != NULL);

This is doubly wrong, as asserts shouldn't be used for error checking.

oohal commented 4 years ago

Given that subscribing to you rmailing list does not work I'll send the pull request anyway, despite the warning that I shouldn't.

Weird. I'll ask our sysadmin to have a look.

You can post directly to the list without being subscribed. The message will go into a moderation queue at first, but I always whitelist people sending actual patches.

The check here in the code is wrong. mmap never returns zero, on failure it returns MAP_FAILED (or -1).

There are 2 more cases in the code, however it's less clear to me how to fix them. In libstb/create-container.c there is an mmap call that doesn't seem to check the return value at all.

In extract-gcov.c there's this:

  addr = mmap(NULL, sb.st_size, PROT_READ, MAP_PRIVATE, fd, 0);
  assert(addr != NULL);

This is doubly wrong, as asserts shouldn't be used for error checking.

Suppose. extract-gcov and print-container are random helper applications which is probably why no one bothered fixing them. Thanks for looking at this stuff.

oohal commented 4 years ago

I folded this into #256 with a fixed up DCO.

e: Our sysadmin looked at our email setup and didn't find any problems. A sub/unsub test worked fine too. Is there any other details you can give us about what didn't work?