Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 14 May 2013 17:37:00 -0400
From:      John Baldwin <jhb@FreeBSD.org>
To:        Jilles Tjoelker <jilles@stack.nl>
Cc:        Konstantin Belousov <kostikbel@gmail.com>, arch@freebsd.org
Subject:   Re: Extending MADV_PROTECT
Message-ID:  <5192AE7C.10105@FreeBSD.org>
In-Reply-To: <20130514192115.GA34869@stack.nl>
References:  <201305071433.27993.jhb@freebsd.org> <201305090814.52166.jhb@freebsd.org> <20130509123147.GT3047@kib.kiev.ua> <201305101535.50633.jhb@freebsd.org> <20130514192115.GA34869@stack.nl>

next in thread | previous in thread | raw e-mail | index | archive | help
On 5/14/13 3:21 PM, Jilles Tjoelker wrote:
> On Fri, May 10, 2013 at 03:35:50PM -0400, John Baldwin wrote:
>> [snip]
>> +int
>> +kern_procctl(struct thread *td, idtype_t idtype, id_t id, u_long com,
>> +    void *data)
>> +{
>> [snip]
>> +	case P_PGID:
>> +		/*
>> +		 * Attempt to apply the operation to all members of the
>> +		 * group.  Ignore processes in the group that can't be
>> +		 * seen.  Stop on the first error encountered.
>> +		 */
>> +		pg = pgfind(id);
>> +		if (pg == NULL) {
>> +			error = ESRCH;
>> +			break;
>> +		}
>> +		PGRP_UNLOCK(pg);
>> +		error = ESRCH;
>> +		LIST_FOREACH(p, &pg->pg_members, p_pglist) {
>> +			PROC_LOCK(p);
>> +			if (p->p_state == PRS_NEW ||
>> +			    p_cansee(td, p) != 0) {
>> +				PROC_UNLOCK(p);
>> +				continue;
>> +			}
>> +			error = kern_procctl_single(td, p, com, data);
>> +			PROC_UNLOCK(p);
>> +			if (error)
>> +				break;
>> +		}
>> +		break;
> 
> I think it does not really make sense that the set of affected processes
> depends on the order in &pg->pg_members.
> 
> Comparing other functions, kill() returns success if it could signal any
> process (even it could not signal other processes matched by the
> argument). This is also most consistent with general POSIX/Unix
> philosophy that a function only fails if it committed no change (but
> there are various places where this is not the case). On the other hand,
> setpriority() affects all matches processes it can but returns an error
> if any one fails, even if some other process was affected.
> 
> All this is not very important for process protection because it
> requires root privileges anyway but future procctl commands may well be
> accessible to normal users (I'm thinking of avoiding proliferation of
> pd* calls in particular).

I originally used that approach in pprotect() since that is what ktrace
uses.  I did it this way in procctl() to err on the side of reporting
errors vs not, but I can easily change it.  This is something I wasn't
sure of and very much appreciate feedback on.

Do you have any thoughts on having this be more ioctl-like ("automatic"
copyin/out and size encoded in cmd) vs ptrace-like (explicit sizes and
in/out keyed off of command)?

-- 
John Baldwin



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?5192AE7C.10105>