[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <1249404763.4762.26.camel@laptop>
Date: Tue, 04 Aug 2009 18:52:43 +0200
From: Peter Zijlstra <a.p.zijlstra@...llo.nl>
To: Oleg Nesterov <oleg@...hat.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>, eranian@...il.com,
mingo@...e.hu, linux-kernel@...r.kernel.org, tglx@...utronix.de,
robert.richter@....com, paulus@...ba.org, andi@...stfloor.org,
mpjohn@...ibm.com, cel@...ibm.com, cjashfor@...ibm.com,
mucci@...s.utk.edu, terpstra@...s.utk.edu,
perfmon2-devel@...ts.sourceforge.net, mtk.manpages@...glemail.com,
roland@...hat.com
Subject: Re: [PATCH 3/2 -v3] fcntl: F_[SG]ETOWN_EX
On Tue, 2009-08-04 at 18:20 +0200, Oleg Nesterov wrote:
> 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;
D'0h very true.
> > + 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.
We'd need that case to unset/clear the owner, so returning -ESRCH might
confuse users I think.
> > @@ -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.
How about the below delta, it changes send_sigurg_to_task() to also use
do_send_sig_info() which looses the check_kill_permission() check, but
your previous changes lost that same thing from SIGIO -- so I'm hoping
that's ok.
> Otherwise, personally I think this is what we need to solve the problem.
yay!
I'll look over the bits again and send out a -v4 later today.
----
Index: linux-2.6/fs/fcntl.c
===================================================================
--- linux-2.6.orig/fs/fcntl.c
+++ linux-2.6/fs/fcntl.c
@@ -287,6 +287,9 @@ static int f_setown_ex(struct file *filp
case F_OWNER_GID:
type = PIDTYPE_PGID;
break;
+
+ default:
+ return -EINVAL;
}
rcu_read_lock();
@@ -564,10 +567,10 @@ void send_sigio(struct fown_struct *fown
}
static void send_sigurg_to_task(struct task_struct *p,
- struct fown_struct *fown)
+ struct fown_struct *fown, int group)
{
if (sigio_perm(p, fown, SIGURG))
- group_send_sig_info(SIGURG, SEND_SIG_PRIV, p);
+ do_send_sig_info(SIGURG, SEND_SIG_PRIV, p, group);
}
int send_sigurg(struct fown_struct *fown)
@@ -575,10 +578,17 @@ int send_sigurg(struct fown_struct *fown
struct task_struct *p;
enum pid_type type;
struct pid *pid;
+ int group = 1;
int ret = 0;
read_lock(&fown->lock);
+
type = fown->pid_type;
+ if (type == PIDTYPE_MAX) {
+ group = 0;
+ type = PIDTYPE_PID;
+ }
+
pid = fown->pid;
if (!pid)
goto out_unlock_fown;
@@ -587,7 +597,7 @@ int send_sigurg(struct fown_struct *fown
read_lock(&tasklist_lock);
do_each_pid_task(pid, type, p) {
- send_sigurg_to_task(p, fown);
+ send_sigurg_to_task(p, fown, group);
} while_each_pid_task(pid, type, p);
read_unlock(&tasklist_lock);
out_unlock_fown:
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists