ppc32 selinux mprotect diff.

[Dave Jones]

On Tue, Jan 12, 2010 at 02:24:20PM -0500, Stephen Smalley wrote:
 > On Mon, 2010-01-11 at 14:55 -0500, Dave Jones wrote:
 > > We've carried this diff in Fedora for a few years now..
 > > 
 > > --- linux-2.6.26.noarch/security/selinux/hooks.c~	2008-09-25 14:11:17.000000000 -0400
 > > +++ linux-2.6.26.noarch/security/selinux/hooks.c	2008-09-25 14:12:17.000000000 -0400
 > > @@ -3018,7 +3018,6 @@ static int file_map_prot_check(struct fi
 > >  	const struct cred *cred = current_cred();
 > >  	int rc = 0;
 > >  
 > > -#ifndef CONFIG_PPC32
 > >  	if ((prot & PROT_EXEC) && (!file || (!shared && (prot & PROT_WRITE)))) {
 > >  		/*
 > >  		 * We are making executable an anonymous mapping or a
 > > @@ -3029,7 +3028,6 @@ static int file_map_prot_check(struct fi
 > >  		if (rc)
 > >  			goto error;
 > >  	}
 > > -#endif
 > >  
 > >  	if (file) {
 > >  		/* read access is always possible with a mapping */
 > > @@ -3024,7 +3022,6 @@ static int selinux_file_mprotect(struct 
 > >  	if (selinux_checkreqprot)
 > >  		prot = reqprot;
 > >  
 > > -#ifndef CONFIG_PPC32
 > >  	if ((prot & PROT_EXEC) && !(vma->vm_flags & VM_EXEC)) {
 > >  		rc = 0;
 > >  		if (vma->vm_start >= vma->vm_mm->start_brk &&
 > > @@ -3049,7 +3046,6 @@ static int selinux_file_mprotect(struct 
 > >  		if (rc)
 > >  			return rc;
 > >  	}
 > > -#endif
 > >  
 > >  	return file_map_prot_check(vma->vm_file, prot, vma->vm_flags&VM_SHARED);
 > >  }
 > > 
 > > 
 > > 
 > > This needs a fixed toolchain, and a userspace rebuild to work.
 > > For these reasons, it's had difficulty getting upstream.
 > > 
 > > Fedora has a new enough toolchain, and has been rebuilt, so we don't need
 > > the ifdefs.  Other distros don't/haven't, and this patch would break them
 > > if pushed upstream.
 > > 
 > > Could we do something like the (untested) diff below instead,
 > > which might be more palatable to upstream, allowing us to stop
 > > carrying it ?
 > > 
 > > 	Dave
 > > 
 > > diff --git a/security/selinux/Kconfig b/security/selinux/Kconfig
 > > index bca1b74..83a9675 100644
 > > --- a/security/selinux/Kconfig
 > > +++ b/security/selinux/Kconfig
 > > @@ -131,3 +131,10 @@ config SECURITY_SELINUX_POLICYDB_VERSION_MAX_VALUE
 > >  	  installed under /etc/selinux/$SELINUXTYPE/policy, where
 > >  	  SELINUXTYPE is defined in your /etc/selinux/config.
 > >  
 > > +config SELINUX_NEW_ENOUGH_TOOLCHAIN
 > > +	bool "SELinux mprotect checks"
 > > +	default n if PPC32
 > > +	help
 > > +	  This option requires a modern toolchain (FIXME: Version?)
 > > +	  and a userspace rebuild to work.
 > > +
 > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
 > > index 9a2ee84..e805df7 100644
 > > --- a/security/selinux/hooks.c
 > > +++ b/security/selinux/hooks.c
 > > @@ -3009,7 +3009,7 @@ static int file_map_prot_check(struct file *file, unsigned long prot, int shared
 > >  	const struct cred *cred = current_cred();
 > >  	int rc = 0;
 > >  
 > > -#ifndef CONFIG_PPC32
 > > +#ifdef CONFIG_SELINUX_NEW_ENOUGH_TOOLCHAIN
 > >  	if ((prot & PROT_EXEC) && (!file || (!shared && (prot & PROT_WRITE)))) {
 > >  		/*
 > >  		 * We are making executable an anonymous mapping or a
 > > @@ -3081,7 +3081,7 @@ static int selinux_file_mprotect(struct vm_area_struct *vma,
 > >  	if (selinux_checkreqprot)
 > >  		prot = reqprot;
 > >  
 > > -#ifndef CONFIG_PPC32
 > > +#ifdef CONFIG_SELINUX_NEW_ENOUGH_TOOLCHAIN
 > >  	if ((prot & PROT_EXEC) && !(vma->vm_flags & VM_EXEC)) {
 > >  		int rc = 0;
 > >  		if (vma->vm_start >= vma->vm_mm->start_brk &&
 > 
 > IMHO, we should just do one of the following:
 > 1) Push the original diff (i.e. drop the #ifndef CONFIG_PPC32) upstream
 > and see if anyone complains.
 > -or-
 > 2) Drop the diff altogether and forget about exec* checking on ppc32.
 > 
 > I wouldn't favor the new option.

So I remembered this thread yesterday, after spot posted a sparc variant
http://marc.info/?l=linux-sparc&m=126946771412359&w=2
which adds an additional ifdef clause.

any further thoughts on cleaning up this mess?
option 2 seems not practical if spot wants to *add* the new ifdef.

	Dave