Skip to content
This repository has been archived by the owner on Jan 20, 2022. It is now read-only.

[LibOS] Fix a crash after reusing dentry touched by mknod #2505

Merged
merged 1 commit into from
Jul 5, 2021

Conversation

pwmarcz
Copy link
Contributor

@pwmarcz pwmarcz commented Jul 5, 2021

Found while investigating #2419. I tried to make a proper fix in PR #2499, but that seems to be blocked by conflict with tmpfs/strfs code. Merging #2499 would probably make an existing memory leak worse, so I would like to take care of tmpfs and strfs first. In the mean time, here is the workaround for the actual crash.

I ticketed the proper fix as #2507 and intend to work on it.

Description of the changes

This fixes a crash where we create a named pipe, remove it, and then try to create a normal file with the same name. Unfortunately, fixing the underlying problem (dentry reuse) is more problematic, so this is only a workaround for that specific case.

How to test this PR?

  • Should fix the mknod crash in Stress-ng test suite causing memory fault and heap corruption failure in Graphene #2419
    • Note that the mknod stress test still fails, because our mknod doesn't create sockets, and (when these failures are ignored) hits EMFILE from host on creating too many named pipes. However, it should not cause Graphene to crash anymore.
  • I added a short snippet to the end of mkfifo that crashes in same manner on master.

This change is Reviewable

dimakuv
dimakuv previously approved these changes Jul 5, 2021
Copy link

@dimakuv dimakuv left a comment

Choose a reason for hiding this comment

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

Reviewed 3 of 3 files at r1.
Reviewable status: all files reviewed, all discussions resolved, not enough approvals from maintainers (1 more required)

mkow
mkow previously approved these changes Jul 5, 2021
Copy link
Member

@mkow mkow left a comment

Choose a reason for hiding this comment

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

Reviewed 3 of 3 files at r1.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

This fixes a crash where we create a named pipe, remove it, and then try
to create a normal file with the same name. Unfortunately, fixing the
underlying problem (dentry reuse) is more problematic, so this is only a
workaround for that specific case.

Signed-off-by: Paweł Marczewski <[email protected]>
@pwmarcz pwmarcz dismissed stale reviews from mkow and dimakuv via 4e00671 July 5, 2021 14:21
@pwmarcz pwmarcz force-pushed the pawel/fix-mknod-1 branch from 835d209 to 4e00671 Compare July 5, 2021 14:21
Copy link
Member

@mkow mkow left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r2.
Reviewable status: all files reviewed, all discussions resolved, not enough approvals from maintainers (1 more required), not enough approvals from different teams (1 more required, approved so far: ITL)

Copy link

@dimakuv dimakuv left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r2.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@mkow mkow merged commit 4e00671 into master Jul 5, 2021
@mkow mkow deleted the pawel/fix-mknod-1 branch July 5, 2021 16:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants