Re: [Linux-streams] Re: [PATCH] Re: export of sys_call_tabl

Re: [Linux-streams] Re: [PATCH] Re: export of sys_call_tabl

[Petr Vandrovec]

On 10 Oct 02 at 11:32, David Grothe wrote:

> Previous patch was "better" but not correct.  How about this? (changed 
> return 0 to return ret).

Yes, it's ok with me. You can still do
register(NULL, notNULL);
register(notNULL, NULL);
without trigerring any error, but maybe that it can be called feature.
                                        Best regards,
                                            Petr Vandrovec
                                            vandrove@vc.cvut.cz
                                            

Re: [Linux-streams] Re: [PATCH] Re: export of sys_call_tabl

[David Grothe]

[-- Attachment #1: Type: text/plain, Size: 181 bytes --]

Attached is the "final" patch.  I eliminated the unregister function.  This 
is tested on stock 2.4.19 kernel.

Will someone see that it is added to the kernel source?

Thanks,
Dave

[-- Attachment #2: stock-i386-2.4.19.txt --]
[-- Type: text/plain, Size: 2559 bytes --]

--- arch/i386/kernel/entry.S.orig	2002-08-02 19:39:42.000000000 -0500
+++ arch/i386/kernel/entry.S	2002-10-08 15:43:08.000000000 -0500
@@ -584,8 +584,8 @@
 	.long SYMBOL_NAME(sys_capset)           /* 185 */
 	.long SYMBOL_NAME(sys_sigaltstack)
 	.long SYMBOL_NAME(sys_sendfile)
-	.long SYMBOL_NAME(sys_ni_syscall)		/* streams1 */
-	.long SYMBOL_NAME(sys_ni_syscall)		/* streams2 */
+	.long SYMBOL_NAME(sys_getpmsg)		/* streams1 */
+	.long SYMBOL_NAME(sys_putpmsg)		/* streams2 */
 	.long SYMBOL_NAME(sys_vfork)            /* 190 */
 	.long SYMBOL_NAME(sys_getrlimit)
 	.long SYMBOL_NAME(sys_mmap2)
--- kernel/ksyms.c.orig	2002-08-02 19:39:46.000000000 -0500
+++ kernel/ksyms.c	2002-10-10 11:46:54.000000000 -0500
@@ -497,6 +497,9 @@
 EXPORT_SYMBOL(seq_release);
 EXPORT_SYMBOL(seq_read);
 EXPORT_SYMBOL(seq_lseek);
+extern int register_streams_calls(int (*putpmsg) (int,void *,void *,int,int),
+				   int (*getpmsg) (int,void *,void *,int,int));
+EXPORT_SYMBOL(register_streams_calls);
 
 /* Program loader interfaces */
 EXPORT_SYMBOL(setup_arg_pages);
--- kernel/sys.c.orig	2002-08-02 19:39:46.000000000 -0500
+++ kernel/sys.c	2002-10-10 11:46:44.000000000 -0500
@@ -167,6 +167,48 @@
 	return notifier_chain_unregister(&reboot_notifier_list, nb);
 }
 
+static int (*do_putpmsg) (int, void *, void *, int, int) = NULL;
+static int (*do_getpmsg) (int, void *, void *, int, int) = NULL;
+
+static DECLARE_RWSEM(streams_call_sem) ;
+
+long asmlinkage sys_putpmsg(int fd, void *ctlptr, void *datptr, int band, int flags)
+{
+	int ret = -ENOSYS;
+	down_read(&streams_call_sem) ;	/* should return int, but doesn't */
+	if (do_putpmsg)
+		ret = (*do_putpmsg) (fd, ctlptr, datptr, band, flags);
+	up_read(&streams_call_sem);
+	return ret;
+}
+
+long asmlinkage sys_getpmsg(int fd, void *ctlptr, void *datptr, int band, int flags)
+{
+	int ret = -ENOSYS;
+	down_read(&streams_call_sem) ;	/* should return int, but doesn't */
+	if (do_getpmsg)
+		ret = (*do_getpmsg) (fd, ctlptr, datptr, band, flags);
+	up_read(&streams_call_sem);
+	return ret;
+}
+
+int register_streams_calls(int (*putpmsg) (int, void *, void *, int, int),
+			    int (*getpmsg) (int, void *, void *, int, int))
+{
+	int ret = 0;
+	down_write(&streams_call_sem) ;	/* should return int, but doesn't */
+	if (   (putpmsg != NULL && do_putpmsg != NULL)
+	    || (getpmsg != NULL && do_getpmsg != NULL)
+	   )
+		ret = -EBUSY;
+	else {
+		do_putpmsg = putpmsg;
+		do_getpmsg = getpmsg;
+	}
+	up_write(&streams_call_sem);
+	return ret ;
+}
+
 asmlinkage long sys_ni_syscall(void)
 {
 	return -ENOSYS;

Re: [Linux-streams] Re: [PATCH] Re: export of sys_call_tabl

[Christoph Hellwig]

a) please read Documentation/CodingStyle
b) please add a prototype in a header
c) please make it EXPORT_SYMBOL_GPL

