Skip to content
This repository has been archived by the owner on May 4, 2018. It is now read-only.

linux: (try to) handle epoll_ctl returning EINVAL? #1191

Closed
saghul opened this issue Mar 11, 2014 · 5 comments
Closed

linux: (try to) handle epoll_ctl returning EINVAL? #1191

saghul opened this issue Mar 11, 2014 · 5 comments

Comments

@saghul
Copy link
Contributor

saghul commented Mar 11, 2014

This came as a bug report to pyuv, here is the case:

  • user uses socketpair() or whatever and gets a fd
  • uv_pipe_open that fd
  • close the fd
  • uv_run
  • abort()

On Linux, the abort is caused here: https://github.com/joyent/libuv/blob/master/src/unix/linux-core.c#L176-L178

The errno is EINVAL, which according to the man page for epoll_ctl it can be because either the epoll fd or the fd is not a valid file descriptor. Oh the joy!

Now, on one side, one could argue that it's most likely the fd and not the epoll fd. But, what if evil Eve has used uv_backend_fd and closed the epoll fd?

Should we try to handle this? If so, can we assume it's fd the one that failed? Sounds a bit scary but I may be overthinking it.

The pyuv bug: saghul/pyuv#146

/cc @indutny @bnoordhuis

@indutny
Copy link
Contributor

indutny commented Mar 11, 2014

Are you closing other side, or the same fd as was passed to uv_pipe_open?

@saghul
Copy link
Contributor Author

saghul commented Mar 11, 2014

Actually, due to garbage collection both sides of the socketpair are closed. Including the one which was passed to uv_pipe_open, which the one that seems to cause the failure.

@bnoordhuis
Copy link
Contributor

The errno is EINVAL, which according to the man page for epoll_ctl it can be because either the epoll fd or the fd is not a valid file descriptor. Oh the joy!

EINVAL indicates that either the file descriptor is not an epoll file descriptor or that the epoll file descriptor and the target file descriptor point to the same file description.

If the target file descriptor is closed, you get EBADF. If it doesn't support epoll, you get EPERM (rare, but sometimes happens with device drivers.)

Now, on one side, one could argue that it's most likely the fd and not the epoll fd. But, what if evil Eve has used uv_backend_fd and closed the epoll fd?

Then you get EBADF. If you want to know whether that's for the epoll file descriptor or the target file descriptor, you should fstat() them (which isn't race free, of course.)

@saghul
Copy link
Contributor Author

saghul commented Mar 11, 2014

On 03/11/2014 11:22 PM, Ben Noordhuis wrote:

The errno is EINVAL, which according to the man page for epoll_ctl
it can be because either the epoll fd or the fd is not a valid file
descriptor. Oh the joy!

EINVAL indicates that either the file descriptor is not an epoll file
descriptor or that the epoll file descriptor and the target file
descriptor point to the same file description.

If the target file descriptor is closed, you get EBADF. If it doesn't
support epoll, you get EPERM (rare, but sometimes happens with device
drivers.)

Now, on one side, one could argue that it's most likely the fd and
not the epoll fd. But, what if evil Eve has used uv_backend_fd and
closed the epoll fd?

Then you get EBADF. If you want to know whether that's for the epoll
file descriptor or the target file descriptor, you should fstat() them
(which isn't race free, of course.)

Sorry, I said it was EINVAL, but it really was EBADF.

Saúl Ibarra Corretgé
bettercallsaghul.com

@saghul
Copy link
Contributor Author

saghul commented Apr 12, 2014

I don't feel like attempting to fix this. People need to either pass a dup-ed fd or assume we take ownership of the fd and they shouldn't close it. Closing.

@saghul saghul closed this as completed Apr 12, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants