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 fchown04 issue #49

Merged
merged 3 commits into from
Aug 13, 2020
Merged

Conversation

shaikshavali1
Copy link

@shaikshavali1 shaikshavali1 commented Aug 5, 2020

The original test case performs the below tests:

  1. fchown(2) returns -1 and sets errno to EPERM if the effective
    user id of the process does not match the owner of the file
    and the process is not a superuser.
  2. fchown(2) returns -1 and sets errno to EBADF if the file
    descriptor of the specified file is not valid.
  3. fchown(2) returns -1 and sets errno to EROFS if the named file
    resides on a read-only file system.

Problem/issue:
This test case causing oom killer to be invoked and causing
kernel panic. This is because the test case invokes test framework
function "tst_acquire_device" which creates a 256MB of .img file
and binds with a loop device. This is not supported because the
default size of LKL memory is set to 32M. This is kept low due to
the limited size of EPC (Enclave page cache) and to avoid page
cache being swapped out.
Also, the test case invokes "tst_mkfs" framework function which
inturn invokes mkfs utility command with the help of system() syscall.
It is recommended to use root file system, but, this test case
needs a read-only filesystem to test one of the sub-test case.
we don't have a read-only file system mounted in sgx-lkl.
Hence, disabling the code related to the mount and sub test case.

Modifications:

  1. Disable the 3rd subtest case
  2. Disable the code which creates file system, mount, and unmount the filesystem.
    Review-1:
  3. Added the code to create a loop device and mount. This need some changes to lsds/sgx-lkl repo, new PR is raised LTP: Pass ext4 block device to enclave sgx-lkl#764
  4. Enabled the 3rd sub test case.

@davidchisnall
Copy link

I still believe that the correct way of fixing tests like this is to add either partitions or separate block devices to our test environment that provide the required file systems. We could easily add a separate 256MB block device for tests like this and do the mkfs on the outside.

@SeanTAllen
Copy link

This needs a corresponding PR on sgx-lkl so that CI is run with this change. Please speak to @hukoyu if you aren't familiar with how to do that.

@shaikshavali1
Copy link
Author

I still believe that the correct way of fixing tests like this is to add either partitions or separate block devices to our test environment that provide the required file systems. We could easily add a separate 256MB block device for tests like this and do the mkfs on the outside.

@davidchisnall, Is this below approach is fine.

  1. create a disk image file in sgxlkl-miniroot-fs.img.master
    dd if=/dev/zero of=testfs_ext4.img bs=1M count=265
  2. format this to ext4
    mkfs -t ext4 testfs_ext4.img
  3. Update the Framework/test case to pick this testfs_ext4.img and bind it with any available loop device.
  4. mount this loop device in the test case and perform the test.

@hukoyu hukoyu requested a review from prp August 6, 2020 20:34
shaikshavali1 added a commit to shaikshavali1/sgx-lkl that referenced this pull request Aug 7, 2020
Problem:
This test case causing oom killer to be invoked and causing
kernel panic. This is because the test case invokes test framework
function "tst_acquire_device" which creates a 256MB of .img file
and binds with a loop device. This is not supported because the
default size of LKL memory is set to 32M. This is kept low due to
the limited size of EPC (Enclave page cache) and to avoid page
cache being swapped out.
Also, the test case invokes "tst_mkfs" framework function which
inturn invokes mkfs utility command with the help of system() syscall.
It is recommended to use root file system, but, this test case
needs a read-only filesystem to test one of the sub-test case.
we don't have a read-only file system mounted in sgx-lkl.

To support the a formated ext4 filesystem image created in master
.img. Test case will pick this image and use it in test.
Test case changes link: lsds/ltp#49
@prp
Copy link
Member

prp commented Aug 7, 2020

Why do you want to use a loop device here? Can't you mount a separate block device using SGX-LKL, similar to what we do in some of our other CI tests?

shaikshavali1 added a commit to shaikshavali1/sgx-lkl that referenced this pull request Aug 8, 2020
This test case causing oom killer to be invoked and causing
kernel panic. This is because the test case invokes test framework
function "tst_acquire_device" which creates a 256MB of .img file
and binds with a loop device. This is not supported because the
default size of LKL memory is set to 32M. This is kept low due to
the limited size of EPC (Enclave page cache) and to avoid page
cache being swapped out.
Also, the test case invokes "tst_mkfs" framework function which
inturn invokes mkfs utility command with the help of system() syscall.
It is recommended to use root file system, but, this test case
needs a read-only filesystem to test one of the sub-test cases.
we don't have a read-only file system mounted in sgx-lkl.

