On Thu, 2010-03-25 at 15:59 -0400, Dave Jones wrote:
> 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.
I think we can upstream the original diff for ppc32 (removing the
#ifndef CONFIG_PPC32), i.e. re-enable all exec* checks on ppc32, since
that was supposedly fixed in the ppc32 toolchain quite some time ago.
For sparc, assuming that we can't just as easily fix the toolchain there
(I don't know - has anyone filed a bug on the RWE segments in sparc
binaries?), perhaps a compromise would be to disable execmem checking on
private file mappings but retain it on anonymous mappings and retain the
other exec* checks.
Like so:
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 63c2d36..9a4d0e4 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -3004,8 +3004,11 @@ 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
+#if defined(CONFIG_SPARC)
+ if ((prot & PROT_EXEC) && !file) {
+#else
if ((prot & PROT_EXEC) && (!file || (!shared && (prot & PROT_WRITE)))) {
+#endif
/*
* We are making executable an anonymous mapping or a
* private file mapping that will also be writable.
@@ -3015,7 +3018,6 @@ static int file_map_prot_check(struct file *file, unsigned long prot, int shared
if (rc)
goto error;
}
-#endif
if (file) {
/* read access is always possible with a mapping */
@@ -3076,7 +3078,6 @@ static int selinux_file_mprotect(struct vm_area_struct *vma,
if (selinux_checkreqprot)
prot = reqprot;
-#ifndef CONFIG_PPC32
if ((prot & PROT_EXEC) && !(vma->vm_flags & VM_EXEC)) {
int rc = 0;
if (vma->vm_start >= vma->vm_mm->start_brk &&
@@ -3099,7 +3100,6 @@ static int selinux_file_mprotect(struct vm_area_struct *vma,
if (rc)
return rc;
}
-#endif
return file_map_prot_check(vma->vm_file, prot, vma->vm_flags&VM_SHARED);
}
--
Stephen Smalley
National Security Agency
_______________________________________________
kernel mailing list
kernel@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/kernel
