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

fork08 is failing with upgraded LTP suite #254

Open
aniket-intelx opened this issue Nov 30, 2021 · 2 comments
Open

fork08 is failing with upgraded LTP suite #254

aniket-intelx opened this issue Nov 30, 2021 · 2 comments
Labels
enhancement New feature or request P: 2

Comments

@aniket-intelx
Copy link
Contributor

Description of the problem

The fork08 testcase checks that the parent's file descriptors will not be affected by being closed in the child.

Steps to reproduce

Since the issues are seen with the LTP upgraded tests and something that we are still migrating to the local CI, follow the below steps to checkout and run the latest LTP test in your local workspace.

Checkout Gramine with commit ID e0105d39fe6f3d36d6ba4c46377435f6d4f650b7 which is [Docs] Fix documentation for fs.root, as the local CI is not yet updated with gramine-test for LTP suite.

Checkout the local CI repo present here https://github.com/jinengandhi-intel/graphene_local_ci/tree/ltp_upgrade

cd gramine/LibOS/shim/test/ltp
cp -rf <ltp_upgrade_localci_branch>/ltp_src .
cp -rf <ltp_upgrade_localci_branch>ltp_config/* .

# To build the tests and generate manifest files
make -j8 -f Makefile.LTP all LTPCFG=ltp_tests.cfg LTPTESTFILE=./install/runtest/syscalls-new 

# To run the tests
cd install/testcases/bin/
gramine-direct fork08

Expected results

TPASS: read the end of file correctly

Actual results

gramine-direct fork08
[P1:T1:] error: Mounting file:/proc may expose unsanitized, unsafe files to unsuspecting application. Gramine will continue application execution, but this configuration is not recommended for use in production!
/home/intel/graphene_newltp/LibOS/shim/test/ltp/ltp_src/lib/tst_test.c:1362: TINFO: Timeout per run is 0h 05m 00s
/home/intel/graphene_newltp/LibOS/shim/test/ltp/ltp_src/testcases/kernel/syscalls/fork/fork08.c:48: TFAIL: read() returns 1, expected 0: ECHILD (10)

The trace log is attached below.

fork08_ltp_upgrade_log.txt

@dimakuv
Copy link

dimakuv commented Dec 1, 2021

This case is interesting. It is currently not supported in Gramine.

Some analysis follows.

Correct Linux behavior

This fork08 test looks like this: https://github.com/linux-test-project/ltp/blob/96daba2729112b83bb0a2438023c94f2645ae20f/testcases/kernel/syscalls/fork/fork08.c

Basically, the parent opens a testfile_fork08 file and then forks a child. So now the parent and the child processes refer to the same open file description (through two different file descriptors, parent's fd and child's fd). Then the child reads 1 byte from the file -- this forces the Linux kernel to move the file offset to 1. Then the parent tries to read 1 byte, but since the file description is shared between both processes, the parent tries to read from file offset 1, and so the parent gets End-of-File.

See also this explanation: https://stackoverflow.com/questions/36022953/if-a-parent-opens-a-file-then-forks-a-child-does-the-child-have-access-to-the.

Also the man page for fork has this very important passage:

  • The child inherits copies of the parent's set of open file
    descriptors. Each file descriptor in the child refers to the same
    open file description (see open(2)) as the corresponding file
    descriptor in the parent. This means that the two file
    descriptors share open file status flags, file offset, and signal-
    driven I/O attributes (see the description of F_SETOWN and
    F_SETSIG in fcntl(2)).

Incorrect Gramine behavior

Gramine creates two Gramine processes: one parent and one child. Each of them has its own copy of file metadata, including the file offset. In other words, Gramine currently does not share the file offset. So the Gramine-parent doesn't notice that the Gramine-child changed the file offset. Thus, read() in the parent actually reads 1 byte from the file. This is incorrect behavior.

@pwmarcz You were planning to implement the sharing of file metadata, right?

@pwmarcz
Copy link
Contributor

pwmarcz commented Dec 1, 2021

We currently do not sychronize file offsets, and this is generally a hard problem to do practically (I prototyped a "sync engine", and it turned out to be way too complicated). So, probably not coming anytime soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request P: 2
Projects
None yet
Development

No branches or pull requests

3 participants