Re: [Linux-streams] Re: [PATCH] Re: export of sys_call_tabl

[David Grothe]

At 06:27 PM 10/10/2002 Thursday, Christoph Hellwig wrote:
>a) please read Documentation/CodingStyle

Is there a specific problem here?  We tried to imitate the kernel coding 
style with this patch.

>b) please add a prototype in a header

Can you suggest which header file would be appropriate?  I would be glad to 
add the prototype there.

>c) please make it EXPORT_SYMBOL_GPL
LiS is LGPL.  Would it work if the exported symbol was GPL only?

As this is something of a replacement for the old exported sys_call_table, 
which was exported generally, we thought that a general export was appropriate.

Thanks,
Dave

Re: [Linux-streams] Re: [PATCH] Re: export of sys_call_tabl

[Arjan van de Ven]

[-- Attachment #1: Type: text/plain, Size: 216 bytes --]

On Thu, 2002-10-10 at 21:07, David Grothe wrote:

> LiS is LGPL.  Would it work if the exported symbol was GPL only?

since LiS becomes GPL when you link it into the kernel with insmod,
that's not a problem ;)

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [Linux-streams] Re: [PATCH] Re: export of sys_call_tabl

[David Grothe]

Guys, I think we are closing in on it.

I ran the patch through Lindent.  I moved the exported symbol to sys.c and 
made it GPL only.  I tested it on 2.4.19.

If we could agree on a header file location for the prototype, or not to 
have a prototype, I could cook up another candidate for "final" in short order.

Thanks for all the suggestions,
Dave

Re: [Linux-streams] Re: [PATCH] Re: export of sys_call_tabl

[Ole Husgaard]

"Brian F. G. Bidulock" wrote:
> On Fri, 11 Oct 2002, Christoph Hellwig wrote:
> > It is not.  Sys_call_table was exported to allow iBCS/Linux-ABI
> 
> I don't know if it matters, but these two calls putpmsg and getpmsg
> are the calls used by iBCS.

AFAIK, iBCS use these syscalls to emulate TLI, and iBCS
only has this emulation working for the IP protocol suite.

LiS is hooking the same syscalls, and is more protocol
independent.

In this way, iBCS and LiS are competing projects, even
if their base objectives are very different (iBCS aims
for user-level binary portability from SysV, while LiS
aims for kernel-level STREAMS code portability from SysV
to extend to Linux).

> No, I don't think anyone wants proprietary syscalls to be registered
> with this facility.  If _GPL can allow an LGPL module to use the
> facility without problems, that will be the best way to go.

An LGPL module with proprietary code linked into it will
taint the kernel. LiS is often linked with proprietary
code, since it is under LGPL.

IMHO, A not-GPL-only export from the kernel is needed
here.

That will not make these syscalls proprietary. Even with
proprietary drivers linked into LiS, it is impossible to
deviate from the SysV definition of putpmsg/getpmsg
unless the code of LiS itself is modified.

Best Regards,

Ole Husgaard.

Re: [Linux-streams] Re: [PATCH] Re: export of sys_call_tabl

[Arjan van de Ven]

[-- Attachment #1: Type: text/plain, Size: 484 bytes --]

On Sat, 2002-10-12 at 04:29, Ole Husgaard wrote:
> "Brian F. G. Bidulock" wrote:
> > On Fri, 11 Oct 2002, Christoph Hellwig wrote:
> > > It is not.  Sys_call_table was exported to allow iBCS/Linux-ABI
> > 
> > I don't know if it matters, but these two calls putpmsg and getpmsg
> > are the calls used by iBCS.
> 
> AFAIK, iBCS use these syscalls to emulate TLI, and iBCS
iBCS doesn't exist for 2.4 or 2.5 kernels
it's called linux-abi now and does NOT use these syscalls


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [Linux-streams] Re: [PATCH] Re: export of sys_call_tabl

[Brian F. G. Bidulock]

[-- Attachment #1: msg.pgp --]
[-- Type: application/pgp, Size: 1142 bytes --]

Re: [Linux-streams] Re: [PATCH] Re: export of sys_call_tabl

[Brian F. G. Bidulock]

Ole,

I don't think that exporting putpmsg and getpmsg as _GPL will
stop proprietary modules from being linked with LiS.  LiS exports
its symbols on a different basis.  There is no need for a proprietary
module using LiS to access putpmsg or getpmsg or the syscall
registration facility for that matter.  Is you concern that LiS
using a _GPL only facility will force GPL on modules linked with LiS
even though LiS is LGPL?

--brian

On Sat, 12 Oct 2002, Ole Husgaard wrote:

> "Brian F. G. Bidulock" wrote:
> > On Fri, 11 Oct 2002, Christoph Hellwig wrote:
> > > It is not.  Sys_call_table was exported to allow iBCS/Linux-ABI
> > 
> > I don't know if it matters, but these two calls putpmsg and getpmsg
> > are the calls used by iBCS.
> 
> AFAIK, iBCS use these syscalls to emulate TLI, and iBCS
> only has this emulation working for the IP protocol suite.
> 
> LiS is hooking the same syscalls, and is more protocol
> independent.
> 
> In this way, iBCS and LiS are competing projects, even
> if their base objectives are very different (iBCS aims
> for user-level binary portability from SysV, while LiS
> aims for kernel-level STREAMS code portability from SysV
> to extend to Linux).
> 
> > No, I don't think anyone wants proprietary syscalls to be registered
> > with this facility.  If _GPL can allow an LGPL module to use the
> > facility without problems, that will be the best way to go.
> 
> An LGPL module with proprietary code linked into it will
> taint the kernel. LiS is often linked with proprietary
> code, since it is under LGPL.
> 
> IMHO, A not-GPL-only export from the kernel is needed
> here.
> 
> That will not make these syscalls proprietary. Even with
> proprietary drivers linked into LiS, it is impossible to
> deviate from the SysV definition of putpmsg/getpmsg
> unless the code of LiS itself is modified.
> 
> Best Regards,
> 
> Ole Husgaard.

-- 
Brian F. G. Bidulock    ¦ The reasonable man adapts himself to the ¦
bidulock@openss7.org    ¦ world; the unreasonable one persists in  ¦
http://www.openss7.org/ ¦ trying  to adapt the  world  to himself. ¦
                        ¦ Therefore  all  progress  depends on the ¦
                        ¦ unreasonable man. -- George Bernard Shaw ¦

Re: [Linux-streams] Re: [PATCH] Re: export of sys_call_tabl

[Alan Cox]

On Sat, 2002-10-12 at 10:56, Brian F. G. Bidulock wrote:
> Ole,
> 
> I don't think that exporting putpmsg and getpmsg as _GPL will
> stop proprietary modules from being linked with LiS.  LiS exports
> its symbols on a different basis.  There is no need for a proprietary
> module using LiS to access putpmsg or getpmsg or the syscall
> registration facility for that matter.  Is you concern that LiS
> using a _GPL only facility will force GPL on modules linked with LiS
> even though LiS is LGPL?

The _GPL is just to help people and guide them. It doesn't alter what is
and is not an offence under copyright law.

Re: [Linux-streams] Re: [PATCH] Re: export of sys_call_tabl

[Ole Husgaard]

"Brian F. G. Bidulock" wrote:
> Is you concern that LiS
> using a _GPL only facility will force GPL on modules linked with LiS
> even though LiS is LGPL?

No, my concern is that the kernel should be marked as
tainted when appropriate, thus giving volunteer kernel
hackers a change to decide if they want to help with an
oops from a tainted kernel.

For well-defined fundamental interfaces between genuinely
seperate works, I think using EXPORT_SYMBOL is more
appropriate than EXPORT_SYMBOL_GPL.
Syscalls like putpmsg and getpmsg fall into this category,
even if wrapped in a thin layer to facilitate MP-safe
registration and deregistration.

The simple act of a LGPL module hooking two syscalls not
implemented by the standard kernel is IMHO no good reason
for marking the kernel as such tainted.

Best Regards,

Ole Husgaard.

Re: [Linux-streams] Re: [PATCH] Re: export of sys_call_tabl

[Petr Vandrovec]

On 10 Oct 02 at 21:33, Arjan van de Ven wrote:
> On Thu, 2002-10-10 at 21:07, David Grothe wrote:
> 
> > LiS is LGPL.  Would it work if the exported symbol was GPL only?
> 
> since LiS becomes GPL when you link it into the kernel with insmod,
> that's not a problem ;)

Only problem is that modutils do not know about LGPL, you must use
"GPL with additional rights" in source (at least Debian's 2.4.19-3),
and "GPL with additional rights" might be too ambiguous for authors.

                                            Petr Vandrovec
                                            vandrove@vc.cvut.cz
                                            

Re: [Linux-streams] Re: [PATCH] Re: export of sys_call_tabl

[David Grothe]

[-- Attachment #1: Type: text/plain, Size: 203 bytes --]

Here is a proposed "final" patch for the streams hooks.  I think that it 
takes into account all suggestions.  I put the prototype in linux/sys.h.

This is for stock 2.4.19, and has been tested.

-- Dave

[-- Attachment #2: stock-i386-2.4.19.txt --]
[-- Type: text/plain, Size: 2883 bytes --]

--- arch/i386/kernel/entry.S.orig	2002-08-02 19:39:42.000000000 -0500
+++ arch/i386/kernel/entry.S	2002-10-08 15:43:08.000000000 -0500
@@ -584,8 +584,8 @@
 	.long SYMBOL_NAME(sys_capset)           /* 185 */
 	.long SYMBOL_NAME(sys_sigaltstack)
 	.long SYMBOL_NAME(sys_sendfile)
-	.long SYMBOL_NAME(sys_ni_syscall)		/* streams1 */
-	.long SYMBOL_NAME(sys_ni_syscall)		/* streams2 */
+	.long SYMBOL_NAME(sys_getpmsg)		/* streams1 */
+	.long SYMBOL_NAME(sys_putpmsg)		/* streams2 */
 	.long SYMBOL_NAME(sys_vfork)            /* 190 */
 	.long SYMBOL_NAME(sys_getrlimit)
 	.long SYMBOL_NAME(sys_mmap2)
--- kernel/sys.c.orig	2002-08-02 19:39:46.000000000 -0500
+++ kernel/sys.c	2002-10-10 15:00:45.000000000 -0500
@@ -167,6 +167,49 @@
 	return notifier_chain_unregister(&reboot_notifier_list, nb);
 }
 
+static int (*do_putpmsg) (int, void *, void *, int, int) = NULL;
+static int (*do_getpmsg) (int, void *, void *, int, int) = NULL;
+
+static DECLARE_RWSEM(streams_call_sem);
+
+long asmlinkage
+sys_putpmsg(int fd, void *ctlptr, void *datptr, int band, int flags)
+{
+	int ret = -ENOSYS;
+	down_read(&streams_call_sem);	/* should return int, but doesn't */
+	if (do_putpmsg)
+		ret = (*do_putpmsg) (fd, ctlptr, datptr, band, flags);
+	up_read(&streams_call_sem);
+	return ret;
+}
+
+long asmlinkage
+sys_getpmsg(int fd, void *ctlptr, void *datptr, int band, int flags)
+{
+	int ret = -ENOSYS;
+	down_read(&streams_call_sem);	/* should return int, but doesn't */
+	if (do_getpmsg)
+		ret = (*do_getpmsg) (fd, ctlptr, datptr, band, flags);
+	up_read(&streams_call_sem);
+	return ret;
+}
+
+int
+register_streams_calls(int (*putpmsg) (int, void *, void *, int, int),
+		       int (*getpmsg) (int, void *, void *, int, int))
+{
+	int ret = -EBUSY;
+	down_write(&streams_call_sem);	/* should return int, but doesn't */
+	if (   (putpmsg == NULL || do_putpmsg == NULL)
+	    && (getpmsg == NULL || do_getpmsg == NULL)) {
+		do_putpmsg = putpmsg;
+		do_getpmsg = getpmsg;
+		ret = 0;
+	}
+	up_write(&streams_call_sem);
+	return ret;
+}
+
 asmlinkage long sys_ni_syscall(void)
 {
 	return -ENOSYS;
@@ -1286,3 +1329,4 @@
 EXPORT_SYMBOL(unregister_reboot_notifier);
 EXPORT_SYMBOL(in_group_p);
 EXPORT_SYMBOL(in_egroup_p);
+EXPORT_SYMBOL_GPL(register_streams_calls);
--- include/linux/sys.h.orig	2002-10-11 08:59:10.000000000 -0500
+++ include/linux/sys.h	2002-10-11 09:15:04.000000000 -0500
@@ -27,4 +27,16 @@
  * These are system calls that haven't been implemented yet
  * but have an entry in the table for future expansion..
  */
+
+/*
+ * These are registration routines for system calls that are
+ * implemented by loadable modules outside of the kernel
+ * source tree.
+ */
+#if !defined(__ASSEMBLY__)
+extern int
+register_streams_calls(int (*putpmsg) (int, void *, void *, int, int),
+		       int (*getpmsg) (int, void *, void *, int, int)) ;
+
+#endif			/* __ASSEMBLY__ */
 #endif

Re: [Linux-streams] Re: [PATCH] Re: export of sys_call_tabl

[Alan Cox]

On Fri, 2002-10-11 at 15:22, David Grothe wrote:
> Here is a proposed "final" patch for the streams hooks.  I think that it 
> takes into account all suggestions.  I put the prototype in linux/sys.h.
> 
> This is for stock 2.4.19, and has been tested.
> 

Looks good to me

Re: [Linux-streams] Re: [PATCH] Re: export of sys_call_tabl

[David Grothe]

Previous patch was "better" but not correct.  How about this? (changed 
return 0 to return ret).

int register_streams_calls(int (*putpmsg) (int, void *, void *, int, int),
                             int (*getpmsg) (int, void *, void *, int, int))
{
         int ret = 0;
         down_write(&streams_call_sem) ; /* should return int, but doesn't */
         if (   (putpmsg != NULL && do_putpmsg != NULL)
             || (getpmsg != NULL && do_getpmsg != NULL)
            )
                 ret = -EBUSY;
         else {
                 do_putpmsg = putpmsg;
                 do_getpmsg = getpmsg;
         }
         up_write(&streams_call_sem);
         return ret ;
}

Re: [Linux-streams] Re: [PATCH] Re: export of sys_call_tabl

[Petr Vandrovec]

On 10 Oct 02 at 11:01, David Grothe wrote:
> 
> Does this patch address your suggestions?  This has been tested on 2.4.19.

Well, it can be that way. But if you are allowing
register_streams_calls(NULL, NULL), maybe you can move 
unregister_streams_calls() to the headers and make it inline.

And you are returning while holding streams_call_sem semaphore
when failing with -EBUSY. It is not good idea.
                                Petr Vandrovec
                                vandrove@vc.cvut.cz                                                    

+int register_streams_calls(int (*putpmsg) (int, void *, void *, int, int),
+               int (*getpmsg) (int, void *, void *, int, int))
+{
+   down_write(&streams_call_sem) ; /* should return int, but doesn't */
+   if (   (putpmsg != NULL && do_putpmsg != NULL)
+       || (getpmsg != NULL && do_getpmsg != NULL)
+      )
+       return -EBUSY;
+   do_putpmsg = putpmsg;
+   do_getpmsg = getpmsg;
+   up_write(&streams_call_sem);
+   return 0 ;
+}
+
+void unregister_streams_calls(void)
+{
+   register_streams_calls(NULL, NULL);
+}
+
 asmlinkage long sys_ni_syscall(void)
 {
    return -ENOSYS;

Re: [Linux-streams] Re: [PATCH] Re: export of sys_call_tabl

[David Grothe]

This looks better:

int register_streams_calls(int (*putpmsg) (int, void *, void *, int, int),
                             int (*getpmsg) (int, void *, void *, int, int))
{
         int ret = 0;
         down_write(&streams_call_sem) ; /* should return int, but doesn't */
         if (   (putpmsg != NULL && do_putpmsg != NULL)
             || (getpmsg != NULL && do_getpmsg != NULL)
            )
                 ret = -EBUSY;
         else {
                 do_putpmsg = putpmsg;
                 do_getpmsg = getpmsg;
         }
         up_write(&streams_call_sem);
         return 0 ;
}

How about if we just eliminate the unregister_streams routine?  LiS can 
just call register_streams(NULL,NULL) when it wants to unregister.

-- Dave

At 06:25 PM 10/10/2002 Thursday, Petr Vandrovec wrote:
>On 10 Oct 02 at 11:01, David Grothe wrote:
> >
> > Does this patch address your suggestions?  This has been tested on 2.4.19.
>
>Well, it can be that way. But if you are allowing
>register_streams_calls(NULL, NULL), maybe you can move
>unregister_streams_calls() to the headers and make it inline.
>
>And you are returning while holding streams_call_sem semaphore
>when failing with -EBUSY. It is not good idea.
>                                 Petr Vandrovec
>                                 vandrove@vc.cvut.cz 
>
>
>+int register_streams_calls(int (*putpmsg) (int, void *, void *, int, int),
>+               int (*getpmsg) (int, void *, void *, int, int))
>+{
>+   down_write(&streams_call_sem) ; /* should return int, but doesn't */
>+   if (   (putpmsg != NULL && do_putpmsg != NULL)
>+       || (getpmsg != NULL && do_getpmsg != NULL)
>+      )
>+       return -EBUSY;
>+   do_putpmsg = putpmsg;
>+   do_getpmsg = getpmsg;
>+   up_write(&streams_call_sem);
>+   return 0 ;
>+}
>+
>+void unregister_streams_calls(void)
>+{
>+   register_streams_calls(NULL, NULL);
>+}
>+
>  asmlinkage long sys_ni_syscall(void)
>  {
>     return -ENOSYS;