Re: [perfmon2] [PATCH 3/2 -v3] fcntl: F_[SG]ETOWN_EX
Status: Beta
Brought to you by:
seranian
From: Oleg N. <ol...@re...> - 2009-08-04 16:24:39
|
On 08/04, Peter Zijlstra wrote: > > +static int f_setown_ex(struct file *filp, unsigned long arg) > +{ > + struct f_owner_ex * __user owner_p = (void * __user)arg; > + struct f_owner_ex owner; > + struct pid *pid; > + int type; > + int ret; > + > + ret = copy_from_user(&owner, owner_p, sizeof(owner)); > + if (ret) > + return ret; > + > + switch (owner.type) { > + case F_OWNER_TID: > + type = PIDTYPE_MAX; > + break; > + > + case F_OWNER_PID: > + type = PIDTYPE_PID; > + break; > + > + case F_OWNER_GID: > + type = PIDTYPE_PGID; > + break; > + } Note that send_sigio()->do_each_pid_task(type) must use the valid type < PIDTYPE_MAX, or we can crash/etc. This means f_setown_ex() should be careful with the wrong owner->type, the switch() above needs default: return -EINVAL; > + rcu_read_lock(); > + pid = find_vpid(owner.pid); > + ret = __f_setown(filp, pid, type, 1); > + rcu_read_unlock(); > + > + return ret; Perhaps it makes sense to return -ESRCH if owner.pid && !pid, not sure. > @@ -474,16 +540,23 @@ void send_sigio(struct fown_struct *fown > struct task_struct *p; > enum pid_type type; > struct pid *pid; > + int group = 1; > > read_lock(&fown->lock); > + > type = fown->pid_type; > + if (type == PIDTYPE_MAX) { > + group = 0; > + type = PIDTYPE_PID; > + } And send_sigurg() needs the same change. I am not sure we should teach send_sigurg_to_task() to handle the F_OWNER_TID, but we must ensure send_sigurg()->do_each_pid_task() won't be called with PIDTYPE_MAX. Otherwise, personally I think this is what we need to solve the problem. Oleg. |