Re: [PATCH][RFC] sigurg/sigio cleanup for 2.5.31

[Jeff Dike <jdike@karaya.com>]

jmorris@intercode.com.au said:
>   o Fixed fowner race (lockless technique suggested by Alan Cox). 

This looks broken to me.  

+static void f_modown(struct file *filp, unsigned long pid,
+                     uid_t uid, uid_t euid)
+{
+	filp->f_owner.pid = PID_INVALID;
+	wmb();
+	filp->f_owner.uid = uid;
+	filp->f_owner.euid = euid;
+	wmb();
+	filp->f_owner.pid = pid;
+}

@@ -469,6 +491,9 @@
 	struct task_struct * p;
 	int   pid	= fown->pid;
 	
+	if (!pid || pid == PID_INVALID)
+		return;
+	

This introduces a window within which SIGIO will be dropped.  As it stands,
this will break UML.  Lost SIGIOs will cause UML hangs.

If you're determined to avoid spinlocks, why not do something like this:

+	if (!pid)
+		return;
+	while(fown->pid == PID_INVALID) ;

maybe with a cpu_relax() in the loop.

But that starts looking a lot like a spinlock.

Also, shouldn't there be a capable(CAP_KILL) in here rather than a check
for uid == 0?

+static inline int sigio_perm(struct task_struct *p,
+                             struct fown_struct *fown)
+{
+	return ((fown->euid == 0) ||
+ 	        (fown->euid == p->suid) || (fown->euid == p->uid) ||
+ 	        (fown->uid == p->suid) || (fown->uid == p->uid));
+}
+

				Jeff