linux-kernel - Re: [PATCH 3/2 -v3] fcntl: F_[SG]ETOWN

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 4 Aug 2009 18:20:33 +0200
From:	Oleg Nesterov <oleg@...hat.com>
To:	Peter Zijlstra <a.p.zijlstra@...llo.nl>
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 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.

--
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/