add a stub by which a module can bind to the AFS syscall

Hi Linus,

This patch makes it possible for a module to bind safely to the AFS syscall, without having to modify the syscall table directly.

David

diff -uNr -x'*.o' -x'.*' -xTAGS linux-2.5.67/include/linux/syscallstub.h linux-2.5.67-afs/include/linux/syscallstub.h
@@ -0,0 +1,28 @@
+/* syscallstub.h: system call stub management
+ *
+ * Copyright (C) 2003 Red Hat, Inc. All Rights Reserved.
+ * Written by David Howells ([email protected])
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+#ifndef _LINUX_SYSCALLSTUB_H
+#define _LINUX_SYSCALLSTUB_H
+
+#include <linux/module.h>
+
+/*
+ * AFS system call stub management
+ */
+typedef int (*afs_syscall_handler_t)(long cmd, va_list va);
+
+extern int register_afs_syscall_handler(afs_syscall_handler_t handler, struct module *owner);
+extern void unregister_afs_syscall_handler(afs_syscall_handler_t handler, struct module *owner);
+
+extern asmlinkage int sys_afs(long cmd, ...);
+
+
+#endif /* _LINUX_SYSCALLSTUB_H */

diff -uNr -x'*.o' -x'.*' -xTAGS linux-2.5.67/kernel/Makefile linux-2.5.67-afs/kernel/Makefile
@@ -6,7 +6,8 @@
 	    exit.o itimer.o time.o softirq.o resource.o \
 	    sysctl.o capability.o ptrace.o timer.o user.o \
 	    signal.o sys.o kmod.o workqueue.o futex.o pid.o \
-	    rcupdate.o intermodule.o extable.o params.o posix-timers.o
+	    rcupdate.o intermodule.o extable.o params.o posix-timers.o \
+	    syscallstub.o
 
 obj-$(CONFIG_GENERIC_ISA_DMA) += dma.o
 obj-$(CONFIG_SMP) += cpu.o

diff -uNr -x'*.o' -x'.*' -xTAGS linux-2.5.67/arch/i386/kernel/entry.S linux-2.5.67-afs/arch/i386/kernel/entry.S
@@ -721,7 +721,7 @@
 	.long sys_bdflush
 	.long sys_sysfs		/* 135 */
 	.long sys_personality
-	.long sys_ni_syscall	/* reserved for afs_syscall */
+	.long sys_afs
 	.long sys_setfsuid16
 	.long sys_setfsgid16
 	.long sys_llseek	/* 140 */

Umm, you're adding a handler so that a module can register a variadic multiplexer syscall?? I think you need to rething your design..

It's better than the alternative, having OpenAFS patch the system call table itself... ;-)

Jeff

That's already taken care of with David's first patch. There's no reason we can't have an afsctlfs like the nfsctlfs. But anyway, please list the prototypes and usages of the subcalls here, maybe some of them are generally usefull.

This kind of "I need a stuff fr a random syscall multiplexer" requests are silly. APIs need review or you'll get the syssgi syndrome really soon..

i dont believe it needs to be a variadic (the afs syscall will/uses the first params). however, the syscall interface for afs predates linux. changing to a different interface just for linux kernels seems somewhat costly (programmers time) when this existing interface has been well tested and debugged on several other operating systems.

We need to repeat a mistake others did has never been a valid argument in linux devlopment.. Anyway, it's really hard to judge about this before seeing the actual implementation instead of just saying here's a stub I need.

at the time afs was written it wasnt a mistake. syscall was the only (easy) way into the kernel from user space. adding multiple syscalls would have just been completely painful. as for examples, pioctl() -- the user space of the afs syscall -- is a bit like syssgi() i am afraid:

venus/fs.c:     code = pioctl(0, VIOC_GETCELLSTATUS, &blob, 1);
venus/fs.c:    code = pioctl(0, VIOC_SETRXKCRYPT, &blob, 1);
vlserver/sascnvldb.c:   code = pioctl(ti->data, VIOCGETVOLSTAT, &blob, 1);
auth/ktc_nt.c:  code = pioctl(0, VIOCNEWGETTOK, &iob, 0);
auth/ktc_nt.c:  code = pioctl(0, VIOCDELTOK, &iob, 0);
package/package.c:  code = pioctl(0, VIOC_AFS_SYSNAME, &data, 1);
venus/up.c:          code = pioctl(file1, _VICEIOCTL(2), &blob, 1);

in reality, very few things other than afs are going to want to use the afs syscall (arla might be a possible user).

Oh yes, it was. The same mistake as the even earlier SysV IPC mess.

So fix the AFS code up to use a routine for each subcall that can still map to pioctl for !linux. After that we can continue the discussion on how these calls are best implemented on linux.

because time is precious its quite a bit easier to fix one spot in the linux kernel than to fix a hundred or so in the afs code.

It might be easier but it's not the correct fix. I've we had done what's easier all the time Linux would look like SCO Unix or IRIX now..

ehm no.
There is no valid AFS code using this since there is no syscall yet ;)

And it IS more work for linux, since 32 bit emulation on 64 bit machines will be a MAJOR pain if everything is this opaque.

And multiplexed syscalls are a pain for people like me who have to write 32/64 bit translations.

Anton

Which means only one thing - changing that API will affect very few things.

Let's keep the kernel side sane. We don't have to mess with multiplexors and even if we decide to use them, we will be better off by having decoder outside of AFS proper. Again, take a look at interaction between userland and knfsd. Right now we have a sane interface (IO on nfsctl files) and we have a wrapper (sys_nfsctl) that does decode/open/write/read/close.

Hi Alex,

As you suggested on IRC, almost all pioctls can be emulated by either:

(1) doing a decode/mount/open/write/read/close on a file on a special internal filesystem if an inode is not required,

or:

(2) translating the call to get/set/lget/lsetxattr calls if an inode is required.

However, there's always an exception. One of the pioctls requires three things:

(a) a dentry and inode,
(b) an input buffer with a filename in it,
(c) an output buffer for holding a fair amount of data.

This can't be done easily with (1) because of (a), and it can't be done atomically as (2) because two separate calls would have to be made for (b) and
(c) (and the netfs would have to retain some sort of state).

How about, instead, for pioctl calls that require an inode, the AFS multiplexor in the kernel invents a temporary file structure, points it at the dentry and calls its open, ioctl and release calls on inode->f_op?

Or perhaps you'd consider adding an extra inode operation?

	struct inode_operations {
		...
		int (*inodectl) (struct dentry *, unsigned int, unsigned long);
	};

David

I selected a variadic method of argument passing because I didn't want to have to copy the argument block several times in the course of passing from the syscall stub through the multiplexor to the actual handler.

Of course, it might be better if the kernel itself sorted out the individual subcalls and passed them to the filesystem individually or dealt with them itself.

The four calls implemented by Linux are:

(*) int setpag(void)

Set Process Authentication Group number. This could easily be moved into the kernel proper, with the PAG being stored in or depending from the task structure somehow.

This would then obviate the need for OpenAFS to mangle the setgroups and getgroups syscalls.

(*) int pioctl(const char *path, int cmd, void *arg, int followsymlink)

Al Viro's favourite:-) Do ioctl() on a file refered to by pathname. Can't be emulated by open/ioctl/close because:

(a) it can operate directly on symbolic links.

(b) some of its functions don't require a file and don't fail if one can't be opened.

(*) int afs_call(...)

Local client control

(*) int afs_icl(...)

Local client status and logging control.

There are six more which linux doesn't actually support, even though the multiplexor does:

(*) icreate
(*) iopen
(*) idec
(*) iinc
(*) iread
(*) iwrite

Deal with file by inode number.

David

So please submit a patch for doing this in the kernel proper.

You don't expect we merge something that broken? And then as multiplexer inside a multiplexer? This starts to look worse than sys_ipc()..

What's ...?

Has been proposed many times in the context of Coda. Perhaps now that there are 2 filesystems in the tree that want something like this we can afford the extra int in the task structure. More likely we'll just have to wait patiently for a credentials cache, or task ornaments, or something might be possible with the LSM framework.

Coda has the same abomination, but we use an ioctl on a special file in the root of the Coda tree (/coda/.CONTROL). The advantage of this is that if we have multiple clients and mountpoints we know which client is supposed to handle the pioctl.

We don't have anything like that, and most likely just stuffed them into the pioctl multiplexor.

You don't want to do that anyways, it will only work reliably on a few filesystems. ReiserFS can possibly have inode collisions which are resolved based on the parent directory inode. ext3 only journals writes when a filehandle is closed, so opening by inode, writing to it and closing it doesn't trigger a journal update. ramfs and tmpfs really don't have any usable underlying inodes that can be opened with a device/inode number pair because there is no 'device'.

Jan

> On Wed, Apr 30, 2003 at 04:30:20PM +0100, David Howells wrote:
>> The four calls implemented by Linux are:
>>
>> (*) int setpag(void)
>>
>> Set Process Authentication Group number. This could easily be >> moved into the kernel proper, with the PAG being stored in or >> depending from the task structure somehow.
>>
>> This would then obviate the need for OpenAFS to mangle the >> setgroups and getgroups syscalls.

> Has been proposed many times in the context of Coda. Perhaps > now that there are 2 filesystems in the tree that want > something like this we can afford the extra int in the task > structure.

Make that 3. We would be able to make good use of the same feature for strong authentication on NFS.

Cheers,
Trond