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

From: Jeff Dike (jdike@karaya.com)
Date: Fri Aug 16 2002 - 11:25:54 EST


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

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Fri Aug 23 2002 - 22:00:12 EST