This is a first user of sys_indirect. Several of the socket-related system
calls which produce a file handle now can be passed an additional parameter
to set the FD_CLOEXEC flag.
socket.c | 21 +++++++++++++--------
1 file changed, 13 insertions(+), 8 deletions(-)
--- a/net/socket.c
+++ b/net/socket.c
@@ -344,11 +344,11 @@ static struct dentry_operations sockfs_dentry_operations = {
* but we take care of internal coherence yet.
*/
-static int sock_alloc_fd(struct file **filep)
+static int sock_alloc_fd(struct file **filep, int flags)
{
int fd;
- fd = get_unused_fd();
+ fd = get_unused_fd_flags(flags);
if (likely(fd >= 0)) {
struct file *file = get_empty_filp();
@@ -391,10 +391,10 @@ static int sock_attach_fd(struct socket *sock, struct file *file)
return 0;
}
-int sock_map_fd(struct socket *sock)
+static int sock_map_fd_flags(struct socket *sock, int flags)
{
struct file *newfile;
- int fd = sock_alloc_fd(&newfile);
+ int fd = sock_alloc_fd(&newfile, flags);
if (likely(fd >= 0)) {
int err = sock_attach_fd(sock, newfile);
@@ -409,6 +409,11 @@ int sock_map_fd(struct socket *sock)
return fd;
}
+int sock_map_fd(struct socket *sock)
+{
+ return sock_map_fd_flags(sock, 0);
+}
+
static struct socket *sock_from_file(struct file *file, int *err)
{
if (file->f_op == &socket_file_ops)
@@ -1208,7 +1213,7 @@ asmlinkage long sys_socket(int family, int type, int protocol)
if (retval < 0)
goto out;
- retval = sock_map_fd(sock);
+ retval = sock_map_fd_flags(sock, current->indirect_params.file_flags.flags);
if (retval < 0)
goto out_release;
@@ -1249,13 +1254,13 @@ asmlinkage long sys_socketpair(int family, int type, int protocol,
if (err < 0)
goto out_release_both;
- fd1 = sock_alloc_fd(&newfile1);
+ fd1 = sock_alloc_fd(&newfile1, current->indirect_params.file_flags.flags);
if (unlikely(fd1 < 0)) {
err = fd1;
goto out_release_both;
}
- fd2 = sock_alloc_fd(&newfile2);
+ fd2 = sock_alloc_fd(&newfile2, current->indirect_params.file_flags.flags);
if (unlikely(fd2 < 0)) {
err = fd2;
put_filp(newfile1);
@@ -1411,7 +1416,7 @@ asmlinkage long sys_accept(int fd, struct sockaddr __user *upeer_sockaddr,
*/
__module_get(newsock->ops->owner);
- newfd = sock_alloc_fd(&newfile);
+ newfd = sock_alloc_fd(&newfile, current->indirect_params.file_flags.flags);
if (unlikely(newfd < 0)) {
err = newfd;
sock_release(newsock);
Ulrich Drepper a ?crit :
> This is a first user of sys_indirect. Several of the socket-related system
> calls which produce a file handle now can be passed an additional parameter
> to set the FD_CLOEXEC flag.
>
> - retval = sock_map_fd(sock);
> + retval = sock_map_fd_flags(sock, current->indirect_params.file_flags.flags);
Yes, we know now why you wanted sys_indirect so much :)
However, there should be a macro or something to ease reader mind...
#define INDPARAM(SUBSYSNAME,PARAMNAME) \
(current->indirect_params.##SUBSYSNAME.##PARAMNAME)
> + retval = sock_map_fd_flags(sock, INDPARAM(file_flags,flags));
you know... i understand the need for FD_CLOEXEC -- in fact i tried
petitioning for CLOEXEC options to all the fd creating syscalls something
like 7 years ago when i was banging my head against the wall trying to
figure out how to thread apache... but even still i'm not convinced that
extending every system call which creates an fd is the way to do this.
honestly i think there should be a per-task flag which indicates whether
fds are by default F_CLOEXEC or not. my reason: third party libraries.
i can control all my own code in a threaded program, but i can't control
all the code which is linked in. fds are going to leak.
if i set a per task flag then the only thing which would break are third
party libraries which use fork/exec and aren't aware they may need to
unset F_CLOEXEC. personally i'd rather break that than leak fds to
another program.
but hey i'm happy to see this sort of thing is finally being fixed,
thanks.
-dean
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
dean gaudet wrote:
> honestly i think there should be a per-task flag which indicates whether
> fds are by default F_CLOEXEC or not. my reason: third party libraries.
Only somebody who thinks exclusively about applications as opposed to
runtimes/libraries can say something like that. Library writers don't
have the luxury of being able to modify any global state. This has all
been discussed here before.
- --
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (GNU/Linux)
iD8DBQFHPd8b2ijCOnn/RHQRAuPPAKCm5mcOl8dycDenxi7BNFdrf2IfWgCgmaXQ
Fj7V13HU1vX6fM9bRumxRpk=
=UIi1
-----END PGP SIGNATURE-----
On Fri, 16 Nov 2007, Ulrich Drepper wrote:
> dean gaudet wrote:
> > honestly i think there should be a per-task flag which indicates whether
> > fds are by default F_CLOEXEC or not. my reason: third party libraries.
>
> Only somebody who thinks exclusively about applications as opposed to
> runtimes/libraries can say something like that. Library writers don't
> have the luxury of being able to modify any global state. This has all
> been discussed here before.
only someone who thinks about writing libraries can say something like
that. you've solved the problem for yourself, and for well written
applications, but not for the other 99.9999% of libraries out there.
i'm not suggesting the library set the global flag. i'm suggesting that
me as an app writer will do so.
it seems like both methods are useful.
-dean
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
dean gaudet wrote:
> i'm not suggesting the library set the global flag. i'm suggesting that
> me as an app writer will do so.
>
> it seems like both methods are useful.
No, the global flag is hardly ever useful. You almost never know the
details of all the libraries you link to well enough to determine that
they don't need FD_CLOEXEC disabled. Even more problematic, you cannot
know whether they will need it in future.
For applications the solution is simple: wrap to appropriate calls.
Apache has all these apr_ wrappers. But them to some good news after all.
- --
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org
iD8DBQFHPeim2ijCOnn/RHQRAu8xAJsF/0Ir1PWMbHkVRaI5vKOGFS4tMACfVEs9
pMYAiCAU1E2B+7QR0EP+/F8=
=btt9
-----END PGP SIGNATURE-----