From owner-freebsd-arch@FreeBSD.ORG Tue May 14 21:36:56 2013 Return-Path: Delivered-To: arch@freebsd.org Received: from mx1.freebsd.org (mx1.FreeBSD.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id 3291FA89 for ; Tue, 14 May 2013 21:36:56 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from bigwig.baldwin.cx (bigwig.baldwin.cx [IPv6:2001:470:1f11:75::1]) by mx1.freebsd.org (Postfix) with ESMTP id 0FF45F39 for ; Tue, 14 May 2013 21:36:56 +0000 (UTC) Received: from John-Baldwins-MacBook-Air.local (unknown [24.114.252.241]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id 5DB12B964; Tue, 14 May 2013 17:36:55 -0400 (EDT) Message-ID: <5192AE7C.10105@FreeBSD.org> Date: Tue, 14 May 2013 17:37:00 -0400 From: John Baldwin User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:17.0) Gecko/20130328 Thunderbird/17.0.5 MIME-Version: 1.0 To: Jilles Tjoelker Subject: Re: Extending MADV_PROTECT 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> In-Reply-To: <20130514192115.GA34869@stack.nl> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.7 (bigwig.baldwin.cx); Tue, 14 May 2013 17:36:55 -0400 (EDT) Cc: Konstantin Belousov , arch@freebsd.org X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 14 May 2013 21:36:56 -0000 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