Date: Fri, 6 Sep 2013 17:21:28 -0400 From: John Baldwin <jhb@freebsd.org> To: Konstantin Belousov <kostikbel@gmail.com>, "Robert N. M. Watson" <rwatson@freebsd.org> Cc: Jilles Tjoelker <jilles@stack.nl>, freebsd-arch@freebsd.org Subject: Re: Extending MADV_PROTECT Message-ID: <201309061721.28175.jhb@freebsd.org> In-Reply-To: <20130716171648.GI5991@kib.kiev.ua> References: <201305071433.27993.jhb@freebsd.org> <201307161130.40737.jhb@freebsd.org> <20130716171648.GI5991@kib.kiev.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tuesday, July 16, 2013 1:16:48 pm Konstantin Belousov wrote: > On Tue, Jul 16, 2013 at 11:30:40AM -0400, John Baldwin wrote: > > On Saturday, July 13, 2013 1:58:35 pm Konstantin Belousov wrote: > > > On Fri, Jul 12, 2013 at 05:48:57PM -0400, John Baldwin wrote: > > > > On Friday, June 28, 2013 2:46:01 pm John Baldwin wrote: > > > > > Ok, there isn't really a clear consensus here, but I need a system call to let > > > > > me toggle this flag on existing processes. > > > > > > > > > > One reason I don't like the procctl() approach is I am uneasy about forcing > > > > > a certain behavior for how commands treat pgid (first-fail vs best-effort). > > > > > I guess it can always change in the future so that isn't completely unsolvable. > > > > > > > > > > I guess I am fine just making it use hardcoded sizes instead of full-blown > > > > > ioctl encoding. > > > > > > > > Ok, I have updated patches for this for HEAD. I have not yet implemented the > > > > inheritance bits because I'm loathe to add the first bit to a p_flag2. :-P > > > > However, if that's the best course of action I suppose we can do that. > > > > > > > > The kernel patch is at www.freebsd.org/~jhb/patches/procctl.patch > > > > > > > > The patch for the protect binary is at www.freebsd.org/~jhb/patches/protect.patch > > > > > > > > > > It seems that p_cansee() is called twice, once in kern_procctl(), and > > > then in protect_setchild(). > > > > Yes, this is because protect_setchild() can descend to child processes, and > > you might not be able to "see" a child process if it exec'd a suid binary, etc. > > > > > Is AUE_WAIT6 the correct audit event id for procctl ? > > > > Nope. (It probably needs a new one?) > Yes, probably needs a new id. I punted on this for now. > > > I thought proposing to use pget() for P_PID case in kern_procctl(), but > > > indeed open coding of the process lookup is easier, since otherwise > > > you would need to move proctree_lock acquisition to P_PGID. > > > > > > Why do you need PPROT_CLEAR ? If you do need the flag, would it be better > > > to assign a non-zero value to it ? > > > > I need it for 'protect -c' which is similar to 'ktrace -C'. That is, to > > allow protection to be removed from existing processes. I added a constant > > for it to make the code clear as the caller should pick one of PPROT_SET > > or PPROT_CLEAR (kind of like MAP_PRIVATE vs MAP_SHARED for mmap(2)). Otherwise > > you would have 'procctl(..., PROC_SPROTECT, 0)' which is not as obvious > > to me as ', PPROT_CLEAR)'. Also, you can do 'PPROT_CLEAR | PPROT_DESCEND' > > to clear it for all descendants. Having 'PPROT_SET | PPROT_DESCEND' to set > > it for descendenats and just 'PPROT_DESCEND' to clear for descendants doesn't > > seem as readable. > Well, I think that assigning non-zero value is justified. I went with a non-zero value. > > Do you have any thoughts on p_flag2 (vs some other approach, possibly I > > should create a new field for oom-specific flags instead since we may > > eventually store a priority there?) > > p_flag2 is inevitable, it seems. It would be added now, or, even if you > could avoid this, in the nearest future. I do not like mixing a flag > and a priority-like field. Ok, I've added p_flag2. The updated patch (kernel bits) is at http://people.freebsd.org/~jhb/patches/procctl2.patch -- John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201309061721.28175.jhb>