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

lib/9pfs: Stub uk_9pfs_ioctl #928

Closed

Conversation

razvand
Copy link
Contributor

@razvand razvand commented Jun 4, 2023

Change uk_9pfs_ioctl from vfscore_vop_einval to vfscore_vop_nullop, effectively stubbing it. This means a call to uk_9pfs_ioctl will no longer return -EINVAL, but it will return 0, tricking the caller into thinking ioctl() functionality is available.

This is useful in binary-compatibility mode, where certain ioctl() calls may cause the application to end, if an error code (such as -EINVAL is returned). This is the case of Ruby interpreter running in binary-compatibility mode.

Prerequisite checklist

  • Read the contribution guidelines regarding submitting new changes to the project;
  • Tested your changes against relevant architectures and platforms;
  • Ran the checkpatch.uk on your commit series before opening this PR;
  • Updated relevant documentation.

Base target

  • Architecture(s): [e.g. x86_64]
  • Platform(s): [e.g. kvm]
  • Application(s): [e.g. app-elfloader, Ruby in binary-compatibility mode]

Change `uk_9pfs_ioctl` from `vfscore_vop_einval` to
`vfscore_vop_nullop`, effectively stubbing it. This means a call to
`uk_9pfs_ioctl` will no longer return `-EINVAL`, but it will return
`0`, tricking the caller into thinking `ioctl()` functionality is
available.

This is useful in binary-compatibility mode, where certain `ioctl()`
calls may cause the application to end, if an error code (such as
`-EINVAL` is returned). This is the case of Ruby interpreter running in
binary-compatibility mode.

Signed-off-by: Razvan Deaconescu <razvand@unikraft.io>
@razvand razvand added area/lib Internal Unikraft Microlibrary kind/quick-fix Issue is a quick fix lib/vfscore VFS Core Interface lib/9pfs 9p client lang/c Issues or PRs to do with C/C++ topic/syscall Related to syscalls labels Jun 4, 2023
@razvand razvand added this to the v0.14.0 (Prometheus) milestone Jun 4, 2023
@razvand razvand requested a review from a team as a code owner June 4, 2023 15:12
@razvand razvand removed the request for review from a team June 4, 2023 15:12
@unikraft-bot
Copy link
Member

Checkpatch passed

Beep boop! I ran Unikraft's checkpatch.pl support script on your pull request and it all looks good!

SHA commit checkpatch
38d7703 lib/9pfs: Stub uk_9pfs_ioctl

Copy link
Member

@StefanJum StefanJum left a comment

Choose a reason for hiding this comment

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

All good, thanks.
Reviewed-by: Stefan Jumarea stefanjumarea02@gmail.com

Copy link
Member

@marcrittinghaus marcrittinghaus left a comment

Choose a reason for hiding this comment

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

@razvand This could also create problems if the application depends on the correct behavior of ioctl and we stub the call. I assume for Ruby this is not the case. By any chance, do you remember what ioctl ruby attempts to perform?

@StefanJum
Copy link
Member

@razvand This could also create problems if the application depends on the correct behavior of ioctl and we stub the call. I assume for Ruby this is not the case. By any chance, do you remember what ioctl ruby attempts to perform?

@marcrittinghaus it calls fcntl(0x7, F_SETFL, ...), which then calls vfs_ioctl(fp, FIOASYNC...) (which then calls VOP_IOCTL, which is vfscore_vop_einval). So yes, it does try to set O_ASYNC, which we do not support.

@marcrittinghaus
Copy link
Member

Ok, does Ruby work at all then? For O_ASYNC we need signals.

@StefanJum
Copy link
Member

Ok, does Ruby work at all then? For O_ASYNC we need signals.

Yes, helloworld and other simple stuff works, haven't tried anything else more complex.

@razvand
Copy link
Contributor Author

razvand commented Jun 19, 2023

@razvand This could also create problems if the application depends on the correct behavior of ioctl and we stub the call. I assume for Ruby this is not the case. By any chance, do you remember what ioctl ruby attempts to perform?

