gramineproject / gramine

A library OS for Linux multi-process applications, with Intel SGX support
GNU Lesser General Public License v3.0
598 stars 200 forks source link

fork08 is failing with upgraded LTP suite #254

Open aniket-intelx opened 2 years ago

aniket-intelx commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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.