To support the test cases which need a block device a formatted
ext4 filesystem image created and passed
to enclave as extended disk. The test case passes the details about this
new block disk image in the application configuration file. The test framework
will pick this image details in the environment variable and use it in test.
Test case changes link: lsds/ltp#49, lsds/ltp#59.

This PR also revert the PR:lsds#741
This lsds#741, creates a filesystem image in master image.
because of this, each individual test case needs to be modified.
The original test case performs the below tests:
1) fchown(2) returns -1 and sets errno to EPERM if the effective
   user id of the process does not match the owner of the file
   and the process is not a superuser.
2) fchown(2) returns -1 and sets errno to EBADF if the file
   descriptor of the specified file is not valid.
3) fchown(2) returns -1 and sets errno to EROFS if the named file
   resides on a read-only file system.

Problem/issue:
This test case causing oom killer to be invoked and causing
kernel panic. This is because the test case invokes test framework
function "tst_acquire_device" which creates a 256MB of .img file
and binds with a loop device.  This is not supported because the
default size of LKL memory is set to 32M. This is kept low due to
the limited size of EPC (Enclave page cache) and to avoid page
cache being swapped out.
Also, the test case invokes "tst_mkfs" framework function which
inturn invokes mkfs utility command with the help of system() syscall.
It is recommended to use root file system, but, this test case
needs a read-only filesystem to test one of the sub-test case.
we don't have a read-only file system mounted in sgx-lkl.
Hence, disabling the code related to the mount and sub test case.

Modifications:
 1. Disable the 3rd sub test case
 2. Disable the code which creates file system, mount, and unmount
    the filesystem.
1. add code to bind the loop device with ext4 formated file system image
2. mounted the loop device at the mount point.
Updated the test case to use the mounted block device.
@shaikshavali1
Copy link
Author

@davidchisnall, @prp, I updated the changes as per your suggestion. In this PR and PR: lsds/sgx-lkl#750

shaikshavali1 added a commit to shaikshavali1/sgx-lkl that referenced this pull request Aug 11, 2020
This test case causing oom killer to be invoked and causing
kernel panic. This is because the test case invokes test framework
function "tst_acquire_device" which creates a 256MB of .img file
and binds with a loop device. This is not supported because the
default size of LKL memory is set to 32M. This is kept low due to
the limited size of EPC (Enclave page cache) and to avoid page
cache being swapped out.
Also, the test case invokes "tst_mkfs" framework function which
inturn invokes mkfs utility command with the help of system() syscall.
It is recommended to use root file system, but, this test case
needs a read-only filesystem to test one of the sub-test cases.
we don't have a read-only file system mounted in sgx-lkl.

To support the test cases which need a block device a formatted
ext4 filesystem image created and passed
to enclave as extended disk. The test case passes the details about this
new block disk image in the application configuration file. The test framework
will pick this image details in the environment variable and use it in test.
Test case changes link: lsds/ltp#49, lsds/ltp#59.

This PR also revert the PR:lsds#741
This lsds#741, creates a filesystem image in master image.
because of this, each individual test case needs to be modified.
shaikshavali1 added a commit to shaikshavali1/sgx-lkl that referenced this pull request Aug 11, 2020
This test case causing oom killer to be invoked and causing
kernel panic. This is because the test case invokes test framework
function "tst_acquire_device" which creates a 256MB of .img file
and binds with a loop device. This is not supported because the
default size of LKL memory is set to 32M. This is kept low due to
the limited size of EPC (Enclave page cache) and to avoid page
cache being swapped out.
Also, the test case invokes "tst_mkfs" framework function which
inturn invokes mkfs utility command with the help of system() syscall.
It is recommended to use root file system, but, this test case
needs a read-only filesystem to test one of the sub-test cases.
we don't have a read-only file system mounted in sgx-lkl.

To support the test cases which need a block device a formatted
ext4 filesystem image created and passed
to enclave as extended disk. The test case passes the details about this
new block disk image in the application configuration file. The test framework
will pick this image details in the environment variable and use it in test.
Test case changes link: lsds/ltp#49, lsds/ltp#59, lsds/ltp#65,
lsds/ltp#66.
Copy link
Collaborator

@hukoyu hukoyu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. After lsds/sgx-lkl#764 and #65 are merged this fix will work.

@hukoyu
Copy link
Collaborator

hukoyu commented Aug 12, 2020

LGTM. After lsds/sgx-lkl#764 and #65 are merged this fix will work.

@davidchisnall both of dependent PRs merged. This PR is ready to be merged if you approve.

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.

5 participants