[RFC][PATCH] SI_ASYNCIO: should be a kernel signal ?

[RFC][PATCH] SI_ASYNCIO: should be a kernel signal ?

[Sukadev Bhattiprolu]

From: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Date: Fri, 19 Dec 2008 20:45:49 -0800
Subject: [RFC][PATCH] SI_ASYNCIO: should be a kernel signal ?

SI_ASYNCIO is currently defined as -4 in the kernel. This makes it appear
like a "user" signal (i.e SI_FROMUSER() is true). SI_ASYNCIO is generated
by the kernel for async events like SI_MESGQ and SI_POLL, but unlike
SI_ASYNCIO, SI_MESGQ and SI_POLL are "kernel" signals (i.e SI_FROMKERNEL()
is true).

Shouldn't SI_ASYNCIO be a "kernel" signal too ? It is currently generated
from USB core code.

This quick/untested RFC patch changes the in-kernel value of SI_ASYNCIO
as follows so that it becomes a "kernel" signal.

	(7 << 16)|(-4 & 0xffff) = 0x7fffc which is SI_FROMKERNEL().

The user-space value of SI_ASYNCIO continues to be -4.

Known side-effects:

	Is this required to be SI_FROMUSER() to enable the uid checks in
	kill_pid_info_as_uid() ? Also, changing to "kernel" signal would skip
	the permission checks in check_kill_permission().  Would that be a
	problem ?

Why bother now ? (Sigh. Condensed long story)

	Besides the consistency with SI_POLL and SI_MESGQ this could simplify
	implementation of special signal semantics for container-init.  When a
	signal is sent to container-init from user-space, we need to check the
	pid namespace of the sender in send_signal(). But since send_signal()
	can also be called from interrupt context,  we have no way of knowing
	if it is safe to check the pid namespace of the caller.

	If SI_ASYNCIO signal appears as a kernel signal, we could possibly use
	SI_FROMUSER() to check if it safe to reference the pid namespace of
	the sender.

	If this change has no other side-effects/breakage we will explore this
	path further for the signal semantics for container-init. (There could
	be other hurdles along the way...) 

	See also http://lkml.org/lkml/2008/12/20/183

Appreciate any comments on this.

TODO:
	If this makes sense, make corresponding change to the SI_ASYNCIO
	in arch/mips/siginfo.h.

	SI_DETHREAD and SI_SIGIO are currently unused in the kernel. Should
	we similarly make them "kernel" signals too ?

---
 include/asm-generic/siginfo.h |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/include/asm-generic/siginfo.h b/include/asm-generic/siginfo.h
