[CRIU] [PATCH] report the potential selinux problem if mmap_seized() fails

[Oleg Nesterov]

On 04/22, Pavel Emelyanov wrote:
>
> On 04/22/2015 12:41 PM, Cyrill Gorcunov wrote:
> > On Wed, Apr 22, 2015 at 12:39:49PM +0300, Pavel Emelyanov wrote:
> >> On 04/21/2015 08:16 PM, Oleg Nesterov wrote:
> >>> selinux can deny mmap(PROT_WRITE | PROT_EXEC) and in this case it is
> >>> not clear why CRIU fails, "Can't allocate memory for parasite blob"
> >>> doesn't tell too much. Add a pr_warn() hint for the user.
> >>>
> >>> Signed-off-by: Oleg Nesterov <oleg at redhat.com>
> >>> ---
> >>>  arch/x86/crtools.c |   11 +++++++++--
> >>>  1 files changed, 9 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/arch/x86/crtools.c b/arch/x86/crtools.c
> >>> index 02ce2e5..06d824a 100644
> >>> --- a/arch/x86/crtools.c
> >>> +++ b/arch/x86/crtools.c
> >>> @@ -445,8 +445,15 @@ void *mmap_seized(struct parasite_ctl *ctl,
> >>>
> >>>  	err = syscall_seized(ctl, __NR_mmap, &map,
> >>>  			(unsigned long)addr, length, prot, flags, fd, offset);
> >>> -	if (err < 0 || map > TASK_SIZE)
> >>> -		map = 0;
> >>> +	if (err < 0)
> >>> +		return NULL;
> >>> +
> >>> +	if (IS_ERR_VALUE(map)) {
> >>
> >> The IS_ERR_VALUE is only a subset of > TASK_SIZE, so this change is
> >> not 100% correct. In particular the map > TASK_SIZE check was added
> >> for ARM and is important there.
> >
> > It's not, it's x86 specific code
>
> Hm, indeed. But IS_ERR_VALUE vs > TASK_SIZE is not equal there too.
> Hopefully we're OK with this, I'll apply the patch then :)

Yes, thanks Cyrill, IS_ERR_VALUE() should work.

I did this s/TASK_SIZE/IS_ERR_VALUE/ change for cleanups puproses but
forgot to mention this in the changelog.

To me, "map > TASK_SIZE" looks confusing and almost wrong even if the
code works correctly. If err == 0, then map == regs.ax from syscall,
and sys_mmap() can only return the valid addr < TASK_SIZE or the valid
error code so that IS_ERR_VALUE() must be true.

Oleg.