Re: [PATCH 6/X] uprobes: reimplement xol_add_vma() viainstall_special_mapping()

[Stephen Smalley]

On Mon, 2011-10-17 at 16:20 +0530, Srikar Dronamraju wrote:
> > I apologize in advance if this was already discussed, but I just can't
> > understand why xol_add_vma() does not use install_special_mapping().
> > Unless I missed something this should work and this has the following
> > advantages:
> 
> 
> The override_creds was based on what Stephen Smalley suggested 
> https://lkml.org/lkml/2011/4/20/224    
> 
> At that time Peter had suggested install_special_mapping(). However the
> consensus was to go with Stephen's suggestion of override_creds.
> 
> > 
> > 	- we can avoid override_creds() hacks, install_special_mapping()
> > 	  fools security_file_mmap() passing prot/flags = 0
> > 
> > 	- no need to play with vma after do_mmap_pgoff()
> > 
> > 	- no need for get_user_pages(FOLL_WRITE/FOLL_FORCE) hack
> > 
> > 	- no need for do_munmap() if get_user_pages() fails
> > 
> > 	- this protects us from mprotect(READ/WRITE)
> > 
> > 	- this protects from MADV_DONTNEED, the page will be correctly
> > 	  re-instantiated from area->page
> > 
> > 	- this makes xol_vma more "cheap", swapper can't see this page
> > 	  and we avoid the meaningless add_to_swap/pageout.
> > 
> > 	  Note that, before this patch, area->page can't be removed
> > 	  from the swap cache anyway (we have the reference). And it
> > 	  must not, uprobes modifies this page directly.
> 
> Stephan, Eric, 
> 
> Would you agree with Oleg's observation that we would be better off
> using install_special_mapping rather than using override_creds.
> 
> To give you some more information about the problem.
> 
> Uprobes will be a in-kernel debugging facility that provides
> singlestepping out of line. To achieve this, it will create a per-mm vma
> which is not mapped to any file. However this vma has to be executable.
> 
> Slots are made in this executable vma, and one slot can be used to
> single step a original instruction.
> 
> This executable vma that we are creating is not for any particular
> binary but would have to be created dynamically as and when an
> application is debugged. For example, if we were to debug malloc call in
> libc, we would end up adding xol vma to all the live processes in the
> system.
> 
> Since selinux wasnt happy to have an anonymous vma attached, we would
> create a pseudo file using shmem_file_setup. However after comments from
> Peter and Stephan's suggestions we started using override_creds. Peter and
> Oleg suggest that we use install_special_mapping. 
> 
> Are you okay with using install_special_mapping instead of
> override_creds()?

That's fine with me.  But I'm still not clear on how you are controlling
the use of this facility from userspace, which is my primary concern.
Who gets to enable/disable this facility, and what check is applied
between the process that enables it and the target process(es) that are
affected by it?  Is it subject to the same checks as ptrace?

-- 
Stephen Smalley
National Security Agency

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/