index 9695701..7b69598 100644
--- a/include/asm-generic/siginfo.h
+++ b/include/asm-generic/siginfo.h
@@ -124,6 +124,7 @@ typedef struct siginfo {
 #define __SI_CHLD	(4 << 16)
 #define __SI_RT		(5 << 16)
 #define __SI_MESGQ	(6 << 16)
+#define __SI_ASYNCIO	(7 << 16)
 #define __SI_CODE(T,N)	((T) | ((N) & 0xffff))
 #else
 #define __SI_KILL	0
@@ -133,6 +134,7 @@ typedef struct siginfo {
 #define __SI_CHLD	0
 #define __SI_RT		0
 #define __SI_MESGQ	0
+#define __SI_ASYNCIO	0
 #define __SI_CODE(T,N)	(N)
 #endif
 
@@ -145,7 +147,7 @@ typedef struct siginfo {
 #define SI_QUEUE	-1		/* sent by sigqueue */
 #define SI_TIMER __SI_CODE(__SI_TIMER,-2) /* sent by timer expiration */
 #define SI_MESGQ __SI_CODE(__SI_MESGQ,-3) /* sent by real time mesq state change */
-#define SI_ASYNCIO	-4		/* sent by AIO completion */
+#define SI_ASYNCIO __SI_CODE(__SI_ASYNCIO, -4)	/* sent by AIO completion */
 #define SI_SIGIO	-5		/* sent by queued SIGIO */
 #define SI_TKILL	-6		/* sent by tkill system call */
 #define SI_DETHREAD	-7		/* sent by execve() killing subsidiary threads */
-- 
1.5.2.5

Re: [RFC][PATCH] SI_ASYNCIO: should be a kernel signal ?

[Sukadev Bhattiprolu]

Sukadev Bhattiprolu [sukadev@linux.vnet.ibm.com] wrote:
| 
| From: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
| Date: Fri, 19 Dec 2008 20:45:49 -0800
| Subject: [RFC][PATCH] SI_ASYNCIO: should be a kernel signal ?
| 
| SI_ASYNCIO is currently defined as -4 in the kernel. This makes it appear
| like a "user" signal (i.e SI_FROMUSER() is true). SI_ASYNCIO is generated
| by the kernel for async events like SI_MESGQ and SI_POLL, but unlike
| SI_ASYNCIO, SI_MESGQ and SI_POLL are "kernel" signals (i.e SI_FROMKERNEL()
| is true).
| 
| Shouldn't SI_ASYNCIO be a "kernel" signal too ? It is currently generated
| from USB core code.
| 
| This quick/untested RFC patch changes the in-kernel value of SI_ASYNCIO
| as follows so that it becomes a "kernel" signal.
| 
| 	(7 << 16)|(-4 & 0xffff) = 0x7fffc which is SI_FROMKERNEL().
| 
| The user-space value of SI_ASYNCIO continues to be -4.
| 
| Known side-effects:
| 
| 	Is this required to be SI_FROMUSER() to enable the uid checks in
| 	kill_pid_info_as_uid() ? Also, changing to "kernel" signal would skip
| 	the permission checks in check_kill_permission().  Would that be a
| 	problem ?
| 
| Why bother now ? (Sigh. Condensed long story)
| 
| 	Besides the consistency with SI_POLL and SI_MESGQ this could simplify
| 	implementation of special signal semantics for container-init.  When a
| 	signal is sent to container-init from user-space, we need to check the
| 	pid namespace of the sender in send_signal(). But since send_signal()
| 	can also be called from interrupt context,  we have no way of knowing
| 	if it is safe to check the pid namespace of the caller.
| 
| 	If SI_ASYNCIO signal appears as a kernel signal, we could possibly use
| 	SI_FROMUSER() to check if it safe to reference the pid namespace of
| 	the sender.
| 
| 	If this change has no other side-effects/breakage we will explore this
| 	path further for the signal semantics for container-init. (There could
| 	be other hurdles along the way...) 
| 
| 	See also http://lkml.org/lkml/2008/12/20/183
| 
| Appreciate any comments on this.
| 
| TODO:
| 	If this makes sense, make corresponding change to the SI_ASYNCIO
| 	in arch/mips/siginfo.h.

Grr. Importantly, we would also need to update the copy_siginfo*() and
friends for this to be complete.

Re: [RFC][PATCH] SI_ASYNCIO: should be a kernel signal ?

[Roland McGrath]

The value of SI_ASYNCIO is part of the user ABI already, so you
can't really change it anyway.  But, there is a logic why it was
correct for it to be SI_FROMUSER (not just an old choice we are
stuck with, which it also is).

The real purpose of the SI_FROMUSER/SI_FROMKERNEL distinction is not
an abstract idea of "kernel-generated or not".  In fact, it's purely
to distinguish "forgeable" from "guaranteed exact kernel semantics".
SI_ASYNCIO is a notification about something the user asked for
specially, not one about any kernel-enforced sort of event.  If the
user wants to send himself signals with si_code=SI_ASYNCIO, then fine.
Noone's going to be fooled about who instigated such a signal.

In fact, the main use of SI_ASYNCIO signals in practice is for a user
to send them to himself.  That's what glibc (-lrt) does to implement
the POSIX aio_* calls.  The only uses in the kernel are in the USB
code related to some magic ioctls.

Those in-kernel cases call kill_pid_info_as_uid precisely for the same
issue of current vs instigating-process.  These are the only calls to
that function.  It really could be called something else like
"kill_self_from_interrupt", because its only use is this case that's
considered to be, "I'm sending it to myself, delayed."  That is, it
caches the uid/euid along with the struct pid to itself, in case of
instigating it, changing uids, and then having the interrupt deliver
the signal to the same process that is now differently-privileged.

It's not real clear you care about the old vs new pid namespace the
same way we care about the old vs new current_cred.  Since the sender
can only be "myself" or "my former self", maybe just call it "myself"
and be done with it--you just don't need any check in this path.


Thanks,
Roland