Re: [announce] [patch] NX (No eXecute) support for x86, 2.6.7-rc2-bk2

From: Ingo Molnar
Date: Fri Jun 04 2004 - 04:27:35 EST



* Andi Kleen <ak@xxxxxxx> wrote:

> > The whole point of NX, though, is that it prevents certain classes of
> > exploits. If a setuid binary is vulnerable to one of these, then Ingo's
> > patch "fixes" it. Your approach breaks that.
>
> Good point.
>
> But that only applies to the NX personality bit. For the uname
> emulation it is not an issue.
>
> So maybe the dropping on exec should only zero a few selected
> personality bits, but not all.

ok, how about the attached patch then? There's a PERS_DROP_ON_SUID mask
that we drop upon setuid - all the other personality bits get inherited.

Ingo

--- linux/include/linux/personality.h.orig
+++ linux/include/linux/personality.h
@@ -30,6 +30,7 @@ extern int abi_fake_utsname;
*/
enum {
MMAP_PAGE_ZERO = 0x0100000,
+ ADDR_SPACE_EXECUTABLE = 0x0200000,
ADDR_LIMIT_32BIT = 0x0800000,
SHORT_INODE = 0x1000000,
WHOLE_SECONDS = 0x2000000,
@@ -37,6 +38,8 @@ enum {
ADDR_LIMIT_3GB = 0x8000000,
};

+#define PERS_DROP_ON_SUID (MMAP_PAGE_ZERO|ADDR_SPACE_EXECUTABLE)
+
/*
* Personality types.
*
--- linux/include/asm-i386/elf.h.orig
+++ linux/include/asm-i386/elf.h
@@ -117,7 +117,8 @@ typedef struct user_fxsr_struct elf_fpxr
#define AT_SYSINFO_EHDR 33

#ifdef __KERNEL__
-#define SET_PERSONALITY(ex, ibcs2) set_personality((ibcs2)?PER_SVR4:PER_LINUX)
+/* child inherits the personality of the parent */
+#define SET_PERSONALITY(ex, ibcs2) do { } while (0)

extern int dump_task_regs (struct task_struct *, elf_gregset_t *);
extern int dump_task_fpu (struct task_struct *, elf_fpregset_t *);
--- linux/fs/exec.c.orig
+++ linux/fs/exec.c
@@ -886,8 +886,11 @@ int prepare_binprm(struct linux_binprm *

if(!(bprm->file->f_vfsmnt->mnt_flags & MNT_NOSUID)) {
/* Set-uid? */
- if (mode & S_ISUID)
+ if (mode & S_ISUID) {
bprm->e_uid = inode->i_uid;
+ /* reset personality */
+ current->personality &= ~PERS_DROP_ON_SUID;
+ }

/* Set-gid? */
/*
@@ -895,8 +898,11 @@ int prepare_binprm(struct linux_binprm *
* is a candidate for mandatory locking, not a setgid
* executable.
*/
- if ((mode & (S_ISGID | S_IXGRP)) == (S_ISGID | S_IXGRP))
+ if ((mode & (S_ISGID | S_IXGRP)) == (S_ISGID | S_IXGRP)) {
bprm->e_gid = inode->i_gid;
+ /* reset personality */
+ current->personality &= ~PERS_DROP_ON_SUID;
+ }
}

/* fill in binprm security blob */
--- linux/fs/binfmt_elf.c.orig
+++ linux/fs/binfmt_elf.c
@@ -490,7 +490,7 @@ static int load_elf_binary(struct linux_
struct exec interp_ex;
char passed_fileno[6];
struct files_struct *files;
- int executable_stack = EXSTACK_DEFAULT;
+ int executable_stack;

/* Get the exec-header */
elf_ex = *((struct elfhdr *) bprm->buf);
@@ -616,13 +616,19 @@ static int load_elf_binary(struct linux_
}

elf_ppnt = elf_phdata;
- for (i = 0; i < elf_ex.e_phnum; i++, elf_ppnt++)
- if (elf_ppnt->p_type == PT_GNU_STACK) {
- if (elf_ppnt->p_flags & PF_X)
- executable_stack = EXSTACK_ENABLE_X;
- else
- executable_stack = EXSTACK_DISABLE_X;
- }
+ if (current->personality & ADDR_SPACE_EXECUTABLE)
+ executable_stack = EXSTACK_ENABLE_X;
+ else {
+ executable_stack = EXSTACK_DEFAULT;
+ for (i = 0; i < elf_ex.e_phnum; i++, elf_ppnt++)
+ if (elf_ppnt->p_type == PT_GNU_STACK) {
+ if (elf_ppnt->p_flags & PF_X)
+ executable_stack = EXSTACK_ENABLE_X;
+ else
+ executable_stack = EXSTACK_DISABLE_X;
+ break;
+ }
+ }

/* Some simple consistency checks for the interpreter */
if (elf_interpreter) {
-
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/