Re: [PATCH] Fix queued SIGIO
Julian Anastasov wrote: > I'm talking about test8. __SI_CODE is in 2.4, not in 2.2. > The handling is very different. We can't wait for si_code==SI_SIGIO > in 2.4 anymore. SI_SIGIO is used only in 2.2:fs/fcntl.c. 2.4 stores > POLL_xxx in si_code instead of SI_SIGIO. Why does it do that? si_band already stores the POLL_xxx code. -- Jamie - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Fix queued SIGIO
Julian Anastasov wrote: I'm talking about test8. __SI_CODE is in 2.4, not in 2.2. The handling is very different. We can't wait for si_code==SI_SIGIO in 2.4 anymore. SI_SIGIO is used only in 2.2:fs/fcntl.c. 2.4 stores POLL_xxx in si_code instead of SI_SIGIO. Why does it do that? si_band already stores the POLL_xxx code. -- Jamie - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Fix queued SIGIO
Hello, On Wed, 20 Sep 2000, Robert H. de Vries wrote: > > This is not needed for SI_SIGIO. It is not generated from > >the kernel. It seems this interface is already obsoleted. > > I think you are mistaking. The purpose of SIGIO is to notify user programs of > IO events detected by the kernel. So its use is almost exclusively in the > kernel. Not in 2.4-test. You have to show me the right place in the sources :) I'm talking about test8. __SI_CODE is in 2.4, not in 2.2. The handling is very different. We can't wait for si_code==SI_SIGIO in 2.4 anymore. SI_SIGIO is used only in 2.2:fs/fcntl.c. 2.4 stores POLL_xxx in si_code instead of SI_SIGIO. > The reason it currently breaks is that when send_sig_info is used with real > info data the check SI_FROMUSER evaluates to true. > Other calls use a pointer to address 1 (ugh!) to inform bad_signal() that > this signal is generated by the kernel. If your are talking about 2.2, of course it is broken but I'm not sure if this bug can be fixed without breaking the binary compatibility. But what about this ugly fix for 2.2: #define SI_KERN_SIGIO(0x200+SI_SIGIO) /* select better value */ fs/fcntl.c:send_sigio() to send SI_KERN_SIGIO (instead of SI_SIGIO) and send_sig_info() to change it to SI_SIGIO after the EPERM check: /* Send SI_SIGIO to user space */ if (info->si_code == SI_KERN_SIGIO) info->si_code = SI_SIGIO; The FROMUSER check is already performed from sys_rt_sigqueueinfo() before send_sig_info(). The other users of send_sig_info() always use info == 0 or 1. So, this allows the PERM check to be bypassed for the SI_SIGIO sent from the kernel. If there are other such codes that we must rewrite before sending them to the user space we can group them using something like this (0x100..0x1FF, SI_KERNEL=0x80 is not in this group, so > 0x100): if (info->si_code - 0x0100 >= 0) info->si_code -= 0x200; si_code is int No changes in the user space. The main goal: to bypass the EPERM check for SI_SIGIO sent from the kernel. Can such fix solve the problem? > Robert > > -- > Robert de Vries > [EMAIL PROTECTED] Regards -- Julian Anastasov <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Fix queued SIGIO
On Wed, 20 Sep 2000, Julian Anastasov wrote: >Hello, > >On Tue, 19 Sep 2000, Robert H. de Vries wrote: >> It would be better to change SI_SIGIO in all the >> include/asm-*/siginfo.h files from -5 to __SI_CODE(__SI_SIGIO, -5) >> __SI_SIGIO would become (6 << 16). > > This is not needed for SI_SIGIO. It is not generated from >the kernel. It seems this interface is already obsoleted. I think you are mistaking. The purpose of SIGIO is to notify user programs of IO events detected by the kernel. So its use is almost exclusively in the kernel. The reason it currently breaks is that when send_sig_info is used with real info data the check SI_FROMUSER evaluates to true. Other calls use a pointer to address 1 (ugh!) to inform bad_signal() that this signal is generated by the kernel. Robert -- Robert de Vries [EMAIL PROTECTED] - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Fix queued SIGIO
On Wed, 20 Sep 2000, Julian Anastasov wrote: Hello, On Tue, 19 Sep 2000, Robert H. de Vries wrote: It would be better to change SI_SIGIO in all the include/asm-*/siginfo.h files from -5 to __SI_CODE(__SI_SIGIO, -5) __SI_SIGIO would become (6 16). This is not needed for SI_SIGIO. It is not generated from the kernel. It seems this interface is already obsoleted. I think you are mistaking. The purpose of SIGIO is to notify user programs of IO events detected by the kernel. So its use is almost exclusively in the kernel. The reason it currently breaks is that when send_sig_info is used with real info data the check SI_FROMUSER evaluates to true. Other calls use a pointer to address 1 (ugh!) to inform bad_signal() that this signal is generated by the kernel. Robert -- Robert de Vries [EMAIL PROTECTED] - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Fix queued SIGIO
Hello, On Tue, 19 Sep 2000, Robert H. de Vries wrote: > It would be better to change SI_SIGIO in all the > include/asm-*/siginfo.h files from -5 to __SI_CODE(__SI_SIGIO, -5) > __SI_SIGIO would become (6 << 16). This is not needed for SI_SIGIO. It is not generated from the kernel. It seems this interface is already obsoleted. > The code in arch/*/kernel/signal.c only copies the lower 16 bits to user > space. This means that the test SI_FROMKERNEL returns true and user space > still gets the same values as before. > > The same trick is indeed used also in my POSIX timer patch. Yes, for every code generated from kernel. The others are in __SI_KILL group. I see, __SI_CODE is fixed in your patch. Very good. There are some unused groups (__SI_RT) but I can't find descriptions or comments. Is it "Real Time" ? Hm. Regards -- Julian Anastasov <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Fix queued SIGIO
Hello,
On Tue, 19 Sep 2000, Chuck Lever wrote:
> also, the test at issue here (from line 363 of kernel/signal.c):
>
> /* If this was sent by a rt mechanism, try again. */
> if (info->si_code != SI_USER) {
> ret = -EAGAIN;
> goto out;
> }
Hm, I can't find this. It is may be already fixed in the latest
kernels.
> we just published a paper in the ALS proceedings describing our
> implementation of a new system call similar to sigtimedwait() that
> collects many events at once.
This is interesting. Let me know if this paper is
accessible from web.
Regards
--
Julian Anastasov <[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Fix queued SIGIO
On Mon, 18 Sep 2000, Alan Cox wrote: >> The problem is really that SI_SIGIO is negative, but it should be positive >> to make SI_FROMUSER return false on it. >> >> Changing it would unfortunately break binary compatibility. This patch > It would be better to change SI_SIGIO in all the include/asm-*/siginfo.h files from -5 to __SI_CODE(__SI_SIGIO, -5) __SI_SIGIO would become (6 << 16). The code in arch/*/kernel/signal.c only copies the lower 16 bits to user space. This means that the test SI_FROMKERNEL returns true and user space still gets the same values as before. The same trick is indeed used also in my POSIX timer patch. Robert -- Robert de Vries [EMAIL PROTECTED] - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Fix queued SIGIO
On Tue, 19 Sep 2000, Julian Anastasov wrote:
> On Mon, 18 Sep 2000, Andi Kleen wrote:
>
> > > SI_SIGIO is not generated from kernel. The same is for the
> > > other SI_ consts < 0 not defined with __SI_CODE.
> >
> > Ok, then you have already broken binary compatibility between 2.2 and 2.4
>
> Looking in the old kernels, it seems the binary compatibility
> was broken in 2.3.21 when si_code returns POLL_xxx events just like
> mentioned in "The Single UNIX ® Specification, Version 2",
> xsh/signal.h.html and not SI_SIGIO.
>
> SI_SIGIO in si_code for 2.2 does not return any information
> about the events. I even see that Redhat maintains patch against 2.2
> to backport the POLL_xxx events from 2.3. Not sure after the changes
> in 2.4.0-test1. Anyway, 2.2 looks unusable for me and I don't see other
> way this problem to be fixed. The binary compatibility is impossible
> to exist. The applications can support the both ways: the old SI_SIGIO
> and the new POLL_xxx events (recompiled after test1) in si_code.
because the 2.2 kernels are already "broken" in this regard, i can't see
how binary compatibility between 2.2 and 2.4 could even be an
issue. applications can't use this stuff in 2.2 without at least the
RedHat patch.
unless there's a problem implementing a glibc that runs on both 2.2 and
2.4, perhaps this should be revisited.
also, the test at issue here (from line 363 of kernel/signal.c):
/* If this was sent by a rt mechanism, try again. */
if (info->si_code != SI_USER) {
ret = -EAGAIN;
goto out;
}
has always been unclear as to its intent. it seems like there is
overloading going on here -- if the real intent is to prevent users
without credentials from sending "kernel-only" signals, then that should
be the logic here.
> The next step is somebody to implement event merging and to
> allow receiving of many events with one call. For the next kernels.
we just published a paper in the ALS proceedings describing our
implementation of a new system call similar to sigtimedwait() that
collects many events at once.
- Chuck Lever
--
corporate: <[EMAIL PROTECTED]>
personal: <[EMAIL PROTECTED]>
The Linux Scalability project:
http://www.citi.umich.edu/projects/linux-scalability/
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Fix queued SIGIO
On Mon, 18 Sep 2000, Alan Cox wrote: The problem is really that SI_SIGIO is negative, but it should be positive to make SI_FROMUSER return false on it. Changing it would unfortunately break binary compatibility. This patch It would be better to change SI_SIGIO in all the include/asm-*/siginfo.h files from -5 to __SI_CODE(__SI_SIGIO, -5) __SI_SIGIO would become (6 16). The code in arch/*/kernel/signal.c only copies the lower 16 bits to user space. This means that the test SI_FROMKERNEL returns true and user space still gets the same values as before. The same trick is indeed used also in my POSIX timer patch. Robert -- Robert de Vries [EMAIL PROTECTED] - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Fix queued SIGIO
Hello,
On Tue, 19 Sep 2000, Chuck Lever wrote:
also, the test at issue here (from line 363 of kernel/signal.c):
/* If this was sent by a rt mechanism, try again. */
if (info-si_code != SI_USER) {
ret = -EAGAIN;
goto out;
}
Hm, I can't find this. It is may be already fixed in the latest
kernels.
we just published a paper in the ALS proceedings describing our
implementation of a new system call similar to sigtimedwait() that
collects many events at once.
This is interesting. Let me know if this paper is
accessible from web.
Regards
--
Julian Anastasov [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Fix queued SIGIO
Hello, On Mon, 18 Sep 2000, Andi Kleen wrote: > > SI_SIGIO is not generated from kernel. The same is for the > > other SI_ consts < 0 not defined with __SI_CODE. > > Ok, then you have already broken binary compatibility between 2.2 and 2.4 Looking in the old kernels, it seems the binary compatibility was broken in 2.3.21 when si_code returns POLL_xxx events just like mentioned in "The Single UNIX ® Specification, Version 2", xsh/signal.h.html and not SI_SIGIO. SI_SIGIO in si_code for 2.2 does not return any information about the events. I even see that Redhat maintains patch against 2.2 to backport the POLL_xxx events from 2.3. Not sure after the changes in 2.4.0-test1. Anyway, 2.2 looks unusable for me and I don't see other way this problem to be fixed. The binary compatibility is impossible to exist. The applications can support the both ways: the old SI_SIGIO and the new POLL_xxx events (recompiled after test1) in si_code. The next step is somebody to implement event merging and to allow receiving of many events with one call. For the next kernels. Regards -- Julian Anastasov <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Fix queued SIGIO
On Mon, Sep 18, 2000 at 08:04:21PM +0200, Julian Anastasov wrote: > > Hello, > > Everything looks good after test1 except the extra > '<< 16' in __SI_CODE. > > SI_SIGIO is not generated from kernel. The same is for the > other SI_ consts < 0 not defined with __SI_CODE. Ok, then you have already broken binary compatibility between 2.2 and 2.4 -Andi - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Fix queued SIGIO
On Mon, Sep 18, 2000 at 08:56:58PM +0200, Jamie Lokier wrote: [...making SI_FROMUSER exclude SI_ASYNCIO and SI_TIMER...] I haven't checked, but I suspect that would break the glibc user space implementations. Overall the concept of kernel reserved numbers doesn't make too much sense as a API because there is always a legitimate need to emulate it in userspace when you have appropiate credentials. It is just a convenient hack to bypass the credentials checking in signal sending for some cases. > > It'll break programs that try to send SI_SIGIO (=-5) signals from userspace, > > but I think that is ok. > > Actually rt_sigqueueinfo has this test hard-coded in it: > > if (info.si_code >= 0) > return -EPERM; > > with a comment "not even root is allowed to send signals from the > kernel". Changing SI_FROMUSER won't affect this. My patch of course changed this line to if (!SI_FROMUSER()), you probably missed that hunk.. There are two approaches: break the programs that expect to parse si_code in signals/sigwaitinfo or break the program that use sigqueueinfo() with arbitary values. I would have expected that the second one is less painless, but it turned out someone already took the first approach for SIGIO in 2.4 by turning si_code into a bastardized si_band (which I don't quite follow, because si_band already exists) -Andi - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Fix queued SIGIO
Linus, please think this over before applying Andi's patch. Andi Kleen wrote: > The problem is really that SI_SIGIO is negative, but it should be positive > to make SI_FROMUSER return false on it. This is an old problem. There was a thread on this topic last March. Look for "accept() improvements for rt signals". SI_SIGIO is not the only signal that's broken: SI_ASYNCIO, SI_TIMER and SI_MESGQ have the same problem. When those signals are used you'll be adding more and more exceptions to the SI_FROMUSER macro. (For example the POSIX timers patch actually does exactly the same as Andi's patch, but for SI_TIMER instead of SI_SIGIO). It's obvious that SI_SIGIO, SI_ASYNCIO, SI_TIMER and SI_MESGQ should all return false from SI_FROMUSER, assuming SI_FROMUSER is the right thing to use in any case. > Changing it would unfortunately break binary compatibility. This patch > instead changes the definition of SI_FROMUSER/KERNEL to check explicitely > for SI_SIGIO and make it appear like a kernel generated signal type. This > prevents send_sig_info from looking at current . That looks like major band-aid. What does SI_FROMUSER mean anyway? I looked it up on the web. It doesn't appear in manual pages on the web in general, and I didn't find it in Single Unix. Let's investigate. /* * si_code values * Digital reserves positive values for kernel-generated signals. */ Hmm... Inherited from Digital Unix perhaps? Let's try Digital UNIX V4.0F... /usr/include/sys/siginfo.h: /* negative si_codes are reserved for user-generated signals */ #define SI_QUEUE-1 /* sent by sigqueue */ #define SI_USER 0 /* sent by kill, sigsend, raise, etc. */ #define SI_NOINFO 1 /* unknown source */ #define SI_TIMER0x10/* sent by timer expiration */ #define SI_ASYNCIO 0x20/* sent by AIO completion */ #define SI_MESGQ0x40/* sent by realtime mesq state transition */ #define SI_FROMUSER(siptr) ((siptr)->si_code <= 0) #define SI_FROMKERNEL(siptr)((siptr)->si_code > 0) Looks like DEC got this right, Linux blindly copied some of the definitions and made up some of the others (by setting SI_TIMER etc. to negative values but keeping the same definition of SI_FROMUSER). _Something_ of binary compatibility will have to be broken to fix this problem. Either change the SI_TIMER etc. signal numbers, or change the SI_FROMUSER macro. Both of can potentially break applications. Changing SI_SIGINFO would obviously be the most serious. I'd posit that changing SI_FROMUSER is the right fix. But changed to include SI_TIMER, SI_MESGQ and SI_ASYNCIO as well. Andi says: > It'll break programs that try to send SI_SIGIO (=-5) signals from userspace, > but I think that is ok. Actually rt_sigqueueinfo has this test hard-coded in it: if (info.si_code >= 0) return -EPERM; with a comment "not even root is allowed to send signals from the kernel". Changing SI_FROMUSER won't affect this. Perhaps it should. Do we consider SI_SIGIO, SI_TIMER etc. to be in the "not even root is allowed" category or not? have a nice day, -- Jamie - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Fix queued SIGIO
Hello, Everything looks good after test1 except the extra '<< 16' in __SI_CODE. SI_SIGIO is not generated from kernel. The same is for the other SI_ consts < 0 not defined with __SI_CODE. We don't need to mention how broken is 2.2 Regards -- Julian Anastasov <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Fix queued SIGIO
On Mon, Sep 18, 2000 at 01:44:04PM +0200, Alan Cox wrote: > > On Mon, Sep 18, 2000 at 12:34:03PM +0200, Alan Cox wrote: > > > > The problem is really that SI_SIGIO is negative, but it should be positive > > > > to make SI_FROMUSER return false on it. > > > > > > > > Changing it would unfortunately break binary compatibility. This patch > > > > > > Why ? > > > > If a program checks info->si_code for incoming signals. > > ok now what does the value the kernel passes have to do with the value we > write on the user stack - at the moment we blindly copy but we could just > use a tiny lookup table to 'dekernelize' the ID. In fact if you picked a bitflag > you could just mask it That would not help much, the user could just still set the non mapped value in sigqueueinfo() [which would need to be forbidden to avoid users faking kernel signals, which my patch exactly does] -Andi -- This is like TV. I don't like TV. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Fix queued SIGIO
> On Mon, Sep 18, 2000 at 12:34:03PM +0200, Alan Cox wrote: > > > The problem is really that SI_SIGIO is negative, but it should be positive > > > to make SI_FROMUSER return false on it. > > > > > > Changing it would unfortunately break binary compatibility. This patch > > > > Why ? > > If a program checks info->si_code for incoming signals. ok now what does the value the kernel passes have to do with the value we write on the user stack - at the moment we blindly copy but we could just use a tiny lookup table to 'dekernelize' the ID. In fact if you picked a bitflag you could just mask it - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Fix queued SIGIO
On Mon, Sep 18, 2000 at 12:34:03PM +0200, Alan Cox wrote: > > The problem is really that SI_SIGIO is negative, but it should be positive > > to make SI_FROMUSER return false on it. > > > > Changing it would unfortunately break binary compatibility. This patch > > Why ? If a program checks info->si_code for incoming signals. -Andi -- This is like TV. I don't like TV. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Fix queued SIGIO
> The problem is really that SI_SIGIO is negative, but it should be positive > to make SI_FROMUSER return false on it. > > Changing it would unfortunately break binary compatibility. This patch Why ? > It'll break programs that try to send SI_SIGIO (=-5) signals from userspace, > but I think that is ok. Why not just add SI_KERNSIGINFO .. and use that ? - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Fix queued SIGIO
Linus, please think this over before applying Andi's patch. Andi Kleen wrote: > The problem is really that SI_SIGIO is negative, but it should be positive > to make SI_FROMUSER return false on it. This is an old problem. There was a thread on this topic last March. Look for "accept() improvements for rt signals". SI_SIGIO is not the only signal that's broken: SI_ASYNCIO, SI_TIMER and SI_MESGQ have the same problem. When those signals are used you'll be adding more and more exceptions to the SI_FROMUSER macro. (For example the POSIX timers patch actually does exactly the same as Andi's patch, but for SI_TIMER instead of SI_SIGIO). It's obvious that SI_SIGIO, SI_ASYNCIO, SI_TIMER and SI_MESGQ should all return false from SI_FROMUSER, assuming SI_FROMUSER is the right thing to use in any case. > Changing it would unfortunately break binary compatibility. This patch > instead changes the definition of SI_FROMUSER/KERNEL to check explicitely > for SI_SIGIO and make it appear like a kernel generated signal type. This > prevents send_sig_info from looking at current . That looks like major band-aid. What does SI_FROMUSER mean anyway? I looked it up on the web. It doesn't appear in manual pages on the web in general, and I didn't find it in Single Unix. Let's investigate. /* * si_code values * Digital reserves positive values for kernel-generated signals. */ Hmm... Inherited from Digital Unix perhaps? Let's try Digital UNIX V4.0F... /usr/include/sys/siginfo.h: /* negative si_codes are reserved for user-generated signals */ #define SI_QUEUE-1 /* sent by sigqueue */ #define SI_USER 0 /* sent by kill, sigsend, raise, etc. */ #define SI_NOINFO 1 /* unknown source */ #define SI_TIMER0x10/* sent by timer expiration */ #define SI_ASYNCIO 0x20/* sent by AIO completion */ #define SI_MESGQ0x40/* sent by realtime mesq state transition */ #define SI_FROMUSER(siptr) ((siptr)->si_code <= 0) #define SI_FROMKERNEL(siptr)((siptr)->si_code > 0) Looks like DEC got this right, Linux blindly copied some of the definitions and made up some of the others (by setting SI_TIMER etc. to negative values but keeping the same definition of SI_FROMUSER). _Something_ of binary compatibility will have to be broken to fix this problem. Either change the SI_TIMER etc. signal numbers, or change the SI_FROMUSER macro. Both of can potentially break applications. Changing SI_SIGINFO would obviously be the most serious. I'd posit that changing SI_FROMUSER is the right fix. But changed to include SI_TIMER, SI_MESGQ and SI_ASYNCIO as well. Andi says: > It'll break programs that try to send SI_SIGIO (=-5) signals from userspace, > but I think that is ok. Actually rt_sigqueueinfo has this test hard-coded in it: if (info.si_code >= 0) return -EPERM; with a comment "not even root is allowed to send signals from the kernel". Changing SI_FROMUSER won't affect this. Perhaps it should. Do we consider SI_SIGIO, SI_TIMER etc. to be in the "not even root is allowed" category or not? have a nice day, -- Jamie - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Fix queued SIGIO
Linus, please think this over before applying Andi's patch. Andi Kleen wrote: The problem is really that SI_SIGIO is negative, but it should be positive to make SI_FROMUSER return false on it. This is an old problem. There was a thread on this topic last March. Look for "accept() improvements for rt signals". SI_SIGIO is not the only signal that's broken: SI_ASYNCIO, SI_TIMER and SI_MESGQ have the same problem. When those signals are used you'll be adding more and more exceptions to the SI_FROMUSER macro. (For example the POSIX timers patch actually does exactly the same as Andi's patch, but for SI_TIMER instead of SI_SIGIO). It's obvious that SI_SIGIO, SI_ASYNCIO, SI_TIMER and SI_MESGQ should all return false from SI_FROMUSER, assuming SI_FROMUSER is the right thing to use in any case. Changing it would unfortunately break binary compatibility. This patch instead changes the definition of SI_FROMUSER/KERNEL to check explicitely for SI_SIGIO and make it appear like a kernel generated signal type. This prevents send_sig_info from looking at current . That looks like major band-aid. What does SI_FROMUSER mean anyway? I looked it up on the web. It doesn't appear in manual pages on the web in general, and I didn't find it in Single Unix. Let's investigate. /* * si_code values * Digital reserves positive values for kernel-generated signals. */ Hmm... Inherited from Digital Unix perhaps? Let's try Digital UNIX V4.0F... /usr/include/sys/siginfo.h: /* negative si_codes are reserved for user-generated signals */ #define SI_QUEUE-1 /* sent by sigqueue */ #define SI_USER 0 /* sent by kill, sigsend, raise, etc. */ #define SI_NOINFO 1 /* unknown source */ #define SI_TIMER0x10/* sent by timer expiration */ #define SI_ASYNCIO 0x20/* sent by AIO completion */ #define SI_MESGQ0x40/* sent by realtime mesq state transition */ #define SI_FROMUSER(siptr) ((siptr)-si_code = 0) #define SI_FROMKERNEL(siptr)((siptr)-si_code 0) Looks like DEC got this right, Linux blindly copied some of the definitions and made up some of the others (by setting SI_TIMER etc. to negative values but keeping the same definition of SI_FROMUSER). _Something_ of binary compatibility will have to be broken to fix this problem. Either change the SI_TIMER etc. signal numbers, or change the SI_FROMUSER macro. Both of can potentially break applications. Changing SI_SIGINFO would obviously be the most serious. I'd posit that changing SI_FROMUSER is the right fix. But changed to include SI_TIMER, SI_MESGQ and SI_ASYNCIO as well. Andi says: It'll break programs that try to send SI_SIGIO (=-5) signals from userspace, but I think that is ok. Actually rt_sigqueueinfo has this test hard-coded in it: if (info.si_code = 0) return -EPERM; with a comment "not even root is allowed to send signals from the kernel". Changing SI_FROMUSER won't affect this. Perhaps it should. Do we consider SI_SIGIO, SI_TIMER etc. to be in the "not even root is allowed" category or not? have a nice day, -- Jamie - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Fix queued SIGIO
On Mon, Sep 18, 2000 at 12:34:03PM +0200, Alan Cox wrote: The problem is really that SI_SIGIO is negative, but it should be positive to make SI_FROMUSER return false on it. Changing it would unfortunately break binary compatibility. This patch Why ? If a program checks info-si_code for incoming signals. -Andi -- This is like TV. I don't like TV. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Fix queued SIGIO
On Mon, Sep 18, 2000 at 12:34:03PM +0200, Alan Cox wrote: The problem is really that SI_SIGIO is negative, but it should be positive to make SI_FROMUSER return false on it. Changing it would unfortunately break binary compatibility. This patch Why ? If a program checks info-si_code for incoming signals. ok now what does the value the kernel passes have to do with the value we write on the user stack - at the moment we blindly copy but we could just use a tiny lookup table to 'dekernelize' the ID. In fact if you picked a bitflag you could just mask it - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Fix queued SIGIO
Hello, Everything looks good after test1 except the extra ' 16' in __SI_CODE. SI_SIGIO is not generated from kernel. The same is for the other SI_ consts 0 not defined with __SI_CODE. We don't need to mention how broken is 2.2 Regards -- Julian Anastasov [EMAIL PROTECTED] - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Fix queued SIGIO
On Mon, Sep 18, 2000 at 08:56:58PM +0200, Jamie Lokier wrote: [...making SI_FROMUSER exclude SI_ASYNCIO and SI_TIMER...] I haven't checked, but I suspect that would break the glibc user space implementations. Overall the concept of kernel reserved numbers doesn't make too much sense as a API because there is always a legitimate need to emulate it in userspace when you have appropiate credentials. It is just a convenient hack to bypass the credentials checking in signal sending for some cases. It'll break programs that try to send SI_SIGIO (=-5) signals from userspace, but I think that is ok. Actually rt_sigqueueinfo has this test hard-coded in it: if (info.si_code = 0) return -EPERM; with a comment "not even root is allowed to send signals from the kernel". Changing SI_FROMUSER won't affect this. My patch of course changed this line to if (!SI_FROMUSER(info)), you probably missed that hunk.. There are two approaches: break the programs that expect to parse si_code in signals/sigwaitinfo or break the program that use sigqueueinfo() with arbitary values. I would have expected that the second one is less painless, but it turned out someone already took the first approach for SIGIO in 2.4 by turning si_code into a bastardized si_band (which I don't quite follow, because si_band already exists) -Andi - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Fix queued SIGIO
On Mon, Sep 18, 2000 at 08:04:21PM +0200, Julian Anastasov wrote: Hello, Everything looks good after test1 except the extra ' 16' in __SI_CODE. SI_SIGIO is not generated from kernel. The same is for the other SI_ consts 0 not defined with __SI_CODE. Ok, then you have already broken binary compatibility between 2.2 and 2.4 -Andi - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