Indeed, the proper solution would be to have a full implementation of ioctl. One way would to stub it only for the command used by Ruby. The issue is actually part of the fcntl() call:

openat(AT_FDCWD, "/usr/lib/ruby/2.5.0/rubygems.rb", O_RDONLY|O_CLOEXEC|O_NONBLOCK) = fd:7
fcntl(0x7, 0x4, ...) = Invalid argument (-22)
close(fd:7) = OK
ioctl(0x2, 0x5401, ...) = 0x0

The value 0x4 is the F_SETFL command for fcntl.

Copy link
Member

@marcrittinghaus marcrittinghaus left a comment

Choose a reason for hiding this comment

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

openat(AT_FDCWD, "/usr/lib/ruby/2.5.0/rubygems.rb", O_RDONLY|O_CLOEXEC|O_NONBLOCK) = fd:7
fcntl(0x7, 0x4, ...) = Invalid argument (-22)
close(fd:7) = OK
ioctl(0x2, 0x5401, ...) = 0x0

This is interesting. This is another bug in vfscore. The O_NONBLOCK in the openat call should already cause the problem, because it supplies the O_NONBLOCK flag. However, in contrast to fcntl, we don't call vfs_ioctl on the fd when applying the open flags. So depending on the FS this is already broken.

Regarding this PR. For now, it is fine to use vfscore_vop_nullop.

Approved-by: Marc Rittinghaus marc.rittinghaus@unikraft.io

@unikraft-bot unikraft-bot added the ci/merged Merged by CI label Jun 19, 2023
rares-miculescu pushed a commit to rares-miculescu/unikraft that referenced this pull request Jul 1, 2023
Change `uk_9pfs_ioctl` from `vfscore_vop_einval` to
`vfscore_vop_nullop`, effectively stubbing it. This means a call to
`uk_9pfs_ioctl` will no longer return `-EINVAL`, but it will return
`0`, tricking the caller into thinking `ioctl()` functionality is
available.

This is useful in binary-compatibility mode, where certain `ioctl()`
calls may cause the application to end, if an error code (such as
`-EINVAL` is returned). This is the case of Ruby interpreter running in
binary-compatibility mode.

Signed-off-by: Razvan Deaconescu <razvand@unikraft.io>
Reviewed-by: Stefan Jumarea <stefanjumarea02@gmail.com>
Approved-by: Marc Rittinghaus <marc.rittinghaus@unikraft.io>
Tested-by: Unikraft CI <monkey@unikraft.io>
GitHub-Closes: unikraft#928
rares-miculescu pushed a commit to rares-miculescu/unikraft that referenced this pull request Jul 5, 2023
Change `uk_9pfs_ioctl` from `vfscore_vop_einval` to
`vfscore_vop_nullop`, effectively stubbing it. This means a call to
`uk_9pfs_ioctl` will no longer return `-EINVAL`, but it will return
`0`, tricking the caller into thinking `ioctl()` functionality is
available.

This is useful in binary-compatibility mode, where certain `ioctl()`
calls may cause the application to end, if an error code (such as
`-EINVAL` is returned). This is the case of Ruby interpreter running in
binary-compatibility mode.

Signed-off-by: Razvan Deaconescu <razvand@unikraft.io>
Reviewed-by: Stefan Jumarea <stefanjumarea02@gmail.com>
Approved-by: Marc Rittinghaus <marc.rittinghaus@unikraft.io>
Tested-by: Unikraft CI <monkey@unikraft.io>
GitHub-Closes: unikraft#928
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/lib Internal Unikraft Microlibrary ci/merged Merged by CI kind/quick-fix Issue is a quick fix lang/c Issues or PRs to do with C/C++ lib/vfscore VFS Core Interface lib/9pfs 9p client topic/syscall Related to syscalls
Projects
Status: Done!
Development

Successfully merging this pull request may close these issues.

None yet

4 participants