|
From: Nicholas N. <nj...@ca...> - 2004-02-09 09:39:45
|
[moving to valgrind-developers]
On Thu, 5 Feb 2004, Jeremy Fitzhardinge wrote:
> > Jeremy, can you take a look at the patch and tell me whether the
> > fd_allowed() call is needed in POST(epoll_create)? Thanks.
>
> fd_allowed is to check FDs as they're passed into syscalls; it shouldn't
> be necessary for FDs created by a syscall (ie, they should always be in
> a PRE(x)).
Are you sure?
AFAICT, fd_allowed() is used in the PRE() for all syscalls that take an fd
as an argument:
readv, writev, close, dup2, read, write
It is also used in the POST() for syscalls that produce a new fd:
dup, open, creat, pipe, socketcall
Generally, the fd opened is checked, and if it's not one allowed by
Valgrind, we close it again and return VKI_EMFILE.
However, the following syscalls produce an fd but do not have the
fd_allowed check:
dup2, fcntl(dup), fcntl64(dup), socketcall, futex
POST(socketcall) even has the following comment:
/* XXX TODO: check return fd against VG_(max_fd) */
So it seems these last ones should be checked, as should
POST(epoll_create)?
N
|
|
From: Jeremy F. <je...@go...> - 2004-02-10 01:22:16
|
On Mon, 2004-02-09 at 01:39, Nicholas Nethercote wrote: > Are you sure? > > AFAICT, fd_allowed() is used in the PRE() for all syscalls that take an fd > as an argument: > > readv, writev, close, dup2, read, write > > It is also used in the POST() for syscalls that produce a new fd: > > dup, open, creat, pipe, socketcall > > Generally, the fd opened is checked, and if it's not one allowed by > Valgrind, we close it again and return VKI_EMFILE. > > However, the following syscalls produce an fd but do not have the > fd_allowed check: > > dup2, fcntl(dup), fcntl64(dup), socketcall, futex > > POST(socketcall) even has the following comment: > > /* XXX TODO: check return fd against VG_(max_fd) */ > > So it seems these last ones should be checked, as should > POST(epoll_create)? You're right. The check in the POST() functions is to make sure that the kernel didn't allocate a client FD in Valgrind's reserved range. Some syscalls, like dup2, allow the client to ask for any FD they want, and others will just return the next available one, which may be in Valgrind's range. In these cases we should close the FD and return ENFILE (or maybe EMFILE). epoll should be the same. J |
|
From: Nicholas N. <nj...@ca...> - 2004-02-14 15:23:08
Attachments:
fd.patch
|
On Mon, 9 Feb 2004, Jeremy Fitzhardinge wrote: > The check in the POST() functions is to make sure that > the kernel didn't allocate a client FD in Valgrind's reserved range. > Some syscalls, like dup2, allow the client to ask for any FD they want, > and others will just return the next available one, which may be in > Valgrind's range. In these cases we should close the FD and return > ENFILE (or maybe EMFILE). epoll should be the same. Attached patch does this, adding POST() checking for fcntl(), fcntl64(), socketcall.socketpair(), and futex(). Two things I'm not sure about: 1. There's a record_fd_open() in vg_syscalls.c:check_cmsg_for_fds(). I don't know if this needs to be checked; the man page says that recvmsg() can be used to pass fds over sockets... and EMFILE (or ENFILE) aren't among the error values that recvmsg() can return. 2. The man pages for futex() also don't list EMFILE (or ENFILE) as one of the allowed error values. Can someone else take a look at this? Thanks. N |
|
From: Jeremy F. <je...@go...> - 2004-02-16 07:06:19
|
On Sat, 2004-02-14 at 07:20, Nicholas Nethercote wrote: > Two things I'm not sure about: > > 1. There's a record_fd_open() in vg_syscalls.c:check_cmsg_for_fds(). I > don't know if this needs to be checked; the man page says that recvmsg() > can be used to pass fds over sockets... and EMFILE (or ENFILE) aren't > among the error values that recvmsg() can return. > > 2. The man pages for futex() also don't list EMFILE (or ENFILE) as one of > the allowed error values. > > Can someone else take a look at this? Thanks. I think these are both documentation oversights. The kernel will definitely return ENFILE from futex(); I'm not sure about recvmsg; it might return EBADF (which would be confusing). J |