2007-11-20 06:55:45

by Ulrich Drepper

[permalink] [raw]
Subject: [PATCHv4 4/6] Allow setting FD_CLOEXEC flag for new sockets

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.

arch/x86/ia32/Makefile | 1 +
arch/x86/ia32/sys_ia32.c | 4 ++++
include/asm-x86/ia32_unistd.h | 1 +
include/linux/indirect.h | 33 +++++++++++++++++++++++++++++++++
kernel/Makefile | 2 ++
kernel/indirect.c | 4 ++++
net/socket.c | 21 +++++++++++++--------
7 files changed, 58 insertions(+), 8 deletions(-)

--- arch/x86/ia32/Makefile
+++ arch/x86/ia32/Makefile
@@ -36,6 +36,7 @@ $(obj)/vsyscall-sysenter.so.dbg $(obj)/vsyscall-syscall.so.dbg: \
$(obj)/vsyscall-%.so.dbg: $(src)/vsyscall.lds $(obj)/vsyscall-%.o FORCE
$(call if_changed,syscall)

+CFLAGS_sys_ia32.o = -Wno-undef
AFLAGS_vsyscall-sysenter.o = -m32 -Wa,-32
AFLAGS_vsyscall-syscall.o = -m32 -Wa,-32

--- kernel/Makefile
+++ kernel/Makefile
@@ -67,6 +67,8 @@ ifneq ($(CONFIG_SCHED_NO_NO_OMIT_FRAME_POINTER),y)
CFLAGS_sched.o := $(PROFILING) -fno-omit-frame-pointer
endif

+CFLAGS_indirect.o = -Wno-undef
+
$(obj)/configs.o: $(obj)/config_data.h

# config_data.h contains the same information as ikconfig.h but gzipped.
diff -u net/socket.c net/socket.c
--- net/socket.c
+++ net/socket.c
@@ -344,11 +344,11 @@
* 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 @@
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 @@
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 @@
if (retval < 0)
goto out;

- retval = sock_map_fd(sock);
+ retval = sock_map_fd_flags(sock, INDIRECT_PARAM(file_flags, flags));
if (retval < 0)
goto out_release;

@@ -1249,13 +1254,13 @@
if (err < 0)
goto out_release_both;

- fd1 = sock_alloc_fd(&newfile1);
+ fd1 = sock_alloc_fd(&newfile1, INDIRECT_PARAM(file_flags, flags));
if (unlikely(fd1 < 0)) {
err = fd1;
goto out_release_both;
}

- fd2 = sock_alloc_fd(&newfile2);
+ fd2 = sock_alloc_fd(&newfile2, INDIRECT_PARAM(file_flags, flags));
if (unlikely(fd2 < 0)) {
err = fd2;
put_filp(newfile1);
@@ -1411,7 +1416,7 @@
*/
__module_get(newsock->ops->owner);

- newfd = sock_alloc_fd(&newfile);
+ newfd = sock_alloc_fd(&newfile, INDIRECT_PARAM(file_flags, flags));
if (unlikely(newfd < 0)) {
err = newfd;
sock_release(newsock);
diff -u arch/x86/ia32/sys_ia32.c arch/x86/ia32/sys_ia32.c
--- arch/x86/ia32/sys_ia32.c
+++ arch/x86/ia32/sys_ia32.c
@@ -902,6 +902,10 @@

switch (INDIRECT_SYSCALL32(&regs))
{
+#define INDSYSCALL(name) __NR_ia32_##name
+#include <linux/indirect.h>
+ break;
+
default:
return -EINVAL;
}
diff -u include/linux/indirect.h include/linux/indirect.h
--- include/linux/indirect.h
+++ include/linux/indirect.h
@@ -1,6 +1,39 @@
+#ifndef INDSYSCALL
#ifndef _LINUX_INDIRECT_H
#define _LINUX_INDIRECT_H

#include <asm/indirect.h>

+
+union indirect_params {
+ struct {
+ int flags;
+ } file_flags;
+};
+
+#define INDIRECT_PARAM(set, name) current->indirect_params.set.name
+
+#endif
+#else
+
+/* Here comes the list of system calls which can be called through
+ sys_indirect. When the list if support system calls is needed the
+ file including this header is supposed to define a macro "INDSYSCALL"
+ which adds a prefix fitting to the use. If the resulting macro is
+ defined we generate a line
+ case MACRO:
+ */
+#if INDSYSCALL(accept)
+ case INDSYSCALL(accept):
+#endif
+#if INDSYSCALL(socket)
+ case INDSYSCALL(socket):
+#endif
+#if INDSYSCALL(socketcall)
+ case INDSYSCALL(socketcall):
+#endif
+#if INDSYSCALL(socketpair)
+ case INDSYSCALL(socketpair):
+#endif
+
#endif
diff -u kernel/indirect.c kernel/indirect.c
--- kernel/indirect.c
+++ kernel/indirect.c
@@ -19,6 +19,10 @@

switch (INDIRECT_SYSCALL (&regs))
{
+#define INDSYSCALL(name) __NR_##name
+#include <linux/indirect.h>
+ break;
+
default:
return -EINVAL;
}
--- include/asm-x86/ia32_unistd.h
+++ include/asm-x86/ia32_unistd.h
@@ -12,6 +12,7 @@
#define __NR_ia32_exit 1
#define __NR_ia32_read 3
#define __NR_ia32_write 4
+#define __NR_ia32_socketcall 102
#define __NR_ia32_sigreturn 119
#define __NR_ia32_rt_sigreturn 173


2007-11-20 18:00:58

by Zach Brown

[permalink] [raw]
Subject: Re: [PATCHv4 4/6] Allow setting FD_CLOEXEC flag for new sockets


> +union indirect_params {
> + struct {
> + int flags;
> + } file_flags;
> +};

Have you given thought to having to perform compat translation on this?
Today it's only copied directly from the user pointer into the union
in the task_struct.

I'd love if we could only use fixed-width fields in the interface to
avoid the compat nonsense.

- z

2007-11-20 18:12:22

by Ulrich Drepper

[permalink] [raw]
Subject: Re: [PATCHv4 4/6] Allow setting FD_CLOEXEC flag for new sockets

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Zach Brown wrote:
> Have you given thought to having to perform compat translation on this?
> Today it's only copied directly from the user pointer into the union
> in the task_struct.

Since there is no legacy interface to worry about all members added to
the structure can and should be neutral of the word size. We've done
this with some syscalls already (like pread64) where we always use the
wide form in the parameter list. It's just more simple here since it
does not have to split into two 32-bit registers.

- --
➧ 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

iD8DBQFHQyJn2ijCOnn/RHQRAmWeAJ0Q6qBDtZDvsZYlfBnPFL6n11Z+lwCghiVp
NklFHsSnVyQYMD5rinDFQPo=
=Yo5E
-----END PGP SIGNATURE-----

2007-11-20 18:15:39

by Zach Brown

[permalink] [raw]
Subject: Re: [PATCHv4 4/6] Allow setting FD_CLOEXEC flag for new sockets


> Since there is no legacy interface to worry about all members added to
> the structure can and should be neutral of the word size.

OK, perhaps add a giant comment warning about that. History tells us
that people will get it wrong.

> We've done
> this with some syscalls already (like pread64) where we always use the
> wide form in the parameter list. It's just more simple here since it
> does not have to split into two 32-bit registers.

*nod*

- z