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.

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

LTP: fix test case writev06 issue #58

Closed
wants to merge 3 commits into from

Conversation

shaikshavali1
Copy link

Below issues are found in this test case:

  1. mmap is failing with EBADF error if we pass 0 as file descriptor
    with MAP_ANONYMOUS option.
  2. TPASS message is tagged as INFO message.
    Below modifications are performed.
  3. Modified the test case to pass -1 as a file descriptor value
    while invoking mmap.
  4. Modified the print message TAG to TPASS.

Below issues are found in this test case:
1. mmap is failing with EBADF error if we pass 0 as file descriptor
   with MAP_ANONYMOUS option.
2. TPASS message is tagged as INFO message.
Below modifications are performed.
1. Modified the test case to pass -1 as a file descriptor value
   while invoking mmap.
2. Modified the print message TAG to TPASS.
@davidchisnall
Copy link

I think the third bullet point is a real bug (@SeanTAllen, can you take a look?). In POSIX it is undefined what happens when you MAP_ANONYMOUS with an fd that is not -1, but Linux ignores the fd argument (XNU uses it as a metadata parameter to attach to the mapped region). We are not compatible with Linux if we are treating -1 as special and should fix that in our mmap routine.

@SeanTAllen
Copy link

@davidchisnall here's the code:

    // Anonymous mapping/allocation
    else if (fd == -1 && (flags & MAP_ANONYMOUS))
    {
        return (long)enclave_mmap(addr, length, flags & MAP_FIXED, prot, 1);
    }

So yes, MAP_ANONYMOUS is only supported currently if the fd is -1.

Shall we change to ignore the fd entirely?

@SeanTAllen
Copy link

@davidchisnall assuming a yes, I went ahead and created lsds/sgx-lkl#755

@davidchisnall
Copy link

@shaikshavali1, please revert the corresponding changes in this PR. @hukoyu, once lsds/sgx-lkl#755 is merged, please go ahead and merge the version of this without those changes.

@SeanTAllen
Copy link

lsds/sgx-lkl#755 has been merged.

@hukoyu
Copy link
Collaborator

hukoyu commented Aug 11, 2020

Submitted PR lsds/sgx-lkl#765 to enable this test as is without patch and it is passing. This patch not needed anymore. Closing.

@hukoyu hukoyu closed this Aug 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants