* [PATCH 1/3] fs/binfmts: Add a name field to the binfmt struct
@ 2013-07-25 15:40 Zach Levis
2013-07-25 15:40 ` [PATCH 2/3] fs/binfmts: Better handling of binfmt loops Zach Levis
` (17 more replies)
0 siblings, 18 replies; 35+ messages in thread
From: Zach Levis @ 2013-07-25 15:40 UTC (permalink / raw)
To: linux-fsdevel; +Cc: linux-kernel, viro, akpm, Zach Levis, Zach Levis
Adding the name field helps when printing error messages referring to
specific binfmts
Signed-off-by: Zach Levis <zach@zachsthings.com>
Signed-off-by: Zach Levis <zml@linux.vnet.ibm.com>
---
fs/binfmt_aout.c | 1 +
fs/binfmt_elf.c | 1 +
fs/binfmt_elf_fdpic.c | 1 +
fs/binfmt_em86.c | 1 +
fs/binfmt_flat.c | 1 +
fs/binfmt_misc.c | 1 +
fs/binfmt_script.c | 1 +
fs/binfmt_som.c | 1 +
include/linux/binfmts.h | 1 +
9 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/fs/binfmt_aout.c b/fs/binfmt_aout.c
index bce8769..f6bed04 100644
--- a/fs/binfmt_aout.c
+++ b/fs/binfmt_aout.c
@@ -115,6 +115,7 @@ end_coredump:
#endif
static struct linux_binfmt aout_format = {
+ .name = "a.out",
.module = THIS_MODULE,
.load_binary = load_aout_binary,
.load_shlib = load_aout_library,
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index f8a0b0e..2533969 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -75,6 +75,7 @@ static int elf_core_dump(struct coredump_params *cprm);
#define ELF_PAGEALIGN(_v) (((_v) + ELF_MIN_ALIGN - 1) & ~(ELF_MIN_ALIGN - 1))
static struct linux_binfmt elf_format = {
+ .name = "elf",
.module = THIS_MODULE,
.load_binary = load_elf_binary,
.load_shlib = load_elf_library,
diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c
index c166f32..6be4448 100644
--- a/fs/binfmt_elf_fdpic.c
+++ b/fs/binfmt_elf_fdpic.c
@@ -81,6 +81,7 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm);
#endif
static struct linux_binfmt elf_fdpic_format = {
+ .name = "FDPIC ELF",
.module = THIS_MODULE,
.load_binary = load_elf_fdpic_binary,
#ifdef CONFIG_ELF_CORE
diff --git a/fs/binfmt_em86.c b/fs/binfmt_em86.c
index 037a3e2..8404cdd 100644
--- a/fs/binfmt_em86.c
+++ b/fs/binfmt_em86.c
@@ -93,6 +93,7 @@ static int load_em86(struct linux_binprm *bprm)
}
static struct linux_binfmt em86_format = {
+ .name = "em86",
.module = THIS_MODULE,
.load_binary = load_em86,
};
diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
index d50bbe5..a7641e1 100644
--- a/fs/binfmt_flat.c
+++ b/fs/binfmt_flat.c
@@ -92,6 +92,7 @@ static int load_flat_binary(struct linux_binprm *);
static int flat_core_dump(struct coredump_params *cprm);
static struct linux_binfmt flat_format = {
+ .name = "flat",
.module = THIS_MODULE,
.load_binary = load_flat_binary,
.core_dump = flat_core_dump,
diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c
index 1c740e1..d74afc0 100644
--- a/fs/binfmt_misc.c
+++ b/fs/binfmt_misc.c
@@ -694,6 +694,7 @@ static struct dentry *bm_mount(struct file_system_type *fs_type,
}
static struct linux_binfmt misc_format = {
+ .name = "misc",
.module = THIS_MODULE,
.load_binary = load_misc_binary,
};
diff --git a/fs/binfmt_script.c b/fs/binfmt_script.c
index 5027a3e..53f3c17 100644
--- a/fs/binfmt_script.c
+++ b/fs/binfmt_script.c
@@ -99,6 +99,7 @@ static int load_script(struct linux_binprm *bprm)
}
static struct linux_binfmt script_format = {
+ .name = "script",
.module = THIS_MODULE,
.load_binary = load_script,
};
diff --git a/fs/binfmt_som.c b/fs/binfmt_som.c
index 4e00ed6..1c3ccc5 100644
--- a/fs/binfmt_som.c
+++ b/fs/binfmt_som.c
@@ -53,6 +53,7 @@ static int som_core_dump(struct coredump_params *cprm);
#define SOM_PAGEALIGN(_v) (((_v) + SOM_PAGESIZE - 1) & ~(SOM_PAGESIZE - 1))
static struct linux_binfmt som_format = {
+ .name = "SOM",
.module = THIS_MODULE,
.load_binary = load_som_binary,
.load_shlib = load_som_library,
diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
index 70cf138..402a74a 100644
--- a/include/linux/binfmts.h
+++ b/include/linux/binfmts.h
@@ -70,6 +70,7 @@ struct coredump_params {
struct linux_binfmt {
struct list_head lh;
struct module *module;
+ const char *name;
int (*load_binary)(struct linux_binprm *);
int (*load_shlib)(struct file *);
int (*core_dump)(struct coredump_params *cprm);
--
1.7.1
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH 2/3] fs/binfmts: Better handling of binfmt loops
2013-07-25 15:40 [PATCH 1/3] fs/binfmts: Add a name field to the binfmt struct Zach Levis
@ 2013-07-25 15:40 ` Zach Levis
2013-07-30 21:04 ` Andrew Morton
2013-07-25 15:40 ` [PATCH 3/3] fs/binfmts: Whitespace fixes with scripts/cleanfile Zach Levis
` (16 subsequent siblings)
17 siblings, 1 reply; 35+ messages in thread
From: Zach Levis @ 2013-07-25 15:40 UTC (permalink / raw)
To: linux-fsdevel; +Cc: linux-kernel, viro, akpm, Zach Levis, Zach Levis
With these changes, when a binfmt loop is encountered,
the ELOOP will propogate back to the 0 depth. At this point the
argv and argc values will be reset to what they were originally and an
attempt is made to continue with the following binfmt handlers.
Caveats:
- binfmt_misc is skipped as a whole. To allow for a more generic
solution that works for any binfmt without a lot of duplicated code
and added complexity, the error detection code is applied on the
binfmt level.
- This is a fallback solution. It attempts to restore the state to allow
executing most binaries without side effects, but it may not work for
everything and should not be depended on for regular usage.
My (rough, but functional) test scripts for this issue are available at:
https://gist.github.com/zml2008/6075418
Signed-off-by: Zach Levis <zach@zachsthings.com>
Signed-off-by: Zach Levis <zml@linux.vnet.ibm.com>
---
fs/exec.c | 30 +++++++++++++++++++++++++++++-
include/linux/binfmts.h | 5 ++++-
2 files changed, 33 insertions(+), 2 deletions(-)
diff --git a/fs/exec.c b/fs/exec.c
index ffd7a81..fa59f05 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1401,13 +1401,40 @@ int search_binary_handler(struct linux_binprm *bprm)
if (!try_module_get(fmt->module))
continue;
read_unlock(&binfmt_lock);
+ bprm->previous_binfmts[1] = bprm->previous_binfmts[0];
+ bprm->previous_binfmts[0] = fmt;
+
bprm->recursion_depth = depth + 1;
retval = fn(bprm);
bprm->recursion_depth = depth;
+ if (retval == -ELOOP && depth == 0) { /* cur, previous */
+ pr_err("Too much recursion with binfmts (0:%s, -1:%s) in file %s, skipping (base %s).\n",
+ bprm->previous_binfmts[0]->name,
+ bprm->previous_binfmts[1]->name,
+ bprm->filename,
+ fmt->name);
+
+ /* Put argv back in its place */
+ while (bprm->argc > 0) {
+ retval = remove_arg_zero(bprm);
+ if (retval)
+ return retval;
+ }
+
+ copy_strings(bprm->argc_orig, *((struct user_arg_ptr *) bprm->argv_orig), bprm);
+ bprm->argc = bprm->argc_orig;
+ retval = -ENOEXEC;
+ continue;
+ }
+
if (retval >= 0) {
if (depth == 0) {
trace_sched_process_exec(current, old_pid, bprm);
ptrace_event(PTRACE_EVENT_EXEC, old_vpid);
+ /* Successful execution, now null out the cached argv
+ * (we don't want to access it later)
+ * */
+ bprm->argv_orig = NULL;
}
put_binfmt(fmt);
allow_write_access(bprm->file);
@@ -1515,7 +1542,7 @@ static int do_execve_common(const char *filename,
if (retval)
goto out_file;
- bprm->argc = count(argv, MAX_ARG_STRINGS);
+ bprm->argc_orig = bprm->argc = count(argv, MAX_ARG_STRINGS);
if ((retval = bprm->argc) < 0)
goto out;
@@ -1539,6 +1566,7 @@ static int do_execve_common(const char *filename,
retval = copy_strings(bprm->argc, argv, bprm);
if (retval < 0)
goto out;
+ bprm->argv_orig = &argv;
retval = search_binary_handler(bprm);
if (retval < 0)
diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
index 402a74a..2e3dc66 100644
--- a/include/linux/binfmts.h
+++ b/include/linux/binfmts.h
@@ -32,11 +32,14 @@ struct linux_binprm {
unsigned int taso:1;
#endif
unsigned int recursion_depth;
+ /* current binfmt at 0 and previous binfmt at 1 */
+ struct linux_binfmt *previous_binfmts[2];
struct file * file;
struct cred *cred; /* new credentials */
int unsafe; /* how unsafe this exec is (mask of LSM_UNSAFE_*) */
unsigned int per_clear; /* bits to clear in current->personality */
- int argc, envc;
+ int argc, argc_orig, envc;
+ void *argv_orig;
const char * filename; /* Name of binary as seen by procps */
const char * interp; /* Name of the binary really executed. Most
of the time same as filename, but could be
--
1.7.1
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH 3/3] fs/binfmts: Whitespace fixes with scripts/cleanfile
2013-07-25 15:40 [PATCH 1/3] fs/binfmts: Add a name field to the binfmt struct Zach Levis
2013-07-25 15:40 ` [PATCH 2/3] fs/binfmts: Better handling of binfmt loops Zach Levis
@ 2013-07-25 15:40 ` Zach Levis
2013-08-02 22:12 ` [PATCH v2 0/3] fs/binfmts: Improve handling of loops Zach Levis
` (15 subsequent siblings)
17 siblings, 0 replies; 35+ messages in thread
From: Zach Levis @ 2013-07-25 15:40 UTC (permalink / raw)
To: linux-fsdevel; +Cc: linux-kernel, viro, akpm, Zach Levis, Zach Levis
Obligatory first-patchset whitespace commit
Signed-off-by: Zach Levis <zach@zachsthings.com>
Signed-off-by: Zach Levis <zml@linux.vnet.ibm.com>
---
fs/binfmt_aout.c | 8 ++++----
fs/binfmt_elf.c | 38 +++++++++++++++++++-------------------
fs/binfmt_elf_fdpic.c | 8 ++++----
fs/binfmt_em86.c | 4 ++--
fs/binfmt_flat.c | 26 +++++++++++++-------------
fs/binfmt_misc.c | 24 ++++++++++++------------
fs/binfmt_script.c | 8 ++++----
fs/binfmt_som.c | 2 +-
fs/exec.c | 12 ++++++------
9 files changed, 65 insertions(+), 65 deletions(-)
diff --git a/fs/binfmt_aout.c b/fs/binfmt_aout.c
index f6bed04..c0a458b 100644
--- a/fs/binfmt_aout.c
+++ b/fs/binfmt_aout.c
@@ -62,7 +62,7 @@ static int aout_core_dump(struct coredump_params *cprm)
fs = get_fs();
set_fs(KERNEL_DS);
has_dumped = 1;
- strncpy(dump.u_comm, current->comm, sizeof(dump.u_comm));
+ strncpy(dump.u_comm, current->comm, sizeof(dump.u_comm));
dump.u_ar0 = offsetof(struct user, regs);
dump.signal = cprm->siginfo->si_signo;
aout_dump_thread(cprm->regs, &dump);
@@ -302,7 +302,7 @@ static int load_aout_binary(struct linux_binprm * bprm)
if ((fd_offset & ~PAGE_MASK) != 0 && printk_ratelimit())
{
- printk(KERN_WARNING
+ printk(KERN_WARNING
"fd_offset is not page aligned. Please convert program: %s\n",
bprm->file->f_path.dentry->d_name.name);
}
@@ -391,12 +391,12 @@ static int load_aout_library(struct file *file)
if ((N_TXTOFF(ex) & ~PAGE_MASK) != 0) {
if (printk_ratelimit())
{
- printk(KERN_WARNING
+ printk(KERN_WARNING
"N_TXTOFF is not page aligned. Please convert library: %s\n",
file->f_path.dentry->d_name.name);
}
vm_brk(start_addr, ex.a_text + ex.a_data + ex.a_bss);
-
+
read_code(file, start_addr, N_TXTOFF(ex),
ex.a_text + ex.a_data);
retval = 0;
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 2533969..02b358c 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -218,7 +218,7 @@ create_elf_tables(struct linux_binprm *bprm, struct elfhdr *exec,
} while (0)
#ifdef ARCH_DLINFO
- /*
+ /*
* ARCH_DLINFO must come first so PPC can do its special alignment of
* AUXV.
* update AT_VECTOR_SIZE_ARCH if the number of NEW_AUX_ENT() in
@@ -239,7 +239,7 @@ create_elf_tables(struct linux_binprm *bprm, struct elfhdr *exec,
NEW_AUX_ENT(AT_EUID, from_kuid_munged(cred->user_ns, cred->euid));
NEW_AUX_ENT(AT_GID, from_kgid_munged(cred->user_ns, cred->gid));
NEW_AUX_ENT(AT_EGID, from_kgid_munged(cred->user_ns, cred->egid));
- NEW_AUX_ENT(AT_SECURE, security_bprm_secureexec(bprm));
+ NEW_AUX_ENT(AT_SECURE, security_bprm_secureexec(bprm));
NEW_AUX_ENT(AT_RANDOM, (elf_addr_t)(unsigned long)u_rand_bytes);
#ifdef ELF_HWCAP2
NEW_AUX_ENT(AT_HWCAP2, ELF_HWCAP2);
@@ -433,7 +433,7 @@ static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex,
error = -EIO;
if (retval != size) {
if (retval < 0)
- error = retval;
+ error = retval;
goto out_close;
}
@@ -452,7 +452,7 @@ static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex,
unsigned long k, map_addr;
if (eppnt->p_flags & PF_R)
- elf_prot = PROT_READ;
+ elf_prot = PROT_READ;
if (eppnt->p_flags & PF_W)
elf_prot |= PROT_WRITE;
if (eppnt->p_flags & PF_X)
@@ -570,7 +570,7 @@ static unsigned long randomize_stack_top(unsigned long stack_top)
static int load_elf_binary(struct linux_binprm *bprm)
{
struct file *interpreter = NULL; /* to shut gcc up */
- unsigned long load_addr = 0, load_bias = 0;
+ unsigned long load_addr = 0, load_bias = 0;
int load_addr_set = 0;
char * elf_interpreter = NULL;
unsigned long error;
@@ -595,7 +595,7 @@ static int load_elf_binary(struct linux_binprm *bprm)
retval = -ENOMEM;
goto out_ret;
}
-
+
/* Get the exec-header */
loc->elf_ex = *((struct elfhdr *)bprm->buf);
@@ -615,7 +615,7 @@ static int load_elf_binary(struct linux_binprm *bprm)
if (loc->elf_ex.e_phentsize != sizeof(struct elf_phdr))
goto out;
if (loc->elf_ex.e_phnum < 1 ||
- loc->elf_ex.e_phnum > 65536U / sizeof(struct elf_phdr))
+ loc->elf_ex.e_phnum > 65536U / sizeof(struct elf_phdr))
goto out;
size = loc->elf_ex.e_phnum * sizeof(struct elf_phdr);
retval = -ENOMEM;
@@ -647,7 +647,7 @@ static int load_elf_binary(struct linux_binprm *bprm)
* is an a.out format binary
*/
retval = -ENOEXEC;
- if (elf_ppnt->p_filesz > PATH_MAX ||
+ if (elf_ppnt->p_filesz > PATH_MAX ||
elf_ppnt->p_filesz < 2)
goto out_free_ph;
@@ -747,7 +747,7 @@ static int load_elf_binary(struct linux_binprm *bprm)
send_sig(SIGKILL, current, 0);
goto out_free_dentry;
}
-
+
current->mm->start_stack = bprm->p;
/* Now we do a little grungy work by mmapping the ELF image into
@@ -762,7 +762,7 @@ static int load_elf_binary(struct linux_binprm *bprm)
if (unlikely (elf_brk > elf_bss)) {
unsigned long nbyte;
-
+
/* There was a PT_LOAD segment with p_memsz > p_filesz
before this one. Map anonymous pages, if needed,
and clear the area. */
@@ -1294,7 +1294,7 @@ static void fill_elf_note_phdr(struct elf_phdr *phdr, int sz, loff_t offset)
return;
}
-static void fill_note(struct memelfnote *note, const char *name, int type,
+static void fill_note(struct memelfnote *note, const char *name, int type,
unsigned int sz, void *data)
{
note->name = name;
@@ -1346,7 +1346,7 @@ static int fill_psinfo(struct elf_prpsinfo *psinfo, struct task_struct *p,
{
const struct cred *cred;
unsigned int i, len;
-
+
/* first copy the parameters from user space */
memset(psinfo, 0, sizeof(struct elf_prpsinfo));
@@ -1380,7 +1380,7 @@ static int fill_psinfo(struct elf_prpsinfo *psinfo, struct task_struct *p,
SET_GID(psinfo->pr_gid, from_kgid_munged(cred->user_ns, cred->gid));
rcu_read_unlock();
strncpy(psinfo->pr_fname, p->comm, sizeof(psinfo->pr_fname));
-
+
return 0;
}
@@ -1781,8 +1781,8 @@ static int elf_dump_thread_status(long signr, struct elf_thread_status *t)
t->num_notes = 0;
fill_prstatus(&t->prstatus, p, signr);
- elf_core_copy_task_regs(p, &t->prstatus.pr_reg);
-
+ elf_core_copy_task_regs(p, &t->prstatus.pr_reg);
+
fill_note(&t->notes[0], "CORE", NT_PRSTATUS, sizeof(t->prstatus),
&(t->prstatus));
t->num_notes++;
@@ -1803,7 +1803,7 @@ static int elf_dump_thread_status(long signr, struct elf_thread_status *t)
t->num_notes++;
sz += notesize(&t->notes[2]);
}
-#endif
+#endif
return sz;
}
@@ -2055,7 +2055,7 @@ static int elf_core_dump(struct coredump_params *cprm)
/*
* We no longer stop all VM operations.
- *
+ *
* This is because those proceses that could possibly change map_count
* or the mmap / vma pages are now blocked in do_exit on current
* finishing this core dump.
@@ -2064,7 +2064,7 @@ static int elf_core_dump(struct coredump_params *cprm)
* the map_count or the pages allocated. So no possibility of crashing
* exists while dumping the mm->vm_next areas to the core file.
*/
-
+
/* alloc memory for large data structures: too large to be on stack */
elf = kmalloc(sizeof(*elf), GFP_KERNEL);
if (!elf)
@@ -2170,7 +2170,7 @@ static int elf_core_dump(struct coredump_params *cprm)
if (!elf_core_write_extra_phdrs(cprm->file, offset, &size, cprm->limit))
goto end_coredump;
- /* write out the notes section */
+ /* write out the notes section */
if (!write_note_info(&info, cprm->file, &foffset))
goto end_coredump;
diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c
index 6be4448..799791b 100644
--- a/fs/binfmt_elf_fdpic.c
+++ b/fs/binfmt_elf_fdpic.c
@@ -1595,8 +1595,8 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm)
struct memelfnote *notes = NULL;
struct elf_prstatus *prstatus = NULL; /* NT_PRSTATUS */
struct elf_prpsinfo *psinfo = NULL; /* NT_PRPSINFO */
- LIST_HEAD(thread_list);
- struct list_head *t;
+ LIST_HEAD(thread_list);
+ struct list_head *t;
elf_fpregset_t *fpu = NULL;
#ifdef ELF_CORE_COPY_XFPREGS
elf_fpxregset_t *xfpu = NULL;
@@ -1705,7 +1705,7 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm)
fill_note(¬es[numnote++], "CORE", NT_AUXV,
i * sizeof(elf_addr_t), auxv);
- /* Try to dump the FPU. */
+ /* Try to dump the FPU. */
if ((prstatus->pr_fpvalid =
elf_core_copy_task_fpregs(current, cprm->regs, fpu)))
fill_note(notes + numnote++,
@@ -1795,7 +1795,7 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm)
if (!elf_core_write_extra_phdrs(cprm->file, offset, &size, cprm->limit))
goto end_coredump;
- /* write out the notes section */
+ /* write out the notes section */
for (i = 0; i < numnote; i++)
if (!writenote(notes + i, cprm->file, &foffset))
goto end_coredump;
diff --git a/fs/binfmt_em86.c b/fs/binfmt_em86.c
index 8404cdd..0e58293 100644
--- a/fs/binfmt_em86.c
+++ b/fs/binfmt_em86.c
@@ -63,11 +63,11 @@ static int load_em86(struct linux_binprm *bprm)
*/
remove_arg_zero(bprm);
retval = copy_strings_kernel(1, &bprm->filename, bprm);
- if (retval < 0) return retval;
+ if (retval < 0) return retval;
bprm->argc++;
if (i_arg) {
retval = copy_strings_kernel(1, &i_arg, bprm);
- if (retval < 0) return retval;
+ if (retval < 0) return retval;
bprm->argc++;
}
retval = copy_strings_kernel(1, &i_name, bprm);
diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
index a7641e1..887b3e5 100644
--- a/fs/binfmt_flat.c
+++ b/fs/binfmt_flat.c
@@ -388,7 +388,7 @@ void old_reloc(unsigned long rl)
#endif
flat_v2_reloc_t r;
unsigned long *ptr;
-
+
r.value = rl;
#if defined(CONFIG_COLDFIRE)
ptr = (unsigned long *) (current->mm->start_code + r.reloc.offset);
@@ -401,7 +401,7 @@ void old_reloc(unsigned long rl)
"(address %p, currently %x) into segment %s\n",
r.reloc.offset, ptr, (int)*ptr, segment[r.reloc.type]);
#endif
-
+
switch (r.reloc.type) {
case OLD_FLAT_RELOC_TYPE_TEXT:
*ptr += current->mm->start_code;
@@ -420,7 +420,7 @@ void old_reloc(unsigned long rl)
#ifdef DEBUG
printk("Relocation became %x\n", (int)*ptr);
#endif
-}
+}
/****************************************************************************/
@@ -479,7 +479,7 @@ static int load_flat_file(struct linux_binprm * bprm,
ret = -ENOEXEC;
goto err;
}
-
+
/* Don't allow old format executables to use shared libraries */
if (rev == OLD_FLAT_VERSION && id != 0) {
printk("BINFMT_FLAT: shared libraries are not available before rev 0x%x\n",
@@ -581,7 +581,7 @@ static int load_flat_file(struct linux_binprm * bprm,
fpos = ntohl(hdr->data_start);
#ifdef CONFIG_BINFMT_ZFLAT
if (flags & FLAT_FLAG_GZDATA) {
- result = decompress_exec(bprm, fpos, (char *) datapos,
+ result = decompress_exec(bprm, fpos, (char *) datapos,
full_data, 0);
} else
#endif
@@ -704,7 +704,7 @@ static int load_flat_file(struct linux_binprm * bprm,
libinfo->lib_list[id].loaded = 1;
libinfo->lib_list[id].entry = (0x00ffffff & ntohl(hdr->entry)) + textpos;
libinfo->lib_list[id].build_date = ntohl(hdr->build_date);
-
+
/*
* We just load the allocations into some temporary memory to
* help simplify all this mumbo jumbo
@@ -784,11 +784,11 @@ static int load_flat_file(struct linux_binprm * bprm,
for (i=0; i < relocs; i++)
old_reloc(ntohl(reloc[i]));
}
-
+
flush_icache_range(start_code, end_code);
/* zero the BSS, BRK and stack areas */
- memset((void*)(datapos + data_len), 0, bss_len +
+ memset((void*)(datapos + data_len), 0, bss_len +
(memp + memp_size - stack_len - /* end brk */
libinfo->lib_list[id].start_brk) + /* start brk */
stack_len);
@@ -882,11 +882,11 @@ static int load_flat_binary(struct linux_binprm * bprm)
stack_len += (bprm->argc + 1) * sizeof(char *); /* the argv array */
stack_len += (bprm->envc + 1) * sizeof(char *); /* the envp array */
stack_len += FLAT_STACK_ALIGN - 1; /* reserve for upcoming alignment */
-
+
res = load_flat_file(bprm, &libinfo, 0, &stack_len);
if (IS_ERR_VALUE(res))
return res;
-
+
/* Update data segment pointers for all libraries */
for (i=0; i<MAX_SHARED_LIBS; i++)
if (libinfo.lib_list[i].loaded)
@@ -908,7 +908,7 @@ static int load_flat_binary(struct linux_binprm * bprm)
((char *) page_address(bprm->page[i/PAGE_SIZE]))[i % PAGE_SIZE];
sp = (unsigned long *) create_flat_tables(p, bprm);
-
+
/* Fake some return addresses to ensure the call chain will
* initialise library in order for us. We are required to call
* lib 1 first, then 2, ... and finally the main program (id 0).
@@ -924,7 +924,7 @@ static int load_flat_binary(struct linux_binprm * bprm)
}
}
#endif
-
+
/* Stash our initial stack pointer into the mm structure */
current->mm->start_stack = (unsigned long )sp;
@@ -933,7 +933,7 @@ static int load_flat_binary(struct linux_binprm * bprm)
#endif
DBG_FLT("start_thread(regs=0x%x, entry=0x%x, start_stack=0x%x)\n",
(int)regs, (int)start_addr, (int)current->mm->start_stack);
-
+
start_thread(regs, start_addr, current->mm->start_stack);
return 0;
diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c
index d74afc0..8b9d701 100644
--- a/fs/binfmt_misc.c
+++ b/fs/binfmt_misc.c
@@ -62,7 +62,7 @@ static struct file_system_type bm_fs_type;
static struct vfsmount *bm_mnt;
static int entry_count;
-/*
+/*
* Check if we support the binfmt
* if we do, return the node, else NULL
* locking is done in load_misc_binary
@@ -138,12 +138,12 @@ static int load_misc_binary(struct linux_binprm *bprm)
/* if the binary should be opened on behalf of the
* interpreter than keep it open and assign descriptor
* to it */
- fd_binary = get_unused_fd();
- if (fd_binary < 0) {
- retval = fd_binary;
- goto _ret;
- }
- fd_install(fd_binary, bprm->file);
+ fd_binary = get_unused_fd();
+ if (fd_binary < 0) {
+ retval = fd_binary;
+ goto _ret;
+ }
+ fd_install(fd_binary, bprm->file);
/* if the binary is not readable than enforce mm->dumpable=0
regardless of the interpreter's permissions */
@@ -156,11 +156,11 @@ static int load_misc_binary(struct linux_binprm *bprm)
bprm->interp_flags |= BINPRM_FLAGS_EXECFD;
bprm->interp_data = fd_binary;
- } else {
- allow_write_access(bprm->file);
- fput(bprm->file);
- bprm->file = NULL;
- }
+ } else {
+ allow_write_access(bprm->file);
+ fput(bprm->file);
+ bprm->file = NULL;
+ }
/* make argv[1] be the path to the binary */
retval = copy_strings_kernel (1, &bprm->interp, bprm);
if (retval < 0)
diff --git a/fs/binfmt_script.c b/fs/binfmt_script.c
index 53f3c17..c5bac1a 100644
--- a/fs/binfmt_script.c
+++ b/fs/binfmt_script.c
@@ -45,7 +45,7 @@ static int load_script(struct linux_binprm *bprm)
break;
}
for (cp = bprm->buf+2; (*cp == ' ') || (*cp == '\t'); cp++);
- if (*cp == '\0')
+ if (*cp == '\0')
return -ENOEXEC; /* No interpreter name found */
i_name = cp;
i_arg = NULL;
@@ -70,15 +70,15 @@ static int load_script(struct linux_binprm *bprm)
if (retval)
return retval;
retval = copy_strings_kernel(1, &bprm->interp, bprm);
- if (retval < 0) return retval;
+ if (retval < 0) return retval;
bprm->argc++;
if (i_arg) {
retval = copy_strings_kernel(1, &i_arg, bprm);
- if (retval < 0) return retval;
+ if (retval < 0) return retval;
bprm->argc++;
}
retval = copy_strings_kernel(1, &i_name, bprm);
- if (retval) return retval;
+ if (retval) return retval;
bprm->argc++;
retval = bprm_change_interp(interp, bprm);
if (retval < 0)
diff --git a/fs/binfmt_som.c b/fs/binfmt_som.c
index 1c3ccc5..88af0d1 100644
--- a/fs/binfmt_som.c
+++ b/fs/binfmt_som.c
@@ -2,7 +2,7 @@
* linux/fs/binfmt_som.c
*
* These are the functions used to load SOM format executables as used
- * by HP-UX.
+ * by HP-UX.
*
* Copyright 1999 Matthew Wilcox <willy@bofh.ai>
* based on binfmt_elf which is
diff --git a/fs/exec.c b/fs/exec.c
index fa59f05..3a259be 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -19,7 +19,7 @@
* current->executable is only used by the procfs. This allows a dispatch
* table to check for several different types of binary formats. We keep
* trying until we recognize the file or we run out of supported binary
- * formats.
+ * formats.
*/
#include <linux/slab.h>
@@ -154,7 +154,7 @@ SYSCALL_DEFINE1(uselib, const char __user *, library)
exit:
fput(file);
out:
- return error;
+ return error;
}
#ifdef CONFIG_MMU
@@ -1139,7 +1139,7 @@ void setup_new_exec(struct linux_binprm * bprm)
group */
current->self_exec_id++;
-
+
flush_signal_handlers(current, 0);
do_close_on_exec(current->files);
}
@@ -1265,8 +1265,8 @@ static int check_unsafe_exec(struct linux_binprm *bprm)
return res;
}
-/*
- * Fill the binprm structure from the inode.
+/*
+ * Fill the binprm structure from the inode.
* Check permissions, then read the first 128 (BINPRM_BUF_SIZE) bytes
*
* This may be called multiple times for binary chains (scripts for example).
@@ -1369,7 +1369,7 @@ EXPORT_SYMBOL(remove_arg_zero);
int search_binary_handler(struct linux_binprm *bprm)
{
unsigned int depth = bprm->recursion_depth;
- int try,retval;
+ int try, retval;
struct linux_binfmt *fmt;
pid_t old_pid, old_vpid;
--
1.7.1
^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: [PATCH 2/3] fs/binfmts: Better handling of binfmt loops
2013-07-25 15:40 ` [PATCH 2/3] fs/binfmts: Better handling of binfmt loops Zach Levis
@ 2013-07-30 21:04 ` Andrew Morton
2013-07-30 23:16 ` Zach Levis
0 siblings, 1 reply; 35+ messages in thread
From: Andrew Morton @ 2013-07-30 21:04 UTC (permalink / raw)
To: Zach Levis; +Cc: linux-fsdevel, linux-kernel, viro, Zach Levis
On Thu, 25 Jul 2013 08:40:44 -0700 Zach Levis <zml@linux.vnet.ibm.com> wrote:
> With these changes, when a binfmt loop is encountered,
> the ELOOP will propogate back to the 0 depth. At this point the
> argv and argc values will be reset to what they were originally and an
> attempt is made to continue with the following binfmt handlers.
hm, why? What problem does this fix? What value does the change offer
to our users?
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 2/3] fs/binfmts: Better handling of binfmt loops
2013-07-30 21:04 ` Andrew Morton
@ 2013-07-30 23:16 ` Zach Levis
2013-07-30 23:26 ` Andrew Morton
0 siblings, 1 reply; 35+ messages in thread
From: Zach Levis @ 2013-07-30 23:16 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-fsdevel, linux-kernel, viro, Zach Levis
Quoting Andrew Morton <akpm@linux-foundation.org>:
> On Thu, 25 Jul 2013 08:40:44 -0700 Zach Levis <zml@linux.vnet.ibm.com> wrote:
>
>> With these changes, when a binfmt loop is encountered,
>> the ELOOP will propogate back to the 0 depth. At this point the
>> argv and argc values will be reset to what they were originally and an
>> attempt is made to continue with the following binfmt handlers.
>
> hm, why? What problem does this fix? What value does the change offer
> to our users?
This is used when the binfmt_misc,script,etc options are configured in
a way that would previously prevent executables from launching that
could be executed with a different binfmt but don't because of a loop
in a prior binfmt.
Example: a qemu is configured to run 64-bit ELFs on an otherwise
32-bit system. The system's owner switches to running with 64-bit
executables, but forgets to disable the binfmt_misc option that
redirects 64bit ELFs to qemu. Since the qemu executable is a 64-bit
ELF now, binfmt_misc keeps on matching it with the qemu rule,
preventing the execution of any 64-bit binary.
With this patch, an error is printed and search_binary_handler()
continues on to the next handler, allowing the original executable to
run normally so the user can (hopefully) fix their misconfiguration
more easily.
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 2/3] fs/binfmts: Better handling of binfmt loops
2013-07-30 23:16 ` Zach Levis
@ 2013-07-30 23:26 ` Andrew Morton
2013-07-31 16:17 ` Zach Levis
0 siblings, 1 reply; 35+ messages in thread
From: Andrew Morton @ 2013-07-30 23:26 UTC (permalink / raw)
To: Zach Levis; +Cc: linux-fsdevel, linux-kernel, viro, Zach Levis
On Tue, 30 Jul 2013 16:16:51 -0700 Zach Levis <zml@linux.vnet.ibm.com> wrote:
>
> Quoting Andrew Morton <akpm@linux-foundation.org>:
>
> > On Thu, 25 Jul 2013 08:40:44 -0700 Zach Levis <zml@linux.vnet.ibm.com> wrote:
> >
> >> With these changes, when a binfmt loop is encountered,
> >> the ELOOP will propogate back to the 0 depth. At this point the
> >> argv and argc values will be reset to what they were originally and an
> >> attempt is made to continue with the following binfmt handlers.
> >
> > hm, why? What problem does this fix? What value does the change offer
> > to our users?
>
> This is used when the binfmt_misc,script,etc options are configured in
> a way that would previously prevent executables from launching that
> could be executed with a different binfmt but don't because of a loop
> in a prior binfmt.
>
> Example: a qemu is configured to run 64-bit ELFs on an otherwise
> 32-bit system. The system's owner switches to running with 64-bit
> executables, but forgets to disable the binfmt_misc option that
> redirects 64bit ELFs to qemu. Since the qemu executable is a 64-bit
> ELF now, binfmt_misc keeps on matching it with the qemu rule,
> preventing the execution of any 64-bit binary.
So the admin can unforget to make that change and no longer has a problem.
> With this patch, an error is printed and search_binary_handler()
> continues on to the next handler, allowing the original executable to
> run normally so the user can (hopefully) fix their misconfiguration
> more easily.
Is all this really worth changing the kernel for? It sounds
a bit marginal.
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 2/3] fs/binfmts: Better handling of binfmt loops
2013-07-30 23:26 ` Andrew Morton
@ 2013-07-31 16:17 ` Zach Levis
0 siblings, 0 replies; 35+ messages in thread
From: Zach Levis @ 2013-07-31 16:17 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-fsdevel, linux-kernel, viro, Zach Levis
Quoting Andrew Morton <akpm@linux-foundation.org>:
> On Tue, 30 Jul 2013 16:16:51 -0700 Zach Levis <zml@linux.vnet.ibm.com> wrote:
>
>>
>> Quoting Andrew Morton <akpm@linux-foundation.org>:
>>
>> > On Thu, 25 Jul 2013 08:40:44 -0700 Zach Levis
>> <zml@linux.vnet.ibm.com> wrote:
>> >
>> >> With these changes, when a binfmt loop is encountered,
>> >> the ELOOP will propogate back to the 0 depth. At this point the
>> >> argv and argc values will be reset to what they were originally and an
>> >> attempt is made to continue with the following binfmt handlers.
>> >
>> > hm, why? What problem does this fix? What value does the change offer
>> > to our users?
>>
>> This is used when the binfmt_misc,script,etc options are configured in
>> a way that would previously prevent executables from launching that
>> could be executed with a different binfmt but don't because of a loop
>> in a prior binfmt.
>>
>> Example: a qemu is configured to run 64-bit ELFs on an otherwise
>> 32-bit system. The system's owner switches to running with 64-bit
>> executables, but forgets to disable the binfmt_misc option that
>> redirects 64bit ELFs to qemu. Since the qemu executable is a 64-bit
>> ELF now, binfmt_misc keeps on matching it with the qemu rule,
>> preventing the execution of any 64-bit binary.
>
> So the admin can unforget to make that change and no longer has a problem.
>
>> With this patch, an error is printed and search_binary_handler()
>> continues on to the next handler, allowing the original executable to
>> run normally so the user can (hopefully) fix their misconfiguration
>> more easily.
>
> Is all this really worth changing the kernel for? It sounds
> a bit marginal.
I'd say it is worth changing because this is a self-contained fix that
doesn't have much in the way of side effects for normal execution but
can help recover a system. Apart from storing a pointer to the
provided arguments, the only changes this requires are in the code for
error handling.
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v2 0/3] fs/binfmts: Improve handling of loops
2013-07-25 15:40 [PATCH 1/3] fs/binfmts: Add a name field to the binfmt struct Zach Levis
2013-07-25 15:40 ` [PATCH 2/3] fs/binfmts: Better handling of binfmt loops Zach Levis
2013-07-25 15:40 ` [PATCH 3/3] fs/binfmts: Whitespace fixes with scripts/cleanfile Zach Levis
@ 2013-08-02 22:12 ` Zach Levis
2013-08-02 22:12 ` [PATCH v2 1/3] fs/binfmts: Add a name field to the binfmt struct Zach Levis
` (14 subsequent siblings)
17 siblings, 0 replies; 35+ messages in thread
From: Zach Levis @ 2013-08-02 22:12 UTC (permalink / raw)
To: akpm, oleg; +Cc: viro, linux-kernel, linux-fsdevel, dan.carpenter, Zach Levis
This v2 is based off Oleg's changes from "exec: more cleanups" and "exec: minor cleanups + minor fix"
It incorporates Oleg and Andrew's suggestions and takes care of the issue from Dan's patch "fs/binfmts: double unlock in search_binary_handler()"
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v2 1/3] fs/binfmts: Add a name field to the binfmt struct
2013-07-25 15:40 [PATCH 1/3] fs/binfmts: Add a name field to the binfmt struct Zach Levis
` (2 preceding siblings ...)
2013-08-02 22:12 ` [PATCH v2 0/3] fs/binfmts: Improve handling of loops Zach Levis
@ 2013-08-02 22:12 ` Zach Levis
2013-08-02 22:12 ` [PATCH v2 2/3] fs/binfmts: Better handling of binfmt loops Zach Levis
` (13 subsequent siblings)
17 siblings, 0 replies; 35+ messages in thread
From: Zach Levis @ 2013-08-02 22:12 UTC (permalink / raw)
To: akpm, oleg
Cc: viro, linux-kernel, linux-fsdevel, dan.carpenter, Zach Levis, Zach Levis
Adding the name field helps when printing error messages referring to
specific binfmts
Signed-off-by: Zach Levis <zach@zachsthings.com>
Signed-off-by: Zach Levis <zml@linux.vnet.ibm.com>
---
fs/binfmt_aout.c | 1 +
fs/binfmt_elf.c | 1 +
fs/binfmt_elf_fdpic.c | 1 +
fs/binfmt_em86.c | 1 +
fs/binfmt_flat.c | 1 +
fs/binfmt_misc.c | 1 +
fs/binfmt_script.c | 1 +
fs/binfmt_som.c | 1 +
include/linux/binfmts.h | 1 +
9 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/fs/binfmt_aout.c b/fs/binfmt_aout.c
index bce8769..f6bed04 100644
--- a/fs/binfmt_aout.c
+++ b/fs/binfmt_aout.c
@@ -115,6 +115,7 @@ end_coredump:
#endif
static struct linux_binfmt aout_format = {
+ .name = "a.out",
.module = THIS_MODULE,
.load_binary = load_aout_binary,
.load_shlib = load_aout_library,
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index f8a0b0e..cd7d546 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -75,6 +75,7 @@ static int elf_core_dump(struct coredump_params *cprm);
#define ELF_PAGEALIGN(_v) (((_v) + ELF_MIN_ALIGN - 1) & ~(ELF_MIN_ALIGN - 1))
static struct linux_binfmt elf_format = {
+ .name = "ELF",
.module = THIS_MODULE,
.load_binary = load_elf_binary,
.load_shlib = load_elf_library,
diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c
index c166f32..6be4448 100644
--- a/fs/binfmt_elf_fdpic.c
+++ b/fs/binfmt_elf_fdpic.c
@@ -81,6 +81,7 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm);
#endif
static struct linux_binfmt elf_fdpic_format = {
+ .name = "FDPIC ELF",
.module = THIS_MODULE,
.load_binary = load_elf_fdpic_binary,
#ifdef CONFIG_ELF_CORE
diff --git a/fs/binfmt_em86.c b/fs/binfmt_em86.c
index 037a3e2..8404cdd 100644
--- a/fs/binfmt_em86.c
+++ b/fs/binfmt_em86.c
@@ -93,6 +93,7 @@ static int load_em86(struct linux_binprm *bprm)
}
static struct linux_binfmt em86_format = {
+ .name = "em86",
.module = THIS_MODULE,
.load_binary = load_em86,
};
diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
index d50bbe5..a7641e1 100644
--- a/fs/binfmt_flat.c
+++ b/fs/binfmt_flat.c
@@ -92,6 +92,7 @@ static int load_flat_binary(struct linux_binprm *);
static int flat_core_dump(struct coredump_params *cprm);
static struct linux_binfmt flat_format = {
+ .name = "flat",
.module = THIS_MODULE,
.load_binary = load_flat_binary,
.core_dump = flat_core_dump,
diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c
index 1c740e1..d74afc0 100644
--- a/fs/binfmt_misc.c
+++ b/fs/binfmt_misc.c
@@ -694,6 +694,7 @@ static struct dentry *bm_mount(struct file_system_type *fs_type,
}
static struct linux_binfmt misc_format = {
+ .name = "misc",
.module = THIS_MODULE,
.load_binary = load_misc_binary,
};
diff --git a/fs/binfmt_script.c b/fs/binfmt_script.c
index 5027a3e..53f3c17 100644
--- a/fs/binfmt_script.c
+++ b/fs/binfmt_script.c
@@ -99,6 +99,7 @@ static int load_script(struct linux_binprm *bprm)
}
static struct linux_binfmt script_format = {
+ .name = "script",
.module = THIS_MODULE,
.load_binary = load_script,
};
diff --git a/fs/binfmt_som.c b/fs/binfmt_som.c
index 4e00ed6..1c3ccc5 100644
--- a/fs/binfmt_som.c
+++ b/fs/binfmt_som.c
@@ -53,6 +53,7 @@ static int som_core_dump(struct coredump_params *cprm);
#define SOM_PAGEALIGN(_v) (((_v) + SOM_PAGESIZE - 1) & ~(SOM_PAGESIZE - 1))
static struct linux_binfmt som_format = {
+ .name = "SOM",
.module = THIS_MODULE,
.load_binary = load_som_binary,
.load_shlib = load_som_library,
diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
index 70cf138..402a74a 100644
--- a/include/linux/binfmts.h
+++ b/include/linux/binfmts.h
@@ -70,6 +70,7 @@ struct coredump_params {
struct linux_binfmt {
struct list_head lh;
struct module *module;
+ const char *name;
int (*load_binary)(struct linux_binprm *);
int (*load_shlib)(struct file *);
int (*core_dump)(struct coredump_params *cprm);
--
1.7.1
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v2 2/3] fs/binfmts: Better handling of binfmt loops
2013-07-25 15:40 [PATCH 1/3] fs/binfmts: Add a name field to the binfmt struct Zach Levis
` (3 preceding siblings ...)
2013-08-02 22:12 ` [PATCH v2 1/3] fs/binfmts: Add a name field to the binfmt struct Zach Levis
@ 2013-08-02 22:12 ` Zach Levis
2013-08-02 22:49 ` Zach Levis
2013-08-02 22:12 ` [PATCH v2 3/3] fs/binfmts: Whitespace fixes with scripts/cleanfile Zach Levis
` (12 subsequent siblings)
17 siblings, 1 reply; 35+ messages in thread
From: Zach Levis @ 2013-08-02 22:12 UTC (permalink / raw)
To: akpm, oleg
Cc: viro, linux-kernel, linux-fsdevel, dan.carpenter, Zach Levis, Zach Levis
With these changes, when a binfmt loop is encountered,
the ELOOP will propogate back to the 0 depth. At this point the
argv and argc values will be reset to what they were originally and an
attempt is made to continue with the following binfmt handlers.
Example: a qemu is configured to run 64-bit ELFs on an otherwise 32-bit
system. The system's owner switches to running with 64-bit executables,
but forgets to disable the binfmt_misc option that redirects 64bit ELFs
to qemu. Since the qemu executable is a 64-bit ELF now, binfmt_misc
keeps on matching it with the qemu rule, preventing the execution of any
64-bit binary.
With these changes, an error is printed and search_binary_handler()
continues on to the next handler, allowing the original executable to
run normally so the user can (hopefully) fix their misconfiguration more
easily.
Caveats:
- binfmt_misc is skipped as a whole. To allow for a more generic
solution that works for any binfmt without a lot of duplicated code
and added complexity, the error detection code is applied on the
binfmt level.
- This is a fallback solution. It attempts to restore the state to allow
executing most binaries without side effects, but it may not work for
everything and should not be depended on for regular usage.
My (rough, but functional) test scripts for this issue are available at:
https://gist.github.com/zml2008/6075418
Signed-off-by: Zach Levis <zach@zachsthings.com>
Signed-off-by: Zach Levis <zml@linux.vnet.ibm.com>
---
fs/exec.c | 27 +++++++++++++++++++++++++++
include/linux/binfmts.h | 7 ++++++-
2 files changed, 33 insertions(+), 1 deletions(-)
diff --git a/fs/exec.c b/fs/exec.c
index eb229e1..54c4929 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1394,6 +1394,9 @@ int search_binary_handler(struct linux_binprm *bprm)
if (!try_module_get(fmt->module))
continue;
read_unlock(&binfmt_lock);
+ bprm->previous_binfmts[1] = bprm->previous_binfmts[0];
+ bprm->previous_binfmts[0] = fmt;
+
bprm->recursion_depth++;
retval = fmt->load_binary(bprm);
bprm->recursion_depth--;
@@ -1404,6 +1407,24 @@ int search_binary_handler(struct linux_binprm *bprm)
}
read_lock(&binfmt_lock);
put_binfmt(fmt);
+ if (retval == -ELOOP && bprm->recursion_depth == 0) { /* cur, previous */
+ pr_err("Too much recursion with binfmts (0:%s, -1:%s) in file %s, skipping (base %s).\n",
+ bprm->previous_binfmts[0]->name,
+ bprm->previous_binfmts[1]->name,
+ bprm->filename,
+ fmt->name);
+
+ /* Put argv back in its place */
+ bprm->p = bprm->p_no_argv;
+
+ bprm->argc = count(*(bprm->argv_orig), MAX_ARG_STRINGS);
+ retval = copy_strings(bprm->argc, *(bprm->argv_orig), bprm);
+ if (retval < 0)
+ return retval;
+
+ retval = -ENOEXEC;
+ continue;
+ }
}
read_unlock(&binfmt_lock);
@@ -1436,6 +1457,10 @@ static int exec_binprm(struct linux_binprm *bprm)
if (ret >= 0) {
trace_sched_process_exec(current, old_pid, bprm);
ptrace_event(PTRACE_EVENT_EXEC, old_vpid);
+ /* Successful execution, now null out the cached argv
+ * (we don't want to access it later)
+ * */
+ bprm->argv_orig = NULL;
current->did_exec = 1;
proc_exec_connector(current);
@@ -1533,9 +1558,11 @@ static int do_execve_common(const char *filename,
if (retval < 0)
goto out;
+ bprm->p_no_argv = bprm->p;
retval = copy_strings(bprm->argc, argv, bprm);
if (retval < 0)
goto out;
+ bprm->argv_orig = &argv;
retval = exec_binprm(bprm);
if (retval < 0)
diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
index 402a74a..42c1656 100644
--- a/include/linux/binfmts.h
+++ b/include/linux/binfmts.h
@@ -8,6 +8,8 @@
#define CORENAME_MAX_SIZE 128
+struct user_arg_ptr; /* Struct from fs/exec.c, for linux_binprm->argv_orig */
+
/*
* This structure is used to hold the arguments that are used when loading binaries.
*/
@@ -21,7 +23,7 @@ struct linux_binprm {
struct page *page[MAX_ARG_PAGES];
#endif
struct mm_struct *mm;
- unsigned long p; /* current top of mem */
+ unsigned long p, p_no_argv; /* current top of mem */
unsigned int
cred_prepared:1,/* true if creds already prepared (multiple
* preps happen for interpreters) */
@@ -32,11 +34,14 @@ struct linux_binprm {
unsigned int taso:1;
#endif
unsigned int recursion_depth;
+ /* current binfmt at 0 and previous binfmt at 1 */
+ struct linux_binfmt *previous_binfmts[2];
struct file * file;
struct cred *cred; /* new credentials */
int unsafe; /* how unsafe this exec is (mask of LSM_UNSAFE_*) */
unsigned int per_clear; /* bits to clear in current->personality */
int argc, envc;
+ struct user_arg_ptr *argv_orig;
const char * filename; /* Name of binary as seen by procps */
const char * interp; /* Name of the binary really executed. Most
of the time same as filename, but could be
--
1.7.1
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v2 3/3] fs/binfmts: Whitespace fixes with scripts/cleanfile
2013-07-25 15:40 [PATCH 1/3] fs/binfmts: Add a name field to the binfmt struct Zach Levis
` (4 preceding siblings ...)
2013-08-02 22:12 ` [PATCH v2 2/3] fs/binfmts: Better handling of binfmt loops Zach Levis
@ 2013-08-02 22:12 ` Zach Levis
2013-08-02 23:21 ` [PATCH v3 0/3] fs/binfmts: Improve handling of loops Zach Levis
` (11 subsequent siblings)
17 siblings, 0 replies; 35+ messages in thread
From: Zach Levis @ 2013-08-02 22:12 UTC (permalink / raw)
To: akpm, oleg
Cc: viro, linux-kernel, linux-fsdevel, dan.carpenter, Zach Levis, Zach Levis
Obligatory first-patchset whitespace commit
Signed-off-by: Zach Levis <zach@zachsthings.com>
Signed-off-by: Zach Levis <zml@linux.vnet.ibm.com>
---
fs/binfmt_aout.c | 8 ++++----
fs/binfmt_elf.c | 38 +++++++++++++++++++-------------------
fs/binfmt_elf_fdpic.c | 8 ++++----
fs/binfmt_em86.c | 4 ++--
fs/binfmt_flat.c | 26 +++++++++++++-------------
fs/binfmt_misc.c | 24 ++++++++++++------------
fs/binfmt_script.c | 8 ++++----
fs/binfmt_som.c | 2 +-
fs/exec.c | 10 +++++-----
9 files changed, 64 insertions(+), 64 deletions(-)
diff --git a/fs/binfmt_aout.c b/fs/binfmt_aout.c
index f6bed04..c0a458b 100644
--- a/fs/binfmt_aout.c
+++ b/fs/binfmt_aout.c
@@ -62,7 +62,7 @@ static int aout_core_dump(struct coredump_params *cprm)
fs = get_fs();
set_fs(KERNEL_DS);
has_dumped = 1;
- strncpy(dump.u_comm, current->comm, sizeof(dump.u_comm));
+ strncpy(dump.u_comm, current->comm, sizeof(dump.u_comm));
dump.u_ar0 = offsetof(struct user, regs);
dump.signal = cprm->siginfo->si_signo;
aout_dump_thread(cprm->regs, &dump);
@@ -302,7 +302,7 @@ static int load_aout_binary(struct linux_binprm * bprm)
if ((fd_offset & ~PAGE_MASK) != 0 && printk_ratelimit())
{
- printk(KERN_WARNING
+ printk(KERN_WARNING
"fd_offset is not page aligned. Please convert program: %s\n",
bprm->file->f_path.dentry->d_name.name);
}
@@ -391,12 +391,12 @@ static int load_aout_library(struct file *file)
if ((N_TXTOFF(ex) & ~PAGE_MASK) != 0) {
if (printk_ratelimit())
{
- printk(KERN_WARNING
+ printk(KERN_WARNING
"N_TXTOFF is not page aligned. Please convert library: %s\n",
file->f_path.dentry->d_name.name);
}
vm_brk(start_addr, ex.a_text + ex.a_data + ex.a_bss);
-
+
read_code(file, start_addr, N_TXTOFF(ex),
ex.a_text + ex.a_data);
retval = 0;
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index cd7d546..9b0b372 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -218,7 +218,7 @@ create_elf_tables(struct linux_binprm *bprm, struct elfhdr *exec,
} while (0)
#ifdef ARCH_DLINFO
- /*
+ /*
* ARCH_DLINFO must come first so PPC can do its special alignment of
* AUXV.
* update AT_VECTOR_SIZE_ARCH if the number of NEW_AUX_ENT() in
@@ -239,7 +239,7 @@ create_elf_tables(struct linux_binprm *bprm, struct elfhdr *exec,
NEW_AUX_ENT(AT_EUID, from_kuid_munged(cred->user_ns, cred->euid));
NEW_AUX_ENT(AT_GID, from_kgid_munged(cred->user_ns, cred->gid));
NEW_AUX_ENT(AT_EGID, from_kgid_munged(cred->user_ns, cred->egid));
- NEW_AUX_ENT(AT_SECURE, security_bprm_secureexec(bprm));
+ NEW_AUX_ENT(AT_SECURE, security_bprm_secureexec(bprm));
NEW_AUX_ENT(AT_RANDOM, (elf_addr_t)(unsigned long)u_rand_bytes);
#ifdef ELF_HWCAP2
NEW_AUX_ENT(AT_HWCAP2, ELF_HWCAP2);
@@ -433,7 +433,7 @@ static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex,
error = -EIO;
if (retval != size) {
if (retval < 0)
- error = retval;
+ error = retval;
goto out_close;
}
@@ -452,7 +452,7 @@ static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex,
unsigned long k, map_addr;
if (eppnt->p_flags & PF_R)
- elf_prot = PROT_READ;
+ elf_prot = PROT_READ;
if (eppnt->p_flags & PF_W)
elf_prot |= PROT_WRITE;
if (eppnt->p_flags & PF_X)
@@ -570,7 +570,7 @@ static unsigned long randomize_stack_top(unsigned long stack_top)
static int load_elf_binary(struct linux_binprm *bprm)
{
struct file *interpreter = NULL; /* to shut gcc up */
- unsigned long load_addr = 0, load_bias = 0;
+ unsigned long load_addr = 0, load_bias = 0;
int load_addr_set = 0;
char * elf_interpreter = NULL;
unsigned long error;
@@ -595,7 +595,7 @@ static int load_elf_binary(struct linux_binprm *bprm)
retval = -ENOMEM;
goto out_ret;
}
-
+
/* Get the exec-header */
loc->elf_ex = *((struct elfhdr *)bprm->buf);
@@ -615,7 +615,7 @@ static int load_elf_binary(struct linux_binprm *bprm)
if (loc->elf_ex.e_phentsize != sizeof(struct elf_phdr))
goto out;
if (loc->elf_ex.e_phnum < 1 ||
- loc->elf_ex.e_phnum > 65536U / sizeof(struct elf_phdr))
+ loc->elf_ex.e_phnum > 65536U / sizeof(struct elf_phdr))
goto out;
size = loc->elf_ex.e_phnum * sizeof(struct elf_phdr);
retval = -ENOMEM;
@@ -647,7 +647,7 @@ static int load_elf_binary(struct linux_binprm *bprm)
* is an a.out format binary
*/
retval = -ENOEXEC;
- if (elf_ppnt->p_filesz > PATH_MAX ||
+ if (elf_ppnt->p_filesz > PATH_MAX ||
elf_ppnt->p_filesz < 2)
goto out_free_ph;
@@ -747,7 +747,7 @@ static int load_elf_binary(struct linux_binprm *bprm)
send_sig(SIGKILL, current, 0);
goto out_free_dentry;
}
-
+
current->mm->start_stack = bprm->p;
/* Now we do a little grungy work by mmapping the ELF image into
@@ -762,7 +762,7 @@ static int load_elf_binary(struct linux_binprm *bprm)
if (unlikely (elf_brk > elf_bss)) {
unsigned long nbyte;
-
+
/* There was a PT_LOAD segment with p_memsz > p_filesz
before this one. Map anonymous pages, if needed,
and clear the area. */
@@ -1294,7 +1294,7 @@ static void fill_elf_note_phdr(struct elf_phdr *phdr, int sz, loff_t offset)
return;
}
-static void fill_note(struct memelfnote *note, const char *name, int type,
+static void fill_note(struct memelfnote *note, const char *name, int type,
unsigned int sz, void *data)
{
note->name = name;
@@ -1346,7 +1346,7 @@ static int fill_psinfo(struct elf_prpsinfo *psinfo, struct task_struct *p,
{
const struct cred *cred;
unsigned int i, len;
-
+
/* first copy the parameters from user space */
memset(psinfo, 0, sizeof(struct elf_prpsinfo));
@@ -1380,7 +1380,7 @@ static int fill_psinfo(struct elf_prpsinfo *psinfo, struct task_struct *p,
SET_GID(psinfo->pr_gid, from_kgid_munged(cred->user_ns, cred->gid));
rcu_read_unlock();
strncpy(psinfo->pr_fname, p->comm, sizeof(psinfo->pr_fname));
-
+
return 0;
}
@@ -1781,8 +1781,8 @@ static int elf_dump_thread_status(long signr, struct elf_thread_status *t)
t->num_notes = 0;
fill_prstatus(&t->prstatus, p, signr);
- elf_core_copy_task_regs(p, &t->prstatus.pr_reg);
-
+ elf_core_copy_task_regs(p, &t->prstatus.pr_reg);
+
fill_note(&t->notes[0], "CORE", NT_PRSTATUS, sizeof(t->prstatus),
&(t->prstatus));
t->num_notes++;
@@ -1803,7 +1803,7 @@ static int elf_dump_thread_status(long signr, struct elf_thread_status *t)
t->num_notes++;
sz += notesize(&t->notes[2]);
}
-#endif
+#endif
return sz;
}
@@ -2055,7 +2055,7 @@ static int elf_core_dump(struct coredump_params *cprm)
/*
* We no longer stop all VM operations.
- *
+ *
* This is because those proceses that could possibly change map_count
* or the mmap / vma pages are now blocked in do_exit on current
* finishing this core dump.
@@ -2064,7 +2064,7 @@ static int elf_core_dump(struct coredump_params *cprm)
* the map_count or the pages allocated. So no possibility of crashing
* exists while dumping the mm->vm_next areas to the core file.
*/
-
+
/* alloc memory for large data structures: too large to be on stack */
elf = kmalloc(sizeof(*elf), GFP_KERNEL);
if (!elf)
@@ -2170,7 +2170,7 @@ static int elf_core_dump(struct coredump_params *cprm)
if (!elf_core_write_extra_phdrs(cprm->file, offset, &size, cprm->limit))
goto end_coredump;
- /* write out the notes section */
+ /* write out the notes section */
if (!write_note_info(&info, cprm->file, &foffset))
goto end_coredump;
diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c
index 6be4448..799791b 100644
--- a/fs/binfmt_elf_fdpic.c
+++ b/fs/binfmt_elf_fdpic.c
@@ -1595,8 +1595,8 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm)
struct memelfnote *notes = NULL;
struct elf_prstatus *prstatus = NULL; /* NT_PRSTATUS */
struct elf_prpsinfo *psinfo = NULL; /* NT_PRPSINFO */
- LIST_HEAD(thread_list);
- struct list_head *t;
+ LIST_HEAD(thread_list);
+ struct list_head *t;
elf_fpregset_t *fpu = NULL;
#ifdef ELF_CORE_COPY_XFPREGS
elf_fpxregset_t *xfpu = NULL;
@@ -1705,7 +1705,7 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm)
fill_note(¬es[numnote++], "CORE", NT_AUXV,
i * sizeof(elf_addr_t), auxv);
- /* Try to dump the FPU. */
+ /* Try to dump the FPU. */
if ((prstatus->pr_fpvalid =
elf_core_copy_task_fpregs(current, cprm->regs, fpu)))
fill_note(notes + numnote++,
@@ -1795,7 +1795,7 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm)
if (!elf_core_write_extra_phdrs(cprm->file, offset, &size, cprm->limit))
goto end_coredump;
- /* write out the notes section */
+ /* write out the notes section */
for (i = 0; i < numnote; i++)
if (!writenote(notes + i, cprm->file, &foffset))
goto end_coredump;
diff --git a/fs/binfmt_em86.c b/fs/binfmt_em86.c
index 8404cdd..0e58293 100644
--- a/fs/binfmt_em86.c
+++ b/fs/binfmt_em86.c
@@ -63,11 +63,11 @@ static int load_em86(struct linux_binprm *bprm)
*/
remove_arg_zero(bprm);
retval = copy_strings_kernel(1, &bprm->filename, bprm);
- if (retval < 0) return retval;
+ if (retval < 0) return retval;
bprm->argc++;
if (i_arg) {
retval = copy_strings_kernel(1, &i_arg, bprm);
- if (retval < 0) return retval;
+ if (retval < 0) return retval;
bprm->argc++;
}
retval = copy_strings_kernel(1, &i_name, bprm);
diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
index a7641e1..887b3e5 100644
--- a/fs/binfmt_flat.c
+++ b/fs/binfmt_flat.c
@@ -388,7 +388,7 @@ void old_reloc(unsigned long rl)
#endif
flat_v2_reloc_t r;
unsigned long *ptr;
-
+
r.value = rl;
#if defined(CONFIG_COLDFIRE)
ptr = (unsigned long *) (current->mm->start_code + r.reloc.offset);
@@ -401,7 +401,7 @@ void old_reloc(unsigned long rl)
"(address %p, currently %x) into segment %s\n",
r.reloc.offset, ptr, (int)*ptr, segment[r.reloc.type]);
#endif
-
+
switch (r.reloc.type) {
case OLD_FLAT_RELOC_TYPE_TEXT:
*ptr += current->mm->start_code;
@@ -420,7 +420,7 @@ void old_reloc(unsigned long rl)
#ifdef DEBUG
printk("Relocation became %x\n", (int)*ptr);
#endif
-}
+}
/****************************************************************************/
@@ -479,7 +479,7 @@ static int load_flat_file(struct linux_binprm * bprm,
ret = -ENOEXEC;
goto err;
}
-
+
/* Don't allow old format executables to use shared libraries */
if (rev == OLD_FLAT_VERSION && id != 0) {
printk("BINFMT_FLAT: shared libraries are not available before rev 0x%x\n",
@@ -581,7 +581,7 @@ static int load_flat_file(struct linux_binprm * bprm,
fpos = ntohl(hdr->data_start);
#ifdef CONFIG_BINFMT_ZFLAT
if (flags & FLAT_FLAG_GZDATA) {
- result = decompress_exec(bprm, fpos, (char *) datapos,
+ result = decompress_exec(bprm, fpos, (char *) datapos,
full_data, 0);
} else
#endif
@@ -704,7 +704,7 @@ static int load_flat_file(struct linux_binprm * bprm,
libinfo->lib_list[id].loaded = 1;
libinfo->lib_list[id].entry = (0x00ffffff & ntohl(hdr->entry)) + textpos;
libinfo->lib_list[id].build_date = ntohl(hdr->build_date);
-
+
/*
* We just load the allocations into some temporary memory to
* help simplify all this mumbo jumbo
@@ -784,11 +784,11 @@ static int load_flat_file(struct linux_binprm * bprm,
for (i=0; i < relocs; i++)
old_reloc(ntohl(reloc[i]));
}
-
+
flush_icache_range(start_code, end_code);
/* zero the BSS, BRK and stack areas */
- memset((void*)(datapos + data_len), 0, bss_len +
+ memset((void*)(datapos + data_len), 0, bss_len +
(memp + memp_size - stack_len - /* end brk */
libinfo->lib_list[id].start_brk) + /* start brk */
stack_len);
@@ -882,11 +882,11 @@ static int load_flat_binary(struct linux_binprm * bprm)
stack_len += (bprm->argc + 1) * sizeof(char *); /* the argv array */
stack_len += (bprm->envc + 1) * sizeof(char *); /* the envp array */
stack_len += FLAT_STACK_ALIGN - 1; /* reserve for upcoming alignment */
-
+
res = load_flat_file(bprm, &libinfo, 0, &stack_len);
if (IS_ERR_VALUE(res))
return res;
-
+
/* Update data segment pointers for all libraries */
for (i=0; i<MAX_SHARED_LIBS; i++)
if (libinfo.lib_list[i].loaded)
@@ -908,7 +908,7 @@ static int load_flat_binary(struct linux_binprm * bprm)
((char *) page_address(bprm->page[i/PAGE_SIZE]))[i % PAGE_SIZE];
sp = (unsigned long *) create_flat_tables(p, bprm);
-
+
/* Fake some return addresses to ensure the call chain will
* initialise library in order for us. We are required to call
* lib 1 first, then 2, ... and finally the main program (id 0).
@@ -924,7 +924,7 @@ static int load_flat_binary(struct linux_binprm * bprm)
}
}
#endif
-
+
/* Stash our initial stack pointer into the mm structure */
current->mm->start_stack = (unsigned long )sp;
@@ -933,7 +933,7 @@ static int load_flat_binary(struct linux_binprm * bprm)
#endif
DBG_FLT("start_thread(regs=0x%x, entry=0x%x, start_stack=0x%x)\n",
(int)regs, (int)start_addr, (int)current->mm->start_stack);
-
+
start_thread(regs, start_addr, current->mm->start_stack);
return 0;
diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c
index d74afc0..8b9d701 100644
--- a/fs/binfmt_misc.c
+++ b/fs/binfmt_misc.c
@@ -62,7 +62,7 @@ static struct file_system_type bm_fs_type;
static struct vfsmount *bm_mnt;
static int entry_count;
-/*
+/*
* Check if we support the binfmt
* if we do, return the node, else NULL
* locking is done in load_misc_binary
@@ -138,12 +138,12 @@ static int load_misc_binary(struct linux_binprm *bprm)
/* if the binary should be opened on behalf of the
* interpreter than keep it open and assign descriptor
* to it */
- fd_binary = get_unused_fd();
- if (fd_binary < 0) {
- retval = fd_binary;
- goto _ret;
- }
- fd_install(fd_binary, bprm->file);
+ fd_binary = get_unused_fd();
+ if (fd_binary < 0) {
+ retval = fd_binary;
+ goto _ret;
+ }
+ fd_install(fd_binary, bprm->file);
/* if the binary is not readable than enforce mm->dumpable=0
regardless of the interpreter's permissions */
@@ -156,11 +156,11 @@ static int load_misc_binary(struct linux_binprm *bprm)
bprm->interp_flags |= BINPRM_FLAGS_EXECFD;
bprm->interp_data = fd_binary;
- } else {
- allow_write_access(bprm->file);
- fput(bprm->file);
- bprm->file = NULL;
- }
+ } else {
+ allow_write_access(bprm->file);
+ fput(bprm->file);
+ bprm->file = NULL;
+ }
/* make argv[1] be the path to the binary */
retval = copy_strings_kernel (1, &bprm->interp, bprm);
if (retval < 0)
diff --git a/fs/binfmt_script.c b/fs/binfmt_script.c
index 53f3c17..c5bac1a 100644
--- a/fs/binfmt_script.c
+++ b/fs/binfmt_script.c
@@ -45,7 +45,7 @@ static int load_script(struct linux_binprm *bprm)
break;
}
for (cp = bprm->buf+2; (*cp == ' ') || (*cp == '\t'); cp++);
- if (*cp == '\0')
+ if (*cp == '\0')
return -ENOEXEC; /* No interpreter name found */
i_name = cp;
i_arg = NULL;
@@ -70,15 +70,15 @@ static int load_script(struct linux_binprm *bprm)
if (retval)
return retval;
retval = copy_strings_kernel(1, &bprm->interp, bprm);
- if (retval < 0) return retval;
+ if (retval < 0) return retval;
bprm->argc++;
if (i_arg) {
retval = copy_strings_kernel(1, &i_arg, bprm);
- if (retval < 0) return retval;
+ if (retval < 0) return retval;
bprm->argc++;
}
retval = copy_strings_kernel(1, &i_name, bprm);
- if (retval) return retval;
+ if (retval) return retval;
bprm->argc++;
retval = bprm_change_interp(interp, bprm);
if (retval < 0)
diff --git a/fs/binfmt_som.c b/fs/binfmt_som.c
index 1c3ccc5..88af0d1 100644
--- a/fs/binfmt_som.c
+++ b/fs/binfmt_som.c
@@ -2,7 +2,7 @@
* linux/fs/binfmt_som.c
*
* These are the functions used to load SOM format executables as used
- * by HP-UX.
+ * by HP-UX.
*
* Copyright 1999 Matthew Wilcox <willy@bofh.ai>
* based on binfmt_elf which is
diff --git a/fs/exec.c b/fs/exec.c
index 54c4929..99db8b6 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -19,7 +19,7 @@
* current->executable is only used by the procfs. This allows a dispatch
* table to check for several different types of binary formats. We keep
* trying until we recognize the file or we run out of supported binary
- * formats.
+ * formats.
*/
#include <linux/slab.h>
@@ -156,7 +156,7 @@ SYSCALL_DEFINE1(uselib, const char __user *, library)
exit:
fput(file);
out:
- return error;
+ return error;
}
#ifdef CONFIG_MMU
@@ -1141,7 +1141,7 @@ void setup_new_exec(struct linux_binprm * bprm)
group */
current->self_exec_id++;
-
+
flush_signal_handlers(current, 0);
do_close_on_exec(current->files);
}
@@ -1267,8 +1267,8 @@ static int check_unsafe_exec(struct linux_binprm *bprm)
return res;
}
-/*
- * Fill the binprm structure from the inode.
+/*
+ * Fill the binprm structure from the inode.
* Check permissions, then read the first 128 (BINPRM_BUF_SIZE) bytes
*
* This may be called multiple times for binary chains (scripts for example).
--
1.7.1
^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: [PATCH v2 2/3] fs/binfmts: Better handling of binfmt loops
2013-08-02 22:12 ` [PATCH v2 2/3] fs/binfmts: Better handling of binfmt loops Zach Levis
@ 2013-08-02 22:49 ` Zach Levis
0 siblings, 0 replies; 35+ messages in thread
From: Zach Levis @ 2013-08-02 22:49 UTC (permalink / raw)
To: Zach Levis
Cc: akpm, oleg, viro, linux-kernel, linux-fsdevel, dan.carpenter, Zach Levis
On 08/02/2013 03:12 PM, Zach Levis wrote:
> + if (retval == -ELOOP && bprm->recursion_depth == 0) { /* cur, previous */
> + pr_err("Too much recursion with binfmts (0:%s, -1:%s) in file %s, skipping (base %s).\n",
> + bprm->previous_binfmts[0]->name,
> + bprm->previous_binfmts[1]->name,
> + bprm->filename,
> + fmt->name);
> +
> + /* Put argv back in its place */
> + bprm->p = bprm->p_no_argv;
> +
> + bprm->argc = count(*(bprm->argv_orig), MAX_ARG_STRINGS);
> + retval = copy_strings(bprm->argc, *(bprm->argv_orig), bprm);
> + if (retval < 0)
> + return retval;
> +
> + retval = -ENOEXEC;
> + continue;
> + }
> }
> read_unlock(&binfmt_lock);
>
NEVERMIND, I messed up the order here so this doesn't work! v3 coming up
without the stupid.
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v3 0/3] fs/binfmts: Improve handling of loops
2013-07-25 15:40 [PATCH 1/3] fs/binfmts: Add a name field to the binfmt struct Zach Levis
` (5 preceding siblings ...)
2013-08-02 22:12 ` [PATCH v2 3/3] fs/binfmts: Whitespace fixes with scripts/cleanfile Zach Levis
@ 2013-08-02 23:21 ` Zach Levis
2013-08-06 21:11 ` Kees Cook
2013-08-02 23:21 ` [PATCH v3 1/3] fs/binfmts: Add a name field to the binfmt struct Zach Levis
` (10 subsequent siblings)
17 siblings, 1 reply; 35+ messages in thread
From: Zach Levis @ 2013-08-02 23:21 UTC (permalink / raw)
To: akpm, oleg; +Cc: viro, linux-kernel, linux-fsdevel, dan.carpenter, Zach Levis
This v3 is based off Oleg's changes from "exec: more cleanups" and "exec: minor cleanups + minor fix"
It incorporates Oleg and Andrew's suggestions and takes care of the issue from Dan's patch "fs/binfmts: double unlock in search_binary_handler()"
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v3 1/3] fs/binfmts: Add a name field to the binfmt struct
2013-07-25 15:40 [PATCH 1/3] fs/binfmts: Add a name field to the binfmt struct Zach Levis
` (6 preceding siblings ...)
2013-08-02 23:21 ` [PATCH v3 0/3] fs/binfmts: Improve handling of loops Zach Levis
@ 2013-08-02 23:21 ` Zach Levis
2013-08-03 16:41 ` Oleg Nesterov
2013-08-02 23:21 ` [PATCH v3 2/3] fs/binfmts: Better handling of binfmt loops Zach Levis
` (9 subsequent siblings)
17 siblings, 1 reply; 35+ messages in thread
From: Zach Levis @ 2013-08-02 23:21 UTC (permalink / raw)
To: akpm, oleg
Cc: viro, linux-kernel, linux-fsdevel, dan.carpenter, Zach Levis, Zach Levis
Adding the name field helps when printing error messages referring to
specific binfmts
Signed-off-by: Zach Levis <zach@zachsthings.com>
Signed-off-by: Zach Levis <zml@linux.vnet.ibm.com>
---
fs/binfmt_aout.c | 1 +
fs/binfmt_elf.c | 1 +
fs/binfmt_elf_fdpic.c | 1 +
fs/binfmt_em86.c | 1 +
fs/binfmt_flat.c | 1 +
fs/binfmt_misc.c | 1 +
fs/binfmt_script.c | 1 +
fs/binfmt_som.c | 1 +
include/linux/binfmts.h | 1 +
9 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/fs/binfmt_aout.c b/fs/binfmt_aout.c
index bce8769..f6bed04 100644
--- a/fs/binfmt_aout.c
+++ b/fs/binfmt_aout.c
@@ -115,6 +115,7 @@ end_coredump:
#endif
static struct linux_binfmt aout_format = {
+ .name = "a.out",
.module = THIS_MODULE,
.load_binary = load_aout_binary,
.load_shlib = load_aout_library,
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index f8a0b0e..cd7d546 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -75,6 +75,7 @@ static int elf_core_dump(struct coredump_params *cprm);
#define ELF_PAGEALIGN(_v) (((_v) + ELF_MIN_ALIGN - 1) & ~(ELF_MIN_ALIGN - 1))
static struct linux_binfmt elf_format = {
+ .name = "ELF",
.module = THIS_MODULE,
.load_binary = load_elf_binary,
.load_shlib = load_elf_library,
diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c
index c166f32..6be4448 100644
--- a/fs/binfmt_elf_fdpic.c
+++ b/fs/binfmt_elf_fdpic.c
@@ -81,6 +81,7 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm);
#endif
static struct linux_binfmt elf_fdpic_format = {
+ .name = "FDPIC ELF",
.module = THIS_MODULE,
.load_binary = load_elf_fdpic_binary,
#ifdef CONFIG_ELF_CORE
diff --git a/fs/binfmt_em86.c b/fs/binfmt_em86.c
index 037a3e2..8404cdd 100644
--- a/fs/binfmt_em86.c
+++ b/fs/binfmt_em86.c
@@ -93,6 +93,7 @@ static int load_em86(struct linux_binprm *bprm)
}
static struct linux_binfmt em86_format = {
+ .name = "em86",
.module = THIS_MODULE,
.load_binary = load_em86,
};
diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
index d50bbe5..a7641e1 100644
--- a/fs/binfmt_flat.c
+++ b/fs/binfmt_flat.c
@@ -92,6 +92,7 @@ static int load_flat_binary(struct linux_binprm *);
static int flat_core_dump(struct coredump_params *cprm);
static struct linux_binfmt flat_format = {
+ .name = "flat",
.module = THIS_MODULE,
.load_binary = load_flat_binary,
.core_dump = flat_core_dump,
diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c
index 1c740e1..d74afc0 100644
--- a/fs/binfmt_misc.c
+++ b/fs/binfmt_misc.c
@@ -694,6 +694,7 @@ static struct dentry *bm_mount(struct file_system_type *fs_type,
}
static struct linux_binfmt misc_format = {
+ .name = "misc",
.module = THIS_MODULE,
.load_binary = load_misc_binary,
};
diff --git a/fs/binfmt_script.c b/fs/binfmt_script.c
index 5027a3e..53f3c17 100644
--- a/fs/binfmt_script.c
+++ b/fs/binfmt_script.c
@@ -99,6 +99,7 @@ static int load_script(struct linux_binprm *bprm)
}
static struct linux_binfmt script_format = {
+ .name = "script",
.module = THIS_MODULE,
.load_binary = load_script,
};
diff --git a/fs/binfmt_som.c b/fs/binfmt_som.c
index 4e00ed6..1c3ccc5 100644
--- a/fs/binfmt_som.c
+++ b/fs/binfmt_som.c
@@ -53,6 +53,7 @@ static int som_core_dump(struct coredump_params *cprm);
#define SOM_PAGEALIGN(_v) (((_v) + SOM_PAGESIZE - 1) & ~(SOM_PAGESIZE - 1))
static struct linux_binfmt som_format = {
+ .name = "SOM",
.module = THIS_MODULE,
.load_binary = load_som_binary,
.load_shlib = load_som_library,
diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
index 70cf138..402a74a 100644
--- a/include/linux/binfmts.h
+++ b/include/linux/binfmts.h
@@ -70,6 +70,7 @@ struct coredump_params {
struct linux_binfmt {
struct list_head lh;
struct module *module;
+ const char *name;
int (*load_binary)(struct linux_binprm *);
int (*load_shlib)(struct file *);
int (*core_dump)(struct coredump_params *cprm);
--
1.7.1
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v3 2/3] fs/binfmts: Better handling of binfmt loops
2013-07-25 15:40 [PATCH 1/3] fs/binfmts: Add a name field to the binfmt struct Zach Levis
` (7 preceding siblings ...)
2013-08-02 23:21 ` [PATCH v3 1/3] fs/binfmts: Add a name field to the binfmt struct Zach Levis
@ 2013-08-02 23:21 ` Zach Levis
2013-08-03 16:42 ` Oleg Nesterov
2013-08-02 23:21 ` [PATCH v3 3/3] fs/binfmts: Whitespace fixes with scripts/cleanfile Zach Levis
` (8 subsequent siblings)
17 siblings, 1 reply; 35+ messages in thread
From: Zach Levis @ 2013-08-02 23:21 UTC (permalink / raw)
To: akpm, oleg
Cc: viro, linux-kernel, linux-fsdevel, dan.carpenter, Zach Levis, Zach Levis
With these changes, when a binfmt loop is encountered,
the ELOOP will propogate back to the 0 depth. At this point the
argv and argc values will be reset to what they were originally and an
attempt is made to continue with the following binfmt handlers.
Example: a qemu is configured to run 64-bit ELFs on an otherwise 32-bit
system. The system's owner switches to running with 64-bit executables,
but forgets to disable the binfmt_misc option that redirects 64bit ELFs
to qemu. Since the qemu executable is a 64-bit ELF now, binfmt_misc
keeps on matching it with the qemu rule, preventing the execution of any
64-bit binary.
With these changes, an error is printed and search_binary_handler()
continues on to the next handler, allowing the original executable to
run normally so the user can (hopefully) fix their misconfiguration more
easily.
Caveats:
- binfmt_misc is skipped as a whole. To allow for a more generic
solution that works for any binfmt without a lot of duplicated code
and added complexity, the error detection code is applied on the
binfmt level.
- This is a fallback solution. It attempts to restore the state to allow
executing most binaries without side effects, but it may not work for
everything and should not be depended on for regular usage.
My (rough, but functional) test scripts for this issue are available at:
https://gist.github.com/zml2008/6075418
Signed-off-by: Zach Levis <zach@zachsthings.com>
Signed-off-by: Zach Levis <zml@linux.vnet.ibm.com>
---
fs/exec.c | 28 +++++++++++++++++++++++++++-
include/linux/binfmts.h | 7 ++++++-
2 files changed, 33 insertions(+), 2 deletions(-)
diff --git a/fs/exec.c b/fs/exec.c
index eb229e1..779361d 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1394,14 +1394,34 @@ int search_binary_handler(struct linux_binprm *bprm)
if (!try_module_get(fmt->module))
continue;
read_unlock(&binfmt_lock);
+ bprm->previous_binfmts[1] = bprm->previous_binfmts[0];
+ bprm->previous_binfmts[0] = fmt;
+
bprm->recursion_depth++;
retval = fmt->load_binary(bprm);
bprm->recursion_depth--;
- if (retval >= 0 || retval != -ENOEXEC ||
+ if (retval == -ELOOP && bprm->recursion_depth == 0) { /* cur, previous */
+ pr_err("Too much recursion with binfmts (0:%s, -1:%s) in file %s, skipping (base %s).\n",
+ bprm->previous_binfmts[0]->name,
+ bprm->previous_binfmts[1]->name,
+ bprm->filename,
+ fmt->name);
+
+ /* Put argv back in its place */
+ bprm->p = bprm->p_no_argv;
+
+ bprm->argc = count(*(bprm->argv_orig), MAX_ARG_STRINGS);
+ retval = copy_strings(bprm->argc, *(bprm->argv_orig), bprm);
+ if (retval < 0)
+ return retval;
+
+ retval = -ENOEXEC;
+ } else if (retval >= 0 || retval != -ENOEXEC ||
bprm->mm == NULL || bprm->file == NULL) {
put_binfmt(fmt);
return retval;
}
+
read_lock(&binfmt_lock);
put_binfmt(fmt);
}
@@ -1436,6 +1456,10 @@ static int exec_binprm(struct linux_binprm *bprm)
if (ret >= 0) {
trace_sched_process_exec(current, old_pid, bprm);
ptrace_event(PTRACE_EVENT_EXEC, old_vpid);
+ /* Successful execution, now null out the cached argv
+ * (we don't want to access it later)
+ * */
+ bprm->argv_orig = NULL;
current->did_exec = 1;
proc_exec_connector(current);
@@ -1533,9 +1557,11 @@ static int do_execve_common(const char *filename,
if (retval < 0)
goto out;
+ bprm->p_no_argv = bprm->p;
retval = copy_strings(bprm->argc, argv, bprm);
if (retval < 0)
goto out;
+ bprm->argv_orig = &argv;
retval = exec_binprm(bprm);
if (retval < 0)
diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
index 402a74a..42c1656 100644
--- a/include/linux/binfmts.h
+++ b/include/linux/binfmts.h
@@ -8,6 +8,8 @@
#define CORENAME_MAX_SIZE 128
+struct user_arg_ptr; /* Struct from fs/exec.c, for linux_binprm->argv_orig */
+
/*
* This structure is used to hold the arguments that are used when loading binaries.
*/
@@ -21,7 +23,7 @@ struct linux_binprm {
struct page *page[MAX_ARG_PAGES];
#endif
struct mm_struct *mm;
- unsigned long p; /* current top of mem */
+ unsigned long p, p_no_argv; /* current top of mem */
unsigned int
cred_prepared:1,/* true if creds already prepared (multiple
* preps happen for interpreters) */
@@ -32,11 +34,14 @@ struct linux_binprm {
unsigned int taso:1;
#endif
unsigned int recursion_depth;
+ /* current binfmt at 0 and previous binfmt at 1 */
+ struct linux_binfmt *previous_binfmts[2];
struct file * file;
struct cred *cred; /* new credentials */
int unsafe; /* how unsafe this exec is (mask of LSM_UNSAFE_*) */
unsigned int per_clear; /* bits to clear in current->personality */
int argc, envc;
+ struct user_arg_ptr *argv_orig;
const char * filename; /* Name of binary as seen by procps */
const char * interp; /* Name of the binary really executed. Most
of the time same as filename, but could be
--
1.7.1
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v3 3/3] fs/binfmts: Whitespace fixes with scripts/cleanfile
2013-07-25 15:40 [PATCH 1/3] fs/binfmts: Add a name field to the binfmt struct Zach Levis
` (8 preceding siblings ...)
2013-08-02 23:21 ` [PATCH v3 2/3] fs/binfmts: Better handling of binfmt loops Zach Levis
@ 2013-08-02 23:21 ` Zach Levis
2013-08-03 16:51 ` Oleg Nesterov
2013-08-14 16:31 ` [PATCH v4 0/3] fs/binfmts: Improve handling of loops Zach Levis
` (7 subsequent siblings)
17 siblings, 1 reply; 35+ messages in thread
From: Zach Levis @ 2013-08-02 23:21 UTC (permalink / raw)
To: akpm, oleg
Cc: viro, linux-kernel, linux-fsdevel, dan.carpenter, Zach Levis, Zach Levis
Obligatory first-patchset whitespace commit
Signed-off-by: Zach Levis <zach@zachsthings.com>
Signed-off-by: Zach Levis <zml@linux.vnet.ibm.com>
---
fs/binfmt_aout.c | 8 ++++----
fs/binfmt_elf.c | 38 +++++++++++++++++++-------------------
fs/binfmt_elf_fdpic.c | 8 ++++----
fs/binfmt_em86.c | 4 ++--
fs/binfmt_flat.c | 26 +++++++++++++-------------
fs/binfmt_misc.c | 24 ++++++++++++------------
fs/binfmt_script.c | 8 ++++----
fs/binfmt_som.c | 2 +-
fs/exec.c | 10 +++++-----
9 files changed, 64 insertions(+), 64 deletions(-)
diff --git a/fs/binfmt_aout.c b/fs/binfmt_aout.c
index f6bed04..c0a458b 100644
--- a/fs/binfmt_aout.c
+++ b/fs/binfmt_aout.c
@@ -62,7 +62,7 @@ static int aout_core_dump(struct coredump_params *cprm)
fs = get_fs();
set_fs(KERNEL_DS);
has_dumped = 1;
- strncpy(dump.u_comm, current->comm, sizeof(dump.u_comm));
+ strncpy(dump.u_comm, current->comm, sizeof(dump.u_comm));
dump.u_ar0 = offsetof(struct user, regs);
dump.signal = cprm->siginfo->si_signo;
aout_dump_thread(cprm->regs, &dump);
@@ -302,7 +302,7 @@ static int load_aout_binary(struct linux_binprm * bprm)
if ((fd_offset & ~PAGE_MASK) != 0 && printk_ratelimit())
{
- printk(KERN_WARNING
+ printk(KERN_WARNING
"fd_offset is not page aligned. Please convert program: %s\n",
bprm->file->f_path.dentry->d_name.name);
}
@@ -391,12 +391,12 @@ static int load_aout_library(struct file *file)
if ((N_TXTOFF(ex) & ~PAGE_MASK) != 0) {
if (printk_ratelimit())
{
- printk(KERN_WARNING
+ printk(KERN_WARNING
"N_TXTOFF is not page aligned. Please convert library: %s\n",
file->f_path.dentry->d_name.name);
}
vm_brk(start_addr, ex.a_text + ex.a_data + ex.a_bss);
-
+
read_code(file, start_addr, N_TXTOFF(ex),
ex.a_text + ex.a_data);
retval = 0;
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index cd7d546..9b0b372 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -218,7 +218,7 @@ create_elf_tables(struct linux_binprm *bprm, struct elfhdr *exec,
} while (0)
#ifdef ARCH_DLINFO
- /*
+ /*
* ARCH_DLINFO must come first so PPC can do its special alignment of
* AUXV.
* update AT_VECTOR_SIZE_ARCH if the number of NEW_AUX_ENT() in
@@ -239,7 +239,7 @@ create_elf_tables(struct linux_binprm *bprm, struct elfhdr *exec,
NEW_AUX_ENT(AT_EUID, from_kuid_munged(cred->user_ns, cred->euid));
NEW_AUX_ENT(AT_GID, from_kgid_munged(cred->user_ns, cred->gid));
NEW_AUX_ENT(AT_EGID, from_kgid_munged(cred->user_ns, cred->egid));
- NEW_AUX_ENT(AT_SECURE, security_bprm_secureexec(bprm));
+ NEW_AUX_ENT(AT_SECURE, security_bprm_secureexec(bprm));
NEW_AUX_ENT(AT_RANDOM, (elf_addr_t)(unsigned long)u_rand_bytes);
#ifdef ELF_HWCAP2
NEW_AUX_ENT(AT_HWCAP2, ELF_HWCAP2);
@@ -433,7 +433,7 @@ static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex,
error = -EIO;
if (retval != size) {
if (retval < 0)
- error = retval;
+ error = retval;
goto out_close;
}
@@ -452,7 +452,7 @@ static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex,
unsigned long k, map_addr;
if (eppnt->p_flags & PF_R)
- elf_prot = PROT_READ;
+ elf_prot = PROT_READ;
if (eppnt->p_flags & PF_W)
elf_prot |= PROT_WRITE;
if (eppnt->p_flags & PF_X)
@@ -570,7 +570,7 @@ static unsigned long randomize_stack_top(unsigned long stack_top)
static int load_elf_binary(struct linux_binprm *bprm)
{
struct file *interpreter = NULL; /* to shut gcc up */
- unsigned long load_addr = 0, load_bias = 0;
+ unsigned long load_addr = 0, load_bias = 0;
int load_addr_set = 0;
char * elf_interpreter = NULL;
unsigned long error;
@@ -595,7 +595,7 @@ static int load_elf_binary(struct linux_binprm *bprm)
retval = -ENOMEM;
goto out_ret;
}
-
+
/* Get the exec-header */
loc->elf_ex = *((struct elfhdr *)bprm->buf);
@@ -615,7 +615,7 @@ static int load_elf_binary(struct linux_binprm *bprm)
if (loc->elf_ex.e_phentsize != sizeof(struct elf_phdr))
goto out;
if (loc->elf_ex.e_phnum < 1 ||
- loc->elf_ex.e_phnum > 65536U / sizeof(struct elf_phdr))
+ loc->elf_ex.e_phnum > 65536U / sizeof(struct elf_phdr))
goto out;
size = loc->elf_ex.e_phnum * sizeof(struct elf_phdr);
retval = -ENOMEM;
@@ -647,7 +647,7 @@ static int load_elf_binary(struct linux_binprm *bprm)
* is an a.out format binary
*/
retval = -ENOEXEC;
- if (elf_ppnt->p_filesz > PATH_MAX ||
+ if (elf_ppnt->p_filesz > PATH_MAX ||
elf_ppnt->p_filesz < 2)
goto out_free_ph;
@@ -747,7 +747,7 @@ static int load_elf_binary(struct linux_binprm *bprm)
send_sig(SIGKILL, current, 0);
goto out_free_dentry;
}
-
+
current->mm->start_stack = bprm->p;
/* Now we do a little grungy work by mmapping the ELF image into
@@ -762,7 +762,7 @@ static int load_elf_binary(struct linux_binprm *bprm)
if (unlikely (elf_brk > elf_bss)) {
unsigned long nbyte;
-
+
/* There was a PT_LOAD segment with p_memsz > p_filesz
before this one. Map anonymous pages, if needed,
and clear the area. */
@@ -1294,7 +1294,7 @@ static void fill_elf_note_phdr(struct elf_phdr *phdr, int sz, loff_t offset)
return;
}
-static void fill_note(struct memelfnote *note, const char *name, int type,
+static void fill_note(struct memelfnote *note, const char *name, int type,
unsigned int sz, void *data)
{
note->name = name;
@@ -1346,7 +1346,7 @@ static int fill_psinfo(struct elf_prpsinfo *psinfo, struct task_struct *p,
{
const struct cred *cred;
unsigned int i, len;
-
+
/* first copy the parameters from user space */
memset(psinfo, 0, sizeof(struct elf_prpsinfo));
@@ -1380,7 +1380,7 @@ static int fill_psinfo(struct elf_prpsinfo *psinfo, struct task_struct *p,
SET_GID(psinfo->pr_gid, from_kgid_munged(cred->user_ns, cred->gid));
rcu_read_unlock();
strncpy(psinfo->pr_fname, p->comm, sizeof(psinfo->pr_fname));
-
+
return 0;
}
@@ -1781,8 +1781,8 @@ static int elf_dump_thread_status(long signr, struct elf_thread_status *t)
t->num_notes = 0;
fill_prstatus(&t->prstatus, p, signr);
- elf_core_copy_task_regs(p, &t->prstatus.pr_reg);
-
+ elf_core_copy_task_regs(p, &t->prstatus.pr_reg);
+
fill_note(&t->notes[0], "CORE", NT_PRSTATUS, sizeof(t->prstatus),
&(t->prstatus));
t->num_notes++;
@@ -1803,7 +1803,7 @@ static int elf_dump_thread_status(long signr, struct elf_thread_status *t)
t->num_notes++;
sz += notesize(&t->notes[2]);
}
-#endif
+#endif
return sz;
}
@@ -2055,7 +2055,7 @@ static int elf_core_dump(struct coredump_params *cprm)
/*
* We no longer stop all VM operations.
- *
+ *
* This is because those proceses that could possibly change map_count
* or the mmap / vma pages are now blocked in do_exit on current
* finishing this core dump.
@@ -2064,7 +2064,7 @@ static int elf_core_dump(struct coredump_params *cprm)
* the map_count or the pages allocated. So no possibility of crashing
* exists while dumping the mm->vm_next areas to the core file.
*/
-
+
/* alloc memory for large data structures: too large to be on stack */
elf = kmalloc(sizeof(*elf), GFP_KERNEL);
if (!elf)
@@ -2170,7 +2170,7 @@ static int elf_core_dump(struct coredump_params *cprm)
if (!elf_core_write_extra_phdrs(cprm->file, offset, &size, cprm->limit))
goto end_coredump;
- /* write out the notes section */
+ /* write out the notes section */
if (!write_note_info(&info, cprm->file, &foffset))
goto end_coredump;
diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c
index 6be4448..799791b 100644
--- a/fs/binfmt_elf_fdpic.c
+++ b/fs/binfmt_elf_fdpic.c
@@ -1595,8 +1595,8 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm)
struct memelfnote *notes = NULL;
struct elf_prstatus *prstatus = NULL; /* NT_PRSTATUS */
struct elf_prpsinfo *psinfo = NULL; /* NT_PRPSINFO */
- LIST_HEAD(thread_list);
- struct list_head *t;
+ LIST_HEAD(thread_list);
+ struct list_head *t;
elf_fpregset_t *fpu = NULL;
#ifdef ELF_CORE_COPY_XFPREGS
elf_fpxregset_t *xfpu = NULL;
@@ -1705,7 +1705,7 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm)
fill_note(¬es[numnote++], "CORE", NT_AUXV,
i * sizeof(elf_addr_t), auxv);
- /* Try to dump the FPU. */
+ /* Try to dump the FPU. */
if ((prstatus->pr_fpvalid =
elf_core_copy_task_fpregs(current, cprm->regs, fpu)))
fill_note(notes + numnote++,
@@ -1795,7 +1795,7 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm)
if (!elf_core_write_extra_phdrs(cprm->file, offset, &size, cprm->limit))
goto end_coredump;
- /* write out the notes section */
+ /* write out the notes section */
for (i = 0; i < numnote; i++)
if (!writenote(notes + i, cprm->file, &foffset))
goto end_coredump;
diff --git a/fs/binfmt_em86.c b/fs/binfmt_em86.c
index 8404cdd..0e58293 100644
--- a/fs/binfmt_em86.c
+++ b/fs/binfmt_em86.c
@@ -63,11 +63,11 @@ static int load_em86(struct linux_binprm *bprm)
*/
remove_arg_zero(bprm);
retval = copy_strings_kernel(1, &bprm->filename, bprm);
- if (retval < 0) return retval;
+ if (retval < 0) return retval;
bprm->argc++;
if (i_arg) {
retval = copy_strings_kernel(1, &i_arg, bprm);
- if (retval < 0) return retval;
+ if (retval < 0) return retval;
bprm->argc++;
}
retval = copy_strings_kernel(1, &i_name, bprm);
diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
index a7641e1..887b3e5 100644
--- a/fs/binfmt_flat.c
+++ b/fs/binfmt_flat.c
@@ -388,7 +388,7 @@ void old_reloc(unsigned long rl)
#endif
flat_v2_reloc_t r;
unsigned long *ptr;
-
+
r.value = rl;
#if defined(CONFIG_COLDFIRE)
ptr = (unsigned long *) (current->mm->start_code + r.reloc.offset);
@@ -401,7 +401,7 @@ void old_reloc(unsigned long rl)
"(address %p, currently %x) into segment %s\n",
r.reloc.offset, ptr, (int)*ptr, segment[r.reloc.type]);
#endif
-
+
switch (r.reloc.type) {
case OLD_FLAT_RELOC_TYPE_TEXT:
*ptr += current->mm->start_code;
@@ -420,7 +420,7 @@ void old_reloc(unsigned long rl)
#ifdef DEBUG
printk("Relocation became %x\n", (int)*ptr);
#endif
-}
+}
/****************************************************************************/
@@ -479,7 +479,7 @@ static int load_flat_file(struct linux_binprm * bprm,
ret = -ENOEXEC;
goto err;
}
-
+
/* Don't allow old format executables to use shared libraries */
if (rev == OLD_FLAT_VERSION && id != 0) {
printk("BINFMT_FLAT: shared libraries are not available before rev 0x%x\n",
@@ -581,7 +581,7 @@ static int load_flat_file(struct linux_binprm * bprm,
fpos = ntohl(hdr->data_start);
#ifdef CONFIG_BINFMT_ZFLAT
if (flags & FLAT_FLAG_GZDATA) {
- result = decompress_exec(bprm, fpos, (char *) datapos,
+ result = decompress_exec(bprm, fpos, (char *) datapos,
full_data, 0);
} else
#endif
@@ -704,7 +704,7 @@ static int load_flat_file(struct linux_binprm * bprm,
libinfo->lib_list[id].loaded = 1;
libinfo->lib_list[id].entry = (0x00ffffff & ntohl(hdr->entry)) + textpos;
libinfo->lib_list[id].build_date = ntohl(hdr->build_date);
-
+
/*
* We just load the allocations into some temporary memory to
* help simplify all this mumbo jumbo
@@ -784,11 +784,11 @@ static int load_flat_file(struct linux_binprm * bprm,
for (i=0; i < relocs; i++)
old_reloc(ntohl(reloc[i]));
}
-
+
flush_icache_range(start_code, end_code);
/* zero the BSS, BRK and stack areas */
- memset((void*)(datapos + data_len), 0, bss_len +
+ memset((void*)(datapos + data_len), 0, bss_len +
(memp + memp_size - stack_len - /* end brk */
libinfo->lib_list[id].start_brk) + /* start brk */
stack_len);
@@ -882,11 +882,11 @@ static int load_flat_binary(struct linux_binprm * bprm)
stack_len += (bprm->argc + 1) * sizeof(char *); /* the argv array */
stack_len += (bprm->envc + 1) * sizeof(char *); /* the envp array */
stack_len += FLAT_STACK_ALIGN - 1; /* reserve for upcoming alignment */
-
+
res = load_flat_file(bprm, &libinfo, 0, &stack_len);
if (IS_ERR_VALUE(res))
return res;
-
+
/* Update data segment pointers for all libraries */
for (i=0; i<MAX_SHARED_LIBS; i++)
if (libinfo.lib_list[i].loaded)
@@ -908,7 +908,7 @@ static int load_flat_binary(struct linux_binprm * bprm)
((char *) page_address(bprm->page[i/PAGE_SIZE]))[i % PAGE_SIZE];
sp = (unsigned long *) create_flat_tables(p, bprm);
-
+
/* Fake some return addresses to ensure the call chain will
* initialise library in order for us. We are required to call
* lib 1 first, then 2, ... and finally the main program (id 0).
@@ -924,7 +924,7 @@ static int load_flat_binary(struct linux_binprm * bprm)
}
}
#endif
-
+
/* Stash our initial stack pointer into the mm structure */
current->mm->start_stack = (unsigned long )sp;
@@ -933,7 +933,7 @@ static int load_flat_binary(struct linux_binprm * bprm)
#endif
DBG_FLT("start_thread(regs=0x%x, entry=0x%x, start_stack=0x%x)\n",
(int)regs, (int)start_addr, (int)current->mm->start_stack);
-
+
start_thread(regs, start_addr, current->mm->start_stack);
return 0;
diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c
index d74afc0..8b9d701 100644
--- a/fs/binfmt_misc.c
+++ b/fs/binfmt_misc.c
@@ -62,7 +62,7 @@ static struct file_system_type bm_fs_type;
static struct vfsmount *bm_mnt;
static int entry_count;
-/*
+/*
* Check if we support the binfmt
* if we do, return the node, else NULL
* locking is done in load_misc_binary
@@ -138,12 +138,12 @@ static int load_misc_binary(struct linux_binprm *bprm)
/* if the binary should be opened on behalf of the
* interpreter than keep it open and assign descriptor
* to it */
- fd_binary = get_unused_fd();
- if (fd_binary < 0) {
- retval = fd_binary;
- goto _ret;
- }
- fd_install(fd_binary, bprm->file);
+ fd_binary = get_unused_fd();
+ if (fd_binary < 0) {
+ retval = fd_binary;
+ goto _ret;
+ }
+ fd_install(fd_binary, bprm->file);
/* if the binary is not readable than enforce mm->dumpable=0
regardless of the interpreter's permissions */
@@ -156,11 +156,11 @@ static int load_misc_binary(struct linux_binprm *bprm)
bprm->interp_flags |= BINPRM_FLAGS_EXECFD;
bprm->interp_data = fd_binary;
- } else {
- allow_write_access(bprm->file);
- fput(bprm->file);
- bprm->file = NULL;
- }
+ } else {
+ allow_write_access(bprm->file);
+ fput(bprm->file);
+ bprm->file = NULL;
+ }
/* make argv[1] be the path to the binary */
retval = copy_strings_kernel (1, &bprm->interp, bprm);
if (retval < 0)
diff --git a/fs/binfmt_script.c b/fs/binfmt_script.c
index 53f3c17..c5bac1a 100644
--- a/fs/binfmt_script.c
+++ b/fs/binfmt_script.c
@@ -45,7 +45,7 @@ static int load_script(struct linux_binprm *bprm)
break;
}
for (cp = bprm->buf+2; (*cp == ' ') || (*cp == '\t'); cp++);
- if (*cp == '\0')
+ if (*cp == '\0')
return -ENOEXEC; /* No interpreter name found */
i_name = cp;
i_arg = NULL;
@@ -70,15 +70,15 @@ static int load_script(struct linux_binprm *bprm)
if (retval)
return retval;
retval = copy_strings_kernel(1, &bprm->interp, bprm);
- if (retval < 0) return retval;
+ if (retval < 0) return retval;
bprm->argc++;
if (i_arg) {
retval = copy_strings_kernel(1, &i_arg, bprm);
- if (retval < 0) return retval;
+ if (retval < 0) return retval;
bprm->argc++;
}
retval = copy_strings_kernel(1, &i_name, bprm);
- if (retval) return retval;
+ if (retval) return retval;
bprm->argc++;
retval = bprm_change_interp(interp, bprm);
if (retval < 0)
diff --git a/fs/binfmt_som.c b/fs/binfmt_som.c
index 1c3ccc5..88af0d1 100644
--- a/fs/binfmt_som.c
+++ b/fs/binfmt_som.c
@@ -2,7 +2,7 @@
* linux/fs/binfmt_som.c
*
* These are the functions used to load SOM format executables as used
- * by HP-UX.
+ * by HP-UX.
*
* Copyright 1999 Matthew Wilcox <willy@bofh.ai>
* based on binfmt_elf which is
diff --git a/fs/exec.c b/fs/exec.c
index 779361d..272ce09 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -19,7 +19,7 @@
* current->executable is only used by the procfs. This allows a dispatch
* table to check for several different types of binary formats. We keep
* trying until we recognize the file or we run out of supported binary
- * formats.
+ * formats.
*/
#include <linux/slab.h>
@@ -156,7 +156,7 @@ SYSCALL_DEFINE1(uselib, const char __user *, library)
exit:
fput(file);
out:
- return error;
+ return error;
}
#ifdef CONFIG_MMU
@@ -1141,7 +1141,7 @@ void setup_new_exec(struct linux_binprm * bprm)
group */
current->self_exec_id++;
-
+
flush_signal_handlers(current, 0);
do_close_on_exec(current->files);
}
@@ -1267,8 +1267,8 @@ static int check_unsafe_exec(struct linux_binprm *bprm)
return res;
}
-/*
- * Fill the binprm structure from the inode.
+/*
+ * Fill the binprm structure from the inode.
* Check permissions, then read the first 128 (BINPRM_BUF_SIZE) bytes
*
* This may be called multiple times for binary chains (scripts for example).
--
1.7.1
^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: [PATCH v3 1/3] fs/binfmts: Add a name field to the binfmt struct
2013-08-02 23:21 ` [PATCH v3 1/3] fs/binfmts: Add a name field to the binfmt struct Zach Levis
@ 2013-08-03 16:41 ` Oleg Nesterov
0 siblings, 0 replies; 35+ messages in thread
From: Oleg Nesterov @ 2013-08-03 16:41 UTC (permalink / raw)
To: Zach Levis
Cc: akpm, viro, linux-kernel, linux-fsdevel, dan.carpenter, Zach Levis
On 08/02, Zach Levis wrote:
>
> static struct linux_binfmt elf_format = {
> + .name = "ELF",
Once again, I think it would be better if elf_format and compat_elf_format
have the different names. But with this patch they are both "ELF".
Oleg.
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v3 2/3] fs/binfmts: Better handling of binfmt loops
2013-08-02 23:21 ` [PATCH v3 2/3] fs/binfmts: Better handling of binfmt loops Zach Levis
@ 2013-08-03 16:42 ` Oleg Nesterov
0 siblings, 0 replies; 35+ messages in thread
From: Oleg Nesterov @ 2013-08-03 16:42 UTC (permalink / raw)
To: Zach Levis
Cc: akpm, viro, linux-kernel, linux-fsdevel, dan.carpenter, Zach Levis
On 08/02, Zach Levis wrote:
>
> With these changes, when a binfmt loop is encountered,
> the ELOOP will propogate back to the 0 depth. At this point the
> argv and argc values will be reset to what they were originally and an
> attempt is made to continue with the following binfmt handlers.
>
> Example: a qemu is configured to run 64-bit ELFs on an otherwise 32-bit
> system. The system's owner switches to running with 64-bit executables,
> but forgets to disable the binfmt_misc option that redirects 64bit ELFs
> to qemu. Since the qemu executable is a 64-bit ELF now, binfmt_misc
> keeps on matching it with the qemu rule, preventing the execution of any
> 64-bit binary.
>
> With these changes, an error is printed and search_binary_handler()
> continues on to the next handler, allowing the original executable to
> run normally so the user can (hopefully) fix their misconfiguration more
> easily.
Well. To be honest, I still can't say I like this change.
I won't argue, but I would really like if someone else can ack/nack
the intent.
As for correctness, please see below.
> @@ -1394,14 +1394,34 @@ int search_binary_handler(struct linux_binprm *bprm)
> if (!try_module_get(fmt->module))
> continue;
> read_unlock(&binfmt_lock);
> + bprm->previous_binfmts[1] = bprm->previous_binfmts[0];
> + bprm->previous_binfmts[0] = fmt;
> +
> bprm->recursion_depth++;
> retval = fmt->load_binary(bprm);
> bprm->recursion_depth--;
> - if (retval >= 0 || retval != -ENOEXEC ||
> + if (retval == -ELOOP && bprm->recursion_depth == 0) { /* cur, previous */
> + pr_err("Too much recursion with binfmts (0:%s, -1:%s) in file %s, skipping (base %s).\n",
> + bprm->previous_binfmts[0]->name,
> + bprm->previous_binfmts[1]->name,
This doesn't look safe... previous_binfmts[0] == fmt is fine, but
previous_binfmts[1] can be already unloaded.
> + bprm->filename,
> + fmt->name);
> +
> + /* Put argv back in its place */
> + bprm->p = bprm->p_no_argv;
> +
> + bprm->argc = count(*(bprm->argv_orig), MAX_ARG_STRINGS);
This can fail too.
> + retval = copy_strings(bprm->argc, *(bprm->argv_orig), bprm);
> + if (retval < 0)
> + return retval;
This lacks put_binfmt().
And we should probably also check bprm->file != NULL and mm != NULL to
ensure it is safe to continue (->mm check is probably unneeded, but we
should either do it anyway or remove another one).
> + retval = -ENOEXEC;
Hmm, why?
If this fmt is not last, retval will be changed anyway. Otherwise we
are going to return the error, and -ELOOP obviously makes more sense?
And in fact we should return -ELOOP even if this fmt is not last (unless
another linux_binfmt succceeds, of course). But with this patch ELOOP will
be translated into ENOEXEC, not good.
And once again, why do we need this? Afaics it only can help to "fix" the
misconfigured binfmt_misc.c. So perhaps it would be better to change
load_misc_binary() to detect the loop, do copy_strings() again (we can
add the helper for this to avoid the extra exports) and return -ENOEXEC?
And in this case you do not need previous_binfmts[], you can just do
WARN_ON() which shows the stack.
Note that in general this logic does not look right or I missed something.
OK, we restored argc/argv. But what about binfmt->file/buf ?
Suppose that ->load_script() is called with recursion_depth == 5. It reads
->buf which finally it points to, say, elf binary.
So it does bprm->file = open_exec(interp) + prepare_binprm(), and calls
search_binary_handler() again which results in -ELOOP.
This -ELOOP is propagated up to recursion_depth == 0. We restore argv
and continue.
If the next fmt is elf_format it can happily load this elf binary.
No?
> @@ -1436,6 +1456,10 @@ static int exec_binprm(struct linux_binprm *bprm)
> if (ret >= 0) {
> trace_sched_process_exec(current, old_pid, bprm);
> ptrace_event(PTRACE_EVENT_EXEC, old_vpid);
> + /* Successful execution, now null out the cached argv
> + * (we don't want to access it later)
Yes, and now that we have exec_binprm() this is obvious, so
> + bprm->argv_orig = NULL;
why do we need to reset it?
> @@ -1533,9 +1557,11 @@ static int do_execve_common(const char *filename,
> if (retval < 0)
> goto out;
>
> + bprm->p_no_argv = bprm->p;
> retval = copy_strings(bprm->argc, argv, bprm);
> if (retval < 0)
> goto out;
> + bprm->argv_orig = &argv;
purely cosmetic, but you can initialize both p_no_argv and argv_orig
in one place, but I won't insist.
Oleg.
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v3 3/3] fs/binfmts: Whitespace fixes with scripts/cleanfile
2013-08-02 23:21 ` [PATCH v3 3/3] fs/binfmts: Whitespace fixes with scripts/cleanfile Zach Levis
@ 2013-08-03 16:51 ` Oleg Nesterov
0 siblings, 0 replies; 35+ messages in thread
From: Oleg Nesterov @ 2013-08-03 16:51 UTC (permalink / raw)
To: Zach Levis
Cc: akpm, viro, linux-kernel, linux-fsdevel, dan.carpenter, Zach Levis
On 08/02, Zach Levis wrote:
>
> Obligatory first-patchset whitespace commit
Looks good, perhaps you send it as 1/3 or even separately, so you
won't need to redo it if you need to update 1-2.
> @@ -63,11 +63,11 @@ static int load_em86(struct linux_binprm *bprm)
> */
> remove_arg_zero(bprm);
> retval = copy_strings_kernel(1, &bprm->filename, bprm);
> - if (retval < 0) return retval;
> + if (retval < 0) return retval;
And perhaps you can also fix the coding style when your patch touches
such a code.
Oleg.
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v3 0/3] fs/binfmts: Improve handling of loops
2013-08-02 23:21 ` [PATCH v3 0/3] fs/binfmts: Improve handling of loops Zach Levis
@ 2013-08-06 21:11 ` Kees Cook
2013-08-07 23:30 ` Zach Levis
0 siblings, 1 reply; 35+ messages in thread
From: Kees Cook @ 2013-08-06 21:11 UTC (permalink / raw)
To: Zach Levis; +Cc: akpm, oleg, viro, linux-kernel, linux-fsdevel, dan.carpenter
Hi Zach,
I like the idea behind these clean ups. Thanks for working on them!
On Fri, Aug 02, 2013 at 04:21:40PM -0700, Zach Levis wrote:
> This v3 is based off Oleg's changes from "exec: more cleanups" and
> "exec: minor cleanups + minor fix"
I would echo all of Oleg's comments on the series so far. Additionally,
please use "scripts/checkpatch.pl" for checking your patches for common
errors (see item 4 in Documentation/SubmittingPatches). I see a number
of problems that are detected by the tool:
WARNING: line over 80 characters
#52: FILE: fs/exec.c:1403:
+ if (retval == -ELOOP && bprm->recursion_depth == 0) { /* cur, previous */
...
ERROR: trailing statements should be on next line
#269: FILE: fs/binfmt_em86.c:70:
+ if (retval < 0) return retval;
...
ERROR: "(foo*)" should be "(foo *)"
#341: FILE: fs/binfmt_flat.c:791:
+ memset((void*)(datapos + data_len), 0, bss_len +
etc.
After that, be sure to use "scripts/get_maintainer.pl" for generating
your CC list (see item 5 in SubmittingPatches; I initially missed this
series -- adding more CCs for people that have touched the code can help
with your reviews).
Also, you only need to include a single Signed-off-by line for
yourself. :)
> It incorporates Oleg and Andrew's suggestions and takes care
> of the issue from Dan's patch "fs/binfmts: double unlock in
> search_binary_handler()"
In the commit message, can you include some examples of how to generate
the loops you're encountering? This helps people understand why you're
doing what you're doing and provides a way for people to reproduce the
conditions themselves.
Thanks,
-Kees
--
Kees Cook
Chrome OS Security
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v3 0/3] fs/binfmts: Improve handling of loops
2013-08-06 21:11 ` Kees Cook
@ 2013-08-07 23:30 ` Zach Levis
0 siblings, 0 replies; 35+ messages in thread
From: Zach Levis @ 2013-08-07 23:30 UTC (permalink / raw)
To: Kees Cook
Cc: akpm, oleg, viro, linux-kernel, linux-fsdevel, dan.carpenter, Zach Levis
On 08/06/2013 02:11 PM, Kees Cook wrote:
> Hi Zach,
>
> I like the idea behind these clean ups. Thanks for working on them!
>
> On Fri, Aug 02, 2013 at 04:21:40PM -0700, Zach Levis wrote:
>> This v3 is based off Oleg's changes from "exec: more cleanups" and
>> "exec: minor cleanups + minor fix"
>
> I would echo all of Oleg's comments on the series so far. Additionally,
working on that. v4 will take a little longer since I'm working on some
more significant changes to make sure I reset all of the possible data
in the linux_binprm struct and reuse the code from do_execve_common for
that as much as possible.
> please use "scripts/checkpatch.pl" for checking your patches for common
> errors (see item 4 in Documentation/SubmittingPatches). I see a number
> of problems that are detected by the tool:
>
> WARNING: line over 80 characters
> #52: FILE: fs/exec.c:1403:
> + if (retval == -ELOOP && bprm->recursion_depth == 0) { /* cur, previous */
>
> ...
>
> ERROR: trailing statements should be on next line
> #269: FILE: fs/binfmt_em86.c:70:
> + if (retval < 0) return retval;
>
> ...
>
> ERROR: "(foo*)" should be "(foo *)"
> #341: FILE: fs/binfmt_flat.c:791:
> + memset((void*)(datapos + data_len), 0, bss_len +
>
> etc.
>
will do
> After that, be sure to use "scripts/get_maintainer.pl" for generating
> your CC list (see item 5 in SubmittingPatches; I initially missed this
> series -- adding more CCs for people that have touched the code can help
> with your reviews).
Sorry about that. Will do in the future
>
> Also, you only need to include a single Signed-off-by line for
> yourself. :)
Well, then it seems I'll be sending the next rev from my personal email,
seeing as I've only got a week and a half left on my internship here
(and until I lose access to my IBM email).
>
>> It incorporates Oleg and Andrew's suggestions and takes care
>> of the issue from Dan's patch "fs/binfmts: double unlock in
>> search_binary_handler()"
>
> In the commit message, can you include some examples of how to generate
> the loops you're encountering? This helps people understand why you're
> doing what you're doing and provides a way for people to reproduce the
> conditions themselves.
Commit 2/3 has a link to a gist with testcases for the scripts -- I
stuck them in a gist so they didn't clog up the commit message. If
there's a way you'd prefer me to reference them let me know.
>
> Thanks,
>
> -Kees
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v4 0/3] fs/binfmts: Improve handling of loops
2013-07-25 15:40 [PATCH 1/3] fs/binfmts: Add a name field to the binfmt struct Zach Levis
` (9 preceding siblings ...)
2013-08-02 23:21 ` [PATCH v3 3/3] fs/binfmts: Whitespace fixes with scripts/cleanfile Zach Levis
@ 2013-08-14 16:31 ` Zach Levis
2013-08-14 16:31 ` [PATCH v4 1/3] fs/binfmts: Add a name field to the binfmt struct Zach Levis
` (6 subsequent siblings)
17 siblings, 0 replies; 35+ messages in thread
From: Zach Levis @ 2013-08-14 16:31 UTC (permalink / raw)
To: akpm, oleg
Cc: viro, linux-kernel, linux-fsdevel, dan.carpenter, keescook, cody,
zml, Zach Levis
This v4 is based off Oleg's changes from "exec: more cleanups" and "exec: minor cleanups + minor fix"
It incorporates Oleg and Andrew's suggestions and takes care of the issue from Dan's patch "fs/binfmts: double unlock in search_binary_handler()"
New in v4 is how we handle resetting the linux_binprm data. The same function (init_bprm) is called for the initial do_execve_common and recovering from a loop, making sure that any data that needs to be changed is changed.
Additionally, compat_binfmt_elf gets its own name.
(btw, I'd really like to rename that file to binfmt_elf_compat to match the other binfmts, but not sure if that'd break things or go against some other naming scheme in use)
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v4 1/3] fs/binfmts: Add a name field to the binfmt struct
2013-07-25 15:40 [PATCH 1/3] fs/binfmts: Add a name field to the binfmt struct Zach Levis
` (10 preceding siblings ...)
2013-08-14 16:31 ` [PATCH v4 0/3] fs/binfmts: Improve handling of loops Zach Levis
@ 2013-08-14 16:31 ` Zach Levis
2013-08-14 16:31 ` [PATCH v4 2/3] fs/binfmts: Better handling of binfmt loops Zach Levis
` (5 subsequent siblings)
17 siblings, 0 replies; 35+ messages in thread
From: Zach Levis @ 2013-08-14 16:31 UTC (permalink / raw)
To: akpm, oleg
Cc: viro, linux-kernel, linux-fsdevel, dan.carpenter, keescook, cody,
zml, Zach Levis
Adding the name field helps when printing error messages referring to
specific binfmts
Signed-off-by: Zach Levis <zach@zachsthings.com>
---
fs/binfmt_aout.c | 1 +
fs/binfmt_elf.c | 5 +++++
fs/binfmt_elf_fdpic.c | 1 +
fs/binfmt_em86.c | 1 +
fs/binfmt_flat.c | 1 +
fs/binfmt_misc.c | 1 +
fs/binfmt_script.c | 1 +
fs/binfmt_som.c | 1 +
fs/compat_binfmt_elf.c | 5 +++++
include/linux/binfmts.h | 1 +
10 files changed, 18 insertions(+), 0 deletions(-)
diff --git a/fs/binfmt_aout.c b/fs/binfmt_aout.c
index bce8769..f6bed04 100644
--- a/fs/binfmt_aout.c
+++ b/fs/binfmt_aout.c
@@ -115,6 +115,7 @@ end_coredump:
#endif
static struct linux_binfmt aout_format = {
+ .name = "a.out",
.module = THIS_MODULE,
.load_binary = load_aout_binary,
.load_shlib = load_aout_library,
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index f8a0b0e..864208b 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -74,7 +74,12 @@ static int elf_core_dump(struct coredump_params *cprm);
#define ELF_PAGEOFFSET(_v) ((_v) & (ELF_MIN_ALIGN-1))
#define ELF_PAGEALIGN(_v) (((_v) + ELF_MIN_ALIGN - 1) & ~(ELF_MIN_ALIGN - 1))
+#ifndef ELF_FORMAT_NAME
+#define ELF_FORMAT_NAME "ELF"
+#endif
+
static struct linux_binfmt elf_format = {
+ .name = ELF_FORMAT_NAME,
.module = THIS_MODULE,
.load_binary = load_elf_binary,
.load_shlib = load_elf_library,
diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c
index c166f32..6be4448 100644
--- a/fs/binfmt_elf_fdpic.c
+++ b/fs/binfmt_elf_fdpic.c
@@ -81,6 +81,7 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm);
#endif
static struct linux_binfmt elf_fdpic_format = {
+ .name = "FDPIC ELF",
.module = THIS_MODULE,
.load_binary = load_elf_fdpic_binary,
#ifdef CONFIG_ELF_CORE
diff --git a/fs/binfmt_em86.c b/fs/binfmt_em86.c
index 037a3e2..8404cdd 100644
--- a/fs/binfmt_em86.c
+++ b/fs/binfmt_em86.c
@@ -93,6 +93,7 @@ static int load_em86(struct linux_binprm *bprm)
}
static struct linux_binfmt em86_format = {
+ .name = "em86",
.module = THIS_MODULE,
.load_binary = load_em86,
};
diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
index d50bbe5..a7641e1 100644
--- a/fs/binfmt_flat.c
+++ b/fs/binfmt_flat.c
@@ -92,6 +92,7 @@ static int load_flat_binary(struct linux_binprm *);
static int flat_core_dump(struct coredump_params *cprm);
static struct linux_binfmt flat_format = {
+ .name = "flat",
.module = THIS_MODULE,
.load_binary = load_flat_binary,
.core_dump = flat_core_dump,
diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c
index 1c740e1..d74afc0 100644
--- a/fs/binfmt_misc.c
+++ b/fs/binfmt_misc.c
@@ -694,6 +694,7 @@ static struct dentry *bm_mount(struct file_system_type *fs_type,
}
static struct linux_binfmt misc_format = {
+ .name = "misc",
.module = THIS_MODULE,
.load_binary = load_misc_binary,
};
diff --git a/fs/binfmt_script.c b/fs/binfmt_script.c
index 5027a3e..53f3c17 100644
--- a/fs/binfmt_script.c
+++ b/fs/binfmt_script.c
@@ -99,6 +99,7 @@ static int load_script(struct linux_binprm *bprm)
}
static struct linux_binfmt script_format = {
+ .name = "script",
.module = THIS_MODULE,
.load_binary = load_script,
};
diff --git a/fs/binfmt_som.c b/fs/binfmt_som.c
index 4e00ed6..1c3ccc5 100644
--- a/fs/binfmt_som.c
+++ b/fs/binfmt_som.c
@@ -53,6 +53,7 @@ static int som_core_dump(struct coredump_params *cprm);
#define SOM_PAGEALIGN(_v) (((_v) + SOM_PAGESIZE - 1) & ~(SOM_PAGESIZE - 1))
static struct linux_binfmt som_format = {
+ .name = "SOM",
.module = THIS_MODULE,
.load_binary = load_som_binary,
.load_shlib = load_som_library,
diff --git a/fs/compat_binfmt_elf.c b/fs/compat_binfmt_elf.c
index a81147e..96330f3 100644
--- a/fs/compat_binfmt_elf.c
+++ b/fs/compat_binfmt_elf.c
@@ -131,9 +131,14 @@ static void cputime_to_compat_timeval(const cputime_t cputime,
* might make some debugging less confusing not to duplicate them.
*/
#define elf_format compat_elf_format
+#define elf_format_name compat_elf_format_name
#define init_elf_binfmt init_compat_elf_binfmt
#define exit_elf_binfmt exit_compat_elf_binfmt
+#ifndef ELF_FORMAT_NAME
+#define ELF_FORMAT_NAME "ELF compat"
+#endif
+
/*
* We share all the actual code with the native (64-bit) version.
*/
diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
index 70cf138..402a74a 100644
--- a/include/linux/binfmts.h
+++ b/include/linux/binfmts.h
@@ -70,6 +70,7 @@ struct coredump_params {
struct linux_binfmt {
struct list_head lh;
struct module *module;
+ const char *name;
int (*load_binary)(struct linux_binprm *);
int (*load_shlib)(struct file *);
int (*core_dump)(struct coredump_params *cprm);
--
1.7.1
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v4 2/3] fs/binfmts: Better handling of binfmt loops
2013-07-25 15:40 [PATCH 1/3] fs/binfmts: Add a name field to the binfmt struct Zach Levis
` (11 preceding siblings ...)
2013-08-14 16:31 ` [PATCH v4 1/3] fs/binfmts: Add a name field to the binfmt struct Zach Levis
@ 2013-08-14 16:31 ` Zach Levis
2013-08-14 17:50 ` Oleg Nesterov
2013-08-14 18:16 ` Oleg Nesterov
2013-08-14 16:31 ` [PATCH v4 3/3] fs/binfmts: Whitespace fixes with scripts/cleanfile Zach Levis
` (4 subsequent siblings)
17 siblings, 2 replies; 35+ messages in thread
From: Zach Levis @ 2013-08-14 16:31 UTC (permalink / raw)
To: akpm, oleg
Cc: viro, linux-kernel, linux-fsdevel, dan.carpenter, keescook, cody,
zml, Zach Levis
With these changes, when a binfmt loop is encountered,
the ELOOP will propogate back to the 0 depth. At this point the
argv and argc values will be reset to what they were originally and an
attempt is made to continue with the following binfmt handlers.
Example: a qemu is configured to run 64-bit ELFs on an otherwise 32-bit
system. The system's owner switches to running with 64-bit executables,
but forgets to disable the binfmt_misc option that redirects 64bit ELFs
to qemu. Since the qemu executable is a 64-bit ELF now, binfmt_misc
keeps on matching it with the qemu rule, preventing the execution of any
64-bit binary.
With these changes, an error is printed and search_binary_handler()
continues on to the next handler, allowing the original executable to
run normally so the user can (hopefully) fix their misconfiguration more
easily.
Caveats:
- binfmt_misc is skipped as a whole. To allow for a more generic
solution that works for any binfmt without a lot of duplicated code
and added complexity, the error detection code is applied on the
binfmt level.
- This is a fallback solution. It attempts to restore the state to allow
executing most binaries without side effects, but it may not work for
everything and should not be depended on for regular usage.
My (rough, but functional) test scripts for this issue are available at:
https://gist.github.com/zml2008/6075418
Signed-off-by: Zach Levis <zach@zachsthings.com>
---
fs/exec.c | 192 ++++++++++++++++++++++++++++++++++-------------
include/linux/binfmts.h | 5 +
2 files changed, 144 insertions(+), 53 deletions(-)
diff --git a/fs/exec.c b/fs/exec.c
index eb229e1..474222a 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1365,6 +1365,107 @@ out:
}
EXPORT_SYMBOL(remove_arg_zero);
+static void bprm_close_file(struct linux_binprm *bprm)
+{
+ if (bprm->mm) {
+ acct_arg_size(bprm, 0);
+ mmput(bprm->mm);
+ }
+
+ if (bprm->file) {
+ allow_write_access(bprm->file);
+ fput(bprm->file);
+ }
+}
+
+static int init_bprm(struct linux_binprm *bprm,
+ const char *filename,
+ struct user_arg_ptr *argv,
+ struct user_arg_ptr *envp)
+{
+ int retval;
+ struct file *file;
+
+ file = open_exec(filename);
+ retval = PTR_ERR(file);
+ if (IS_ERR(file))
+ return retval;
+
+ if (!bprm->file)
+ sched_exec();
+
+ bprm->file = file;
+ bprm->filename = filename;
+ bprm->interp = filename;
+
+ retval = bprm_mm_init(bprm);
+ if (retval)
+ goto err;
+
+ retval = bprm->argc = count(*argv, MAX_ARG_STRINGS);
+ if (retval < 0)
+ goto err;
+
+ retval = bprm->envc = count(*envp, MAX_ARG_STRINGS);
+ if (retval < 0)
+ goto err;
+
+ retval = prepare_binprm(bprm);
+ if (retval < 0)
+ goto err;
+
+ retval = copy_strings_kernel(1, &bprm->filename, bprm);
+ if (retval < 0)
+ goto err;
+
+ bprm->argv_orig = argv;
+ bprm->envp_orig = envp;
+ bprm->exec = bprm->p;
+ retval = copy_strings(bprm->envc, *envp, bprm);
+ if (retval < 0)
+ goto err;
+
+ retval = copy_strings(bprm->argc, *argv, bprm);
+ if (retval < 0)
+ goto err;
+
+out:
+ return retval;
+err:
+ bprm_close_file(bprm);
+ goto out;
+}
+
+
+static bool update_prev_binfmts(struct linux_binprm *bprm,
+ struct linux_binfmt *cur_fmt)
+{
+
+ if (!try_module_get(cur_fmt->module))
+ return false;
+ if (bprm->previous_binfmts[1])
+ put_binfmt(bprm->previous_binfmts[1]);
+ bprm->previous_binfmts[1] = bprm->previous_binfmts[0];
+ bprm->previous_binfmts[0] = cur_fmt;
+ return true;
+}
+
+static void put_binfmts(struct linux_binprm *bprm, struct linux_binfmt *cur_fmt)
+{
+ if (bprm->previous_binfmts[1])
+ put_binfmt(bprm->previous_binfmts[1]);
+ if (bprm->previous_binfmts[0])
+ put_binfmt(bprm->previous_binfmts[0]);
+ if (cur_fmt)
+ put_binfmt(cur_fmt);
+}
+
+/* Get name null-safely */
+static inline const char *binfmt_name(struct linux_binfmt *fmt)
+{
+ return fmt ? fmt->name : "NULL";
+}
+
#define printable(c) (((c)=='\t') || ((c)=='\n') || (0x20<=(c) && (c)<=0x7e))
/*
* cycle the list of binary formats handler, until one recognizes the image
@@ -1393,18 +1494,44 @@ int search_binary_handler(struct linux_binprm *bprm)
list_for_each_entry(fmt, &formats, lh) {
if (!try_module_get(fmt->module))
continue;
+
+ if (!update_prev_binfmts(bprm, fmt))
+ continue;
+
read_unlock(&binfmt_lock);
+
bprm->recursion_depth++;
retval = fmt->load_binary(bprm);
bprm->recursion_depth--;
- if (retval >= 0 || retval != -ENOEXEC ||
- bprm->mm == NULL || bprm->file == NULL) {
- put_binfmt(fmt);
+ if (retval == -ELOOP
+ && bprm->recursion_depth == 0) { /* cur, previous */
+ pr_err("Too much recursion with binfmts (0:%s, -1:%s) in file %s, skipping (base %s).\n",
+ binfmt_name(bprm->previous_binfmts[0]),
+ binfmt_name(bprm->previous_binfmts[1]),
+ bprm->filename,
+ fmt->name);
+
+ free_arg_pages(bprm);
+ if (bprm->interp != bprm->filename)
+ kfree(bprm->interp);
+ bprm_close_file(bprm);
+ retval = init_bprm(bprm, bprm->filename,
+ bprm->argv_orig, bprm->envp_orig);
+ if (retval < 0) {
+ put_binfmts(bprm, fmt);
+ return retval;
+ }
+
+ } else if (retval >= 0 || retval != -ENOEXEC ||
+ bprm->mm == NULL || bprm->file == NULL) {
+ put_binfmts(bprm, fmt);
return retval;
}
+
read_lock(&binfmt_lock);
put_binfmt(fmt);
}
+ put_binfmts(bprm, NULL);
read_unlock(&binfmt_lock);
if (need_retry && retval == -ENOEXEC) {
@@ -1436,6 +1563,11 @@ static int exec_binprm(struct linux_binprm *bprm)
if (ret >= 0) {
trace_sched_process_exec(current, old_pid, bprm);
ptrace_event(PTRACE_EVENT_EXEC, old_vpid);
+ /* Successful execution, now null out the cached argv
+ * (we don't want to access it later)
+ * */
+ bprm->argv_orig = NULL;
+ bprm->envp_orig = NULL;
current->did_exec = 1;
proc_exec_connector(current);
@@ -1448,6 +1580,7 @@ static int exec_binprm(struct linux_binprm *bprm)
return ret;
}
+
/*
* sys_execve() executes a new program.
*/
@@ -1456,7 +1589,6 @@ static int do_execve_common(const char *filename,
struct user_arg_ptr envp)
{
struct linux_binprm *bprm;
- struct file *file;
struct files_struct *displaced;
bool clear_in_exec;
int retval;
@@ -1497,45 +1629,9 @@ static int do_execve_common(const char *filename,
clear_in_exec = retval;
current->in_execve = 1;
- file = open_exec(filename);
- retval = PTR_ERR(file);
- if (IS_ERR(file))
- goto out_unmark;
-
- sched_exec();
-
- bprm->file = file;
- bprm->filename = filename;
- bprm->interp = filename;
-
- retval = bprm_mm_init(bprm);
- if (retval)
- goto out_file;
-
- bprm->argc = count(argv, MAX_ARG_STRINGS);
- if ((retval = bprm->argc) < 0)
- goto out;
-
- bprm->envc = count(envp, MAX_ARG_STRINGS);
- if ((retval = bprm->envc) < 0)
- goto out;
-
- retval = prepare_binprm(bprm);
+ retval = init_bprm(bprm, filename, &argv, &envp);
if (retval < 0)
- goto out;
-
- retval = copy_strings_kernel(1, &bprm->filename, bprm);
- if (retval < 0)
- goto out;
-
- bprm->exec = bprm->p;
- retval = copy_strings(bprm->envc, envp, bprm);
- if (retval < 0)
- goto out;
-
- retval = copy_strings(bprm->argc, argv, bprm);
- if (retval < 0)
- goto out;
+ goto out_unmark;
retval = exec_binprm(bprm);
if (retval < 0)
@@ -1551,17 +1647,7 @@ static int do_execve_common(const char *filename,
return retval;
out:
- if (bprm->mm) {
- acct_arg_size(bprm, 0);
- mmput(bprm->mm);
- }
-
-out_file:
- if (bprm->file) {
- allow_write_access(bprm->file);
- fput(bprm->file);
- }
-
+ bprm_close_file(bprm);
out_unmark:
if (clear_in_exec)
current->fs->in_exec = 0;
diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
index 402a74a..3451db8 100644
--- a/include/linux/binfmts.h
+++ b/include/linux/binfmts.h
@@ -8,6 +8,8 @@
#define CORENAME_MAX_SIZE 128
+struct user_arg_ptr; /* Struct from fs/exec.c, for linux_binprm->argv_orig */
+
/*
* This structure is used to hold the arguments that are used when loading binaries.
*/
@@ -32,11 +34,14 @@ struct linux_binprm {
unsigned int taso:1;
#endif
unsigned int recursion_depth;
+ /* current binfmt at 0 and previous binfmt at 1 */
+ struct linux_binfmt *previous_binfmts[2];
struct file * file;
struct cred *cred; /* new credentials */
int unsafe; /* how unsafe this exec is (mask of LSM_UNSAFE_*) */
unsigned int per_clear; /* bits to clear in current->personality */
int argc, envc;
+ struct user_arg_ptr *argv_orig, *envp_orig;
const char * filename; /* Name of binary as seen by procps */
const char * interp; /* Name of the binary really executed. Most
of the time same as filename, but could be
--
1.7.1
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v4 3/3] fs/binfmts: Whitespace fixes with scripts/cleanfile
2013-07-25 15:40 [PATCH 1/3] fs/binfmts: Add a name field to the binfmt struct Zach Levis
` (12 preceding siblings ...)
2013-08-14 16:31 ` [PATCH v4 2/3] fs/binfmts: Better handling of binfmt loops Zach Levis
@ 2013-08-14 16:31 ` Zach Levis
2013-08-15 16:20 ` [PATCH v5 0/3] fs/binfmts: Improve handling of loops Zach Levis
` (3 subsequent siblings)
17 siblings, 0 replies; 35+ messages in thread
From: Zach Levis @ 2013-08-14 16:31 UTC (permalink / raw)
To: akpm, oleg
Cc: viro, linux-kernel, linux-fsdevel, dan.carpenter, keescook, cody,
zml, Zach Levis
Obligatory first-patchset whitespace commit
Signed-off-by: Zach Levis <zach@zachsthings.com>
---
fs/binfmt_aout.c | 8 ++++----
fs/binfmt_elf.c | 38 +++++++++++++++++++-------------------
fs/binfmt_elf_fdpic.c | 8 ++++----
fs/binfmt_em86.c | 9 ++++++---
fs/binfmt_flat.c | 26 +++++++++++++-------------
fs/binfmt_misc.c | 24 ++++++++++++------------
fs/binfmt_script.c | 11 +++++++----
fs/binfmt_som.c | 2 +-
fs/exec.c | 10 +++++-----
9 files changed, 71 insertions(+), 65 deletions(-)
diff --git a/fs/binfmt_aout.c b/fs/binfmt_aout.c
index f6bed04..c0a458b 100644
--- a/fs/binfmt_aout.c
+++ b/fs/binfmt_aout.c
@@ -62,7 +62,7 @@ static int aout_core_dump(struct coredump_params *cprm)
fs = get_fs();
set_fs(KERNEL_DS);
has_dumped = 1;
- strncpy(dump.u_comm, current->comm, sizeof(dump.u_comm));
+ strncpy(dump.u_comm, current->comm, sizeof(dump.u_comm));
dump.u_ar0 = offsetof(struct user, regs);
dump.signal = cprm->siginfo->si_signo;
aout_dump_thread(cprm->regs, &dump);
@@ -302,7 +302,7 @@ static int load_aout_binary(struct linux_binprm * bprm)
if ((fd_offset & ~PAGE_MASK) != 0 && printk_ratelimit())
{
- printk(KERN_WARNING
+ printk(KERN_WARNING
"fd_offset is not page aligned. Please convert program: %s\n",
bprm->file->f_path.dentry->d_name.name);
}
@@ -391,12 +391,12 @@ static int load_aout_library(struct file *file)
if ((N_TXTOFF(ex) & ~PAGE_MASK) != 0) {
if (printk_ratelimit())
{
- printk(KERN_WARNING
+ printk(KERN_WARNING
"N_TXTOFF is not page aligned. Please convert library: %s\n",
file->f_path.dentry->d_name.name);
}
vm_brk(start_addr, ex.a_text + ex.a_data + ex.a_bss);
-
+
read_code(file, start_addr, N_TXTOFF(ex),
ex.a_text + ex.a_data);
retval = 0;
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 864208b..44dfe4e 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -222,7 +222,7 @@ create_elf_tables(struct linux_binprm *bprm, struct elfhdr *exec,
} while (0)
#ifdef ARCH_DLINFO
- /*
+ /*
* ARCH_DLINFO must come first so PPC can do its special alignment of
* AUXV.
* update AT_VECTOR_SIZE_ARCH if the number of NEW_AUX_ENT() in
@@ -243,7 +243,7 @@ create_elf_tables(struct linux_binprm *bprm, struct elfhdr *exec,
NEW_AUX_ENT(AT_EUID, from_kuid_munged(cred->user_ns, cred->euid));
NEW_AUX_ENT(AT_GID, from_kgid_munged(cred->user_ns, cred->gid));
NEW_AUX_ENT(AT_EGID, from_kgid_munged(cred->user_ns, cred->egid));
- NEW_AUX_ENT(AT_SECURE, security_bprm_secureexec(bprm));
+ NEW_AUX_ENT(AT_SECURE, security_bprm_secureexec(bprm));
NEW_AUX_ENT(AT_RANDOM, (elf_addr_t)(unsigned long)u_rand_bytes);
#ifdef ELF_HWCAP2
NEW_AUX_ENT(AT_HWCAP2, ELF_HWCAP2);
@@ -437,7 +437,7 @@ static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex,
error = -EIO;
if (retval != size) {
if (retval < 0)
- error = retval;
+ error = retval;
goto out_close;
}
@@ -456,7 +456,7 @@ static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex,
unsigned long k, map_addr;
if (eppnt->p_flags & PF_R)
- elf_prot = PROT_READ;
+ elf_prot = PROT_READ;
if (eppnt->p_flags & PF_W)
elf_prot |= PROT_WRITE;
if (eppnt->p_flags & PF_X)
@@ -574,7 +574,7 @@ static unsigned long randomize_stack_top(unsigned long stack_top)
static int load_elf_binary(struct linux_binprm *bprm)
{
struct file *interpreter = NULL; /* to shut gcc up */
- unsigned long load_addr = 0, load_bias = 0;
+ unsigned long load_addr = 0, load_bias = 0;
int load_addr_set = 0;
char * elf_interpreter = NULL;
unsigned long error;
@@ -599,7 +599,7 @@ static int load_elf_binary(struct linux_binprm *bprm)
retval = -ENOMEM;
goto out_ret;
}
-
+
/* Get the exec-header */
loc->elf_ex = *((struct elfhdr *)bprm->buf);
@@ -619,7 +619,7 @@ static int load_elf_binary(struct linux_binprm *bprm)
if (loc->elf_ex.e_phentsize != sizeof(struct elf_phdr))
goto out;
if (loc->elf_ex.e_phnum < 1 ||
- loc->elf_ex.e_phnum > 65536U / sizeof(struct elf_phdr))
+ loc->elf_ex.e_phnum > 65536U / sizeof(struct elf_phdr))
goto out;
size = loc->elf_ex.e_phnum * sizeof(struct elf_phdr);
retval = -ENOMEM;
@@ -651,7 +651,7 @@ static int load_elf_binary(struct linux_binprm *bprm)
* is an a.out format binary
*/
retval = -ENOEXEC;
- if (elf_ppnt->p_filesz > PATH_MAX ||
+ if (elf_ppnt->p_filesz > PATH_MAX ||
elf_ppnt->p_filesz < 2)
goto out_free_ph;
@@ -751,7 +751,7 @@ static int load_elf_binary(struct linux_binprm *bprm)
send_sig(SIGKILL, current, 0);
goto out_free_dentry;
}
-
+
current->mm->start_stack = bprm->p;
/* Now we do a little grungy work by mmapping the ELF image into
@@ -766,7 +766,7 @@ static int load_elf_binary(struct linux_binprm *bprm)
if (unlikely (elf_brk > elf_bss)) {
unsigned long nbyte;
-
+
/* There was a PT_LOAD segment with p_memsz > p_filesz
before this one. Map anonymous pages, if needed,
and clear the area. */
@@ -1298,7 +1298,7 @@ static void fill_elf_note_phdr(struct elf_phdr *phdr, int sz, loff_t offset)
return;
}
-static void fill_note(struct memelfnote *note, const char *name, int type,
+static void fill_note(struct memelfnote *note, const char *name, int type,
unsigned int sz, void *data)
{
note->name = name;
@@ -1350,7 +1350,7 @@ static int fill_psinfo(struct elf_prpsinfo *psinfo, struct task_struct *p,
{
const struct cred *cred;
unsigned int i, len;
-
+
/* first copy the parameters from user space */
memset(psinfo, 0, sizeof(struct elf_prpsinfo));
@@ -1384,7 +1384,7 @@ static int fill_psinfo(struct elf_prpsinfo *psinfo, struct task_struct *p,
SET_GID(psinfo->pr_gid, from_kgid_munged(cred->user_ns, cred->gid));
rcu_read_unlock();
strncpy(psinfo->pr_fname, p->comm, sizeof(psinfo->pr_fname));
-
+
return 0;
}
@@ -1785,8 +1785,8 @@ static int elf_dump_thread_status(long signr, struct elf_thread_status *t)
t->num_notes = 0;
fill_prstatus(&t->prstatus, p, signr);
- elf_core_copy_task_regs(p, &t->prstatus.pr_reg);
-
+ elf_core_copy_task_regs(p, &t->prstatus.pr_reg);
+
fill_note(&t->notes[0], "CORE", NT_PRSTATUS, sizeof(t->prstatus),
&(t->prstatus));
t->num_notes++;
@@ -1807,7 +1807,7 @@ static int elf_dump_thread_status(long signr, struct elf_thread_status *t)
t->num_notes++;
sz += notesize(&t->notes[2]);
}
-#endif
+#endif
return sz;
}
@@ -2059,7 +2059,7 @@ static int elf_core_dump(struct coredump_params *cprm)
/*
* We no longer stop all VM operations.
- *
+ *
* This is because those proceses that could possibly change map_count
* or the mmap / vma pages are now blocked in do_exit on current
* finishing this core dump.
@@ -2068,7 +2068,7 @@ static int elf_core_dump(struct coredump_params *cprm)
* the map_count or the pages allocated. So no possibility of crashing
* exists while dumping the mm->vm_next areas to the core file.
*/
-
+
/* alloc memory for large data structures: too large to be on stack */
elf = kmalloc(sizeof(*elf), GFP_KERNEL);
if (!elf)
@@ -2174,7 +2174,7 @@ static int elf_core_dump(struct coredump_params *cprm)
if (!elf_core_write_extra_phdrs(cprm->file, offset, &size, cprm->limit))
goto end_coredump;
- /* write out the notes section */
+ /* write out the notes section */
if (!write_note_info(&info, cprm->file, &foffset))
goto end_coredump;
diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c
index 6be4448..799791b 100644
--- a/fs/binfmt_elf_fdpic.c
+++ b/fs/binfmt_elf_fdpic.c
@@ -1595,8 +1595,8 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm)
struct memelfnote *notes = NULL;
struct elf_prstatus *prstatus = NULL; /* NT_PRSTATUS */
struct elf_prpsinfo *psinfo = NULL; /* NT_PRPSINFO */
- LIST_HEAD(thread_list);
- struct list_head *t;
+ LIST_HEAD(thread_list);
+ struct list_head *t;
elf_fpregset_t *fpu = NULL;
#ifdef ELF_CORE_COPY_XFPREGS
elf_fpxregset_t *xfpu = NULL;
@@ -1705,7 +1705,7 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm)
fill_note(¬es[numnote++], "CORE", NT_AUXV,
i * sizeof(elf_addr_t), auxv);
- /* Try to dump the FPU. */
+ /* Try to dump the FPU. */
if ((prstatus->pr_fpvalid =
elf_core_copy_task_fpregs(current, cprm->regs, fpu)))
fill_note(notes + numnote++,
@@ -1795,7 +1795,7 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm)
if (!elf_core_write_extra_phdrs(cprm->file, offset, &size, cprm->limit))
goto end_coredump;
- /* write out the notes section */
+ /* write out the notes section */
for (i = 0; i < numnote; i++)
if (!writenote(notes + i, cprm->file, &foffset))
goto end_coredump;
diff --git a/fs/binfmt_em86.c b/fs/binfmt_em86.c
index 8404cdd..7398f7e 100644
--- a/fs/binfmt_em86.c
+++ b/fs/binfmt_em86.c
@@ -63,15 +63,18 @@ static int load_em86(struct linux_binprm *bprm)
*/
remove_arg_zero(bprm);
retval = copy_strings_kernel(1, &bprm->filename, bprm);
- if (retval < 0) return retval;
+ if (retval < 0)
+ return retval;
bprm->argc++;
if (i_arg) {
retval = copy_strings_kernel(1, &i_arg, bprm);
- if (retval < 0) return retval;
+ if (retval < 0)
+ return retval;
bprm->argc++;
}
retval = copy_strings_kernel(1, &i_name, bprm);
- if (retval < 0) return retval;
+ if (retval < 0)
+ return retval;
bprm->argc++;
/*
diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
index a7641e1..13e7ad0 100644
--- a/fs/binfmt_flat.c
+++ b/fs/binfmt_flat.c
@@ -388,7 +388,7 @@ void old_reloc(unsigned long rl)
#endif
flat_v2_reloc_t r;
unsigned long *ptr;
-
+
r.value = rl;
#if defined(CONFIG_COLDFIRE)
ptr = (unsigned long *) (current->mm->start_code + r.reloc.offset);
@@ -401,7 +401,7 @@ void old_reloc(unsigned long rl)
"(address %p, currently %x) into segment %s\n",
r.reloc.offset, ptr, (int)*ptr, segment[r.reloc.type]);
#endif
-
+
switch (r.reloc.type) {
case OLD_FLAT_RELOC_TYPE_TEXT:
*ptr += current->mm->start_code;
@@ -420,7 +420,7 @@ void old_reloc(unsigned long rl)
#ifdef DEBUG
printk("Relocation became %x\n", (int)*ptr);
#endif
-}
+}
/****************************************************************************/
@@ -479,7 +479,7 @@ static int load_flat_file(struct linux_binprm * bprm,
ret = -ENOEXEC;
goto err;
}
-
+
/* Don't allow old format executables to use shared libraries */
if (rev == OLD_FLAT_VERSION && id != 0) {
printk("BINFMT_FLAT: shared libraries are not available before rev 0x%x\n",
@@ -581,7 +581,7 @@ static int load_flat_file(struct linux_binprm * bprm,
fpos = ntohl(hdr->data_start);
#ifdef CONFIG_BINFMT_ZFLAT
if (flags & FLAT_FLAG_GZDATA) {
- result = decompress_exec(bprm, fpos, (char *) datapos,
+ result = decompress_exec(bprm, fpos, (char *) datapos,
full_data, 0);
} else
#endif
@@ -704,7 +704,7 @@ static int load_flat_file(struct linux_binprm * bprm,
libinfo->lib_list[id].loaded = 1;
libinfo->lib_list[id].entry = (0x00ffffff & ntohl(hdr->entry)) + textpos;
libinfo->lib_list[id].build_date = ntohl(hdr->build_date);
-
+
/*
* We just load the allocations into some temporary memory to
* help simplify all this mumbo jumbo
@@ -784,11 +784,11 @@ static int load_flat_file(struct linux_binprm * bprm,
for (i=0; i < relocs; i++)
old_reloc(ntohl(reloc[i]));
}
-
+
flush_icache_range(start_code, end_code);
/* zero the BSS, BRK and stack areas */
- memset((void*)(datapos + data_len), 0, bss_len +
+ memset((void *)(datapos + data_len), 0, bss_len +
(memp + memp_size - stack_len - /* end brk */
libinfo->lib_list[id].start_brk) + /* start brk */
stack_len);
@@ -882,11 +882,11 @@ static int load_flat_binary(struct linux_binprm * bprm)
stack_len += (bprm->argc + 1) * sizeof(char *); /* the argv array */
stack_len += (bprm->envc + 1) * sizeof(char *); /* the envp array */
stack_len += FLAT_STACK_ALIGN - 1; /* reserve for upcoming alignment */
-
+
res = load_flat_file(bprm, &libinfo, 0, &stack_len);
if (IS_ERR_VALUE(res))
return res;
-
+
/* Update data segment pointers for all libraries */
for (i=0; i<MAX_SHARED_LIBS; i++)
if (libinfo.lib_list[i].loaded)
@@ -908,7 +908,7 @@ static int load_flat_binary(struct linux_binprm * bprm)
((char *) page_address(bprm->page[i/PAGE_SIZE]))[i % PAGE_SIZE];
sp = (unsigned long *) create_flat_tables(p, bprm);
-
+
/* Fake some return addresses to ensure the call chain will
* initialise library in order for us. We are required to call
* lib 1 first, then 2, ... and finally the main program (id 0).
@@ -924,7 +924,7 @@ static int load_flat_binary(struct linux_binprm * bprm)
}
}
#endif
-
+
/* Stash our initial stack pointer into the mm structure */
current->mm->start_stack = (unsigned long )sp;
@@ -933,7 +933,7 @@ static int load_flat_binary(struct linux_binprm * bprm)
#endif
DBG_FLT("start_thread(regs=0x%x, entry=0x%x, start_stack=0x%x)\n",
(int)regs, (int)start_addr, (int)current->mm->start_stack);
-
+
start_thread(regs, start_addr, current->mm->start_stack);
return 0;
diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c
index d74afc0..8b9d701 100644
--- a/fs/binfmt_misc.c
+++ b/fs/binfmt_misc.c
@@ -62,7 +62,7 @@ static struct file_system_type bm_fs_type;
static struct vfsmount *bm_mnt;
static int entry_count;
-/*
+/*
* Check if we support the binfmt
* if we do, return the node, else NULL
* locking is done in load_misc_binary
@@ -138,12 +138,12 @@ static int load_misc_binary(struct linux_binprm *bprm)
/* if the binary should be opened on behalf of the
* interpreter than keep it open and assign descriptor
* to it */
- fd_binary = get_unused_fd();
- if (fd_binary < 0) {
- retval = fd_binary;
- goto _ret;
- }
- fd_install(fd_binary, bprm->file);
+ fd_binary = get_unused_fd();
+ if (fd_binary < 0) {
+ retval = fd_binary;
+ goto _ret;
+ }
+ fd_install(fd_binary, bprm->file);
/* if the binary is not readable than enforce mm->dumpable=0
regardless of the interpreter's permissions */
@@ -156,11 +156,11 @@ static int load_misc_binary(struct linux_binprm *bprm)
bprm->interp_flags |= BINPRM_FLAGS_EXECFD;
bprm->interp_data = fd_binary;
- } else {
- allow_write_access(bprm->file);
- fput(bprm->file);
- bprm->file = NULL;
- }
+ } else {
+ allow_write_access(bprm->file);
+ fput(bprm->file);
+ bprm->file = NULL;
+ }
/* make argv[1] be the path to the binary */
retval = copy_strings_kernel (1, &bprm->interp, bprm);
if (retval < 0)
diff --git a/fs/binfmt_script.c b/fs/binfmt_script.c
index 53f3c17..901da7e 100644
--- a/fs/binfmt_script.c
+++ b/fs/binfmt_script.c
@@ -45,7 +45,7 @@ static int load_script(struct linux_binprm *bprm)
break;
}
for (cp = bprm->buf+2; (*cp == ' ') || (*cp == '\t'); cp++);
- if (*cp == '\0')
+ if (*cp == '\0')
return -ENOEXEC; /* No interpreter name found */
i_name = cp;
i_arg = NULL;
@@ -70,15 +70,18 @@ static int load_script(struct linux_binprm *bprm)
if (retval)
return retval;
retval = copy_strings_kernel(1, &bprm->interp, bprm);
- if (retval < 0) return retval;
+ if (retval < 0)
+ return retval;
bprm->argc++;
if (i_arg) {
retval = copy_strings_kernel(1, &i_arg, bprm);
- if (retval < 0) return retval;
+ if (retval < 0)
+ return retval;
bprm->argc++;
}
retval = copy_strings_kernel(1, &i_name, bprm);
- if (retval) return retval;
+ if (retval)
+ return retval;
bprm->argc++;
retval = bprm_change_interp(interp, bprm);
if (retval < 0)
diff --git a/fs/binfmt_som.c b/fs/binfmt_som.c
index 1c3ccc5..88af0d1 100644
--- a/fs/binfmt_som.c
+++ b/fs/binfmt_som.c
@@ -2,7 +2,7 @@
* linux/fs/binfmt_som.c
*
* These are the functions used to load SOM format executables as used
- * by HP-UX.
+ * by HP-UX.
*
* Copyright 1999 Matthew Wilcox <willy@bofh.ai>
* based on binfmt_elf which is
diff --git a/fs/exec.c b/fs/exec.c
index 474222a..62b79e6 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -19,7 +19,7 @@
* current->executable is only used by the procfs. This allows a dispatch
* table to check for several different types of binary formats. We keep
* trying until we recognize the file or we run out of supported binary
- * formats.
+ * formats.
*/
#include <linux/slab.h>
@@ -156,7 +156,7 @@ SYSCALL_DEFINE1(uselib, const char __user *, library)
exit:
fput(file);
out:
- return error;
+ return error;
}
#ifdef CONFIG_MMU
@@ -1141,7 +1141,7 @@ void setup_new_exec(struct linux_binprm * bprm)
group */
current->self_exec_id++;
-
+
flush_signal_handlers(current, 0);
do_close_on_exec(current->files);
}
@@ -1267,8 +1267,8 @@ static int check_unsafe_exec(struct linux_binprm *bprm)
return res;
}
-/*
- * Fill the binprm structure from the inode.
+/*
+ * Fill the binprm structure from the inode.
* Check permissions, then read the first 128 (BINPRM_BUF_SIZE) bytes
*
* This may be called multiple times for binary chains (scripts for example).
--
1.7.1
^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: [PATCH v4 2/3] fs/binfmts: Better handling of binfmt loops
2013-08-14 16:31 ` [PATCH v4 2/3] fs/binfmts: Better handling of binfmt loops Zach Levis
@ 2013-08-14 17:50 ` Oleg Nesterov
2013-08-15 16:26 ` Zach L
2013-08-14 18:16 ` Oleg Nesterov
1 sibling, 1 reply; 35+ messages in thread
From: Oleg Nesterov @ 2013-08-14 17:50 UTC (permalink / raw)
To: Zach Levis
Cc: akpm, viro, linux-kernel, linux-fsdevel, dan.carpenter, keescook,
cody, zml
On 08/14, Zach Levis wrote:
>
> Example: a qemu is configured to run 64-bit ELFs on an otherwise 32-bit
> system. The system's owner switches to running with 64-bit executables,
> but forgets to disable the binfmt_misc option that redirects 64bit ELFs
> to qemu. Since the qemu executable is a 64-bit ELF now, binfmt_misc
> keeps on matching it with the qemu rule, preventing the execution of any
> 64-bit binary.
Honestly, I dislike this version even more, sorry. The patch becomes
much more complex, and and it is still not clear to me why do we want
these complications.
> My (rough, but functional) test scripts for this issue are available at:
> https://gist.github.com/zml2008/6075418
Well, suppose that someone tries to read this changelog in 2014 to
understand the code. Are you sure this link will be still alive?
It would be better to have everything in the changelog, if possible.
> +static void put_binfmts(struct linux_binprm *bprm, struct linux_binfmt *cur_fmt)
> +{
> + if (bprm->previous_binfmts[1])
> + put_binfmt(bprm->previous_binfmts[1]);
> + if (bprm->previous_binfmts[0])
> + put_binfmt(bprm->previous_binfmts[0]);
> + if (cur_fmt)
> + put_binfmt(cur_fmt);
> +}
I didn't actually read this patch, but at first glance this doesn't look
right. Just suppose that ->load_binary() succeeds at depth = N, this will
be called N times.
In fact I am not sure update_prev_binfmts() is right too, but probably
I do not understand the logic. Just suppose that each ->load_binary()
simply returns ENOEXEC at some depth. Do we really want to replace
previous_binfmts[1] every time?
> @@ -1393,18 +1494,44 @@ int search_binary_handler(struct linux_binprm *bprm)
> list_for_each_entry(fmt, &formats, lh) {
> if (!try_module_get(fmt->module))
> continue;
> +
> + if (!update_prev_binfmts(bprm, fmt))
> + continue;
> +
> read_unlock(&binfmt_lock);
> +
> bprm->recursion_depth++;
> retval = fmt->load_binary(bprm);
> bprm->recursion_depth--;
> - if (retval >= 0 || retval != -ENOEXEC ||
> - bprm->mm == NULL || bprm->file == NULL) {
> - put_binfmt(fmt);
> + if (retval == -ELOOP
> + && bprm->recursion_depth == 0) { /* cur, previous */
> + pr_err("Too much recursion with binfmts (0:%s, -1:%s) in file %s, skipping (base %s).\n",
> + binfmt_name(bprm->previous_binfmts[0]),
> + binfmt_name(bprm->previous_binfmts[1]),
> + bprm->filename,
> + fmt->name);
> +
> + free_arg_pages(bprm);
> + if (bprm->interp != bprm->filename)
> + kfree(bprm->interp);
> + bprm_close_file(bprm);
> + retval = init_bprm(bprm, bprm->filename,
> + bprm->argv_orig, bprm->envp_orig);
> + if (retval < 0) {
> + put_binfmts(bprm, fmt);
> + return retval;
> + }
I still think that if we want this, it would be better to move this hack
into load_misc_binary(). Only linux_binprm itself can know it is desirable
(or even safe) to recover/restart in general.
And btw, if we want this, then why we only do this if recursion_depth == 0?
Just condider '#!/path-to-the-binary-which-wants-this-patch".
And again, the patch (afaics) translates -ELOOP into -ENOEXEC on failure,
not good.
Oleg.
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v4 2/3] fs/binfmts: Better handling of binfmt loops
2013-08-14 16:31 ` [PATCH v4 2/3] fs/binfmts: Better handling of binfmt loops Zach Levis
2013-08-14 17:50 ` Oleg Nesterov
@ 2013-08-14 18:16 ` Oleg Nesterov
1 sibling, 0 replies; 35+ messages in thread
From: Oleg Nesterov @ 2013-08-14 18:16 UTC (permalink / raw)
To: Zach Levis
Cc: akpm, viro, linux-kernel, linux-fsdevel, dan.carpenter, keescook,
cody, zml
On 08/14, Zach Levis wrote:
>
> +static void bprm_close_file(struct linux_binprm *bprm)
> +{
> + if (bprm->mm) {
> + acct_arg_size(bprm, 0);
> + mmput(bprm->mm);
> + }
> +
> + if (bprm->file) {
> + allow_write_access(bprm->file);
> + fput(bprm->file);
> + }
> +}
btw this doesn't look right too. This can be called multiple times
during the error handling.
Just suppose that search_binary_handler() does this at depth == 0
when it detects -ELOOP and then open_exec() fails. After that
we return to do_execve_common() which goes to "out:".
Oleg.
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v5 0/3] fs/binfmts: Improve handling of loops
2013-07-25 15:40 [PATCH 1/3] fs/binfmts: Add a name field to the binfmt struct Zach Levis
` (13 preceding siblings ...)
2013-08-14 16:31 ` [PATCH v4 3/3] fs/binfmts: Whitespace fixes with scripts/cleanfile Zach Levis
@ 2013-08-15 16:20 ` Zach Levis
2013-08-15 16:20 ` [PATCH v5 1/3] fs/binfmts: Add a name field to the binfmt struct Zach Levis
` (2 subsequent siblings)
17 siblings, 0 replies; 35+ messages in thread
From: Zach Levis @ 2013-08-15 16:20 UTC (permalink / raw)
To: akpm, oleg
Cc: viro, linux-kernel, linux-fsdevel, dan.carpenter, keescook, cody,
zml, Zach Levis
This v5 is based off Oleg's changes from "exec: more cleanups" and "exec: minor cleanups + minor fix"
v5 is just some small fixes from Oleg and putting the test scripts into the commit message
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v5 1/3] fs/binfmts: Add a name field to the binfmt struct
2013-07-25 15:40 [PATCH 1/3] fs/binfmts: Add a name field to the binfmt struct Zach Levis
` (14 preceding siblings ...)
2013-08-15 16:20 ` [PATCH v5 0/3] fs/binfmts: Improve handling of loops Zach Levis
@ 2013-08-15 16:20 ` Zach Levis
2013-08-15 17:06 ` Kees Cook
2013-08-15 16:20 ` [PATCH v5 2/3] fs/binfmts: Better handling of binfmt loops Zach Levis
2013-08-15 16:20 ` [PATCH v5 3/3] fs/binfmts: Whitespace fixes with scripts/cleanfile Zach Levis
17 siblings, 1 reply; 35+ messages in thread
From: Zach Levis @ 2013-08-15 16:20 UTC (permalink / raw)
To: akpm, oleg
Cc: viro, linux-kernel, linux-fsdevel, dan.carpenter, keescook, cody,
zml, Zach Levis
Adding the name field helps when printing error messages referring to
specific binfmts
Signed-off-by: Zach Levis <zach@zachsthings.com>
---
fs/binfmt_aout.c | 1 +
fs/binfmt_elf.c | 5 +++++
fs/binfmt_elf_fdpic.c | 1 +
fs/binfmt_em86.c | 1 +
fs/binfmt_flat.c | 1 +
fs/binfmt_misc.c | 1 +
fs/binfmt_script.c | 1 +
fs/binfmt_som.c | 1 +
fs/compat_binfmt_elf.c | 5 +++++
include/linux/binfmts.h | 1 +
10 files changed, 18 insertions(+), 0 deletions(-)
diff --git a/fs/binfmt_aout.c b/fs/binfmt_aout.c
index bce8769..f6bed04 100644
--- a/fs/binfmt_aout.c
+++ b/fs/binfmt_aout.c
@@ -115,6 +115,7 @@ end_coredump:
#endif
static struct linux_binfmt aout_format = {
+ .name = "a.out",
.module = THIS_MODULE,
.load_binary = load_aout_binary,
.load_shlib = load_aout_library,
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index f8a0b0e..864208b 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -74,7 +74,12 @@ static int elf_core_dump(struct coredump_params *cprm);
#define ELF_PAGEOFFSET(_v) ((_v) & (ELF_MIN_ALIGN-1))
#define ELF_PAGEALIGN(_v) (((_v) + ELF_MIN_ALIGN - 1) & ~(ELF_MIN_ALIGN - 1))
+#ifndef ELF_FORMAT_NAME
+#define ELF_FORMAT_NAME "ELF"
+#endif
+
static struct linux_binfmt elf_format = {
+ .name = ELF_FORMAT_NAME,
.module = THIS_MODULE,
.load_binary = load_elf_binary,
.load_shlib = load_elf_library,
diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c
index c166f32..6be4448 100644
--- a/fs/binfmt_elf_fdpic.c
+++ b/fs/binfmt_elf_fdpic.c
@@ -81,6 +81,7 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm);
#endif
static struct linux_binfmt elf_fdpic_format = {
+ .name = "FDPIC ELF",
.module = THIS_MODULE,
.load_binary = load_elf_fdpic_binary,
#ifdef CONFIG_ELF_CORE
diff --git a/fs/binfmt_em86.c b/fs/binfmt_em86.c
index 037a3e2..8404cdd 100644
--- a/fs/binfmt_em86.c
+++ b/fs/binfmt_em86.c
@@ -93,6 +93,7 @@ static int load_em86(struct linux_binprm *bprm)
}
static struct linux_binfmt em86_format = {
+ .name = "em86",
.module = THIS_MODULE,
.load_binary = load_em86,
};
diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
index d50bbe5..a7641e1 100644
--- a/fs/binfmt_flat.c
+++ b/fs/binfmt_flat.c
@@ -92,6 +92,7 @@ static int load_flat_binary(struct linux_binprm *);
static int flat_core_dump(struct coredump_params *cprm);
static struct linux_binfmt flat_format = {
+ .name = "flat",
.module = THIS_MODULE,
.load_binary = load_flat_binary,
.core_dump = flat_core_dump,
diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c
index 1c740e1..d74afc0 100644
--- a/fs/binfmt_misc.c
+++ b/fs/binfmt_misc.c
@@ -694,6 +694,7 @@ static struct dentry *bm_mount(struct file_system_type *fs_type,
}
static struct linux_binfmt misc_format = {
+ .name = "misc",
.module = THIS_MODULE,
.load_binary = load_misc_binary,
};
diff --git a/fs/binfmt_script.c b/fs/binfmt_script.c
index 5027a3e..53f3c17 100644
--- a/fs/binfmt_script.c
+++ b/fs/binfmt_script.c
@@ -99,6 +99,7 @@ static int load_script(struct linux_binprm *bprm)
}
static struct linux_binfmt script_format = {
+ .name = "script",
.module = THIS_MODULE,
.load_binary = load_script,
};
diff --git a/fs/binfmt_som.c b/fs/binfmt_som.c
index 4e00ed6..1c3ccc5 100644
--- a/fs/binfmt_som.c
+++ b/fs/binfmt_som.c
@@ -53,6 +53,7 @@ static int som_core_dump(struct coredump_params *cprm);
#define SOM_PAGEALIGN(_v) (((_v) + SOM_PAGESIZE - 1) & ~(SOM_PAGESIZE - 1))
static struct linux_binfmt som_format = {
+ .name = "SOM",
.module = THIS_MODULE,
.load_binary = load_som_binary,
.load_shlib = load_som_library,
diff --git a/fs/compat_binfmt_elf.c b/fs/compat_binfmt_elf.c
index a81147e..96330f3 100644
--- a/fs/compat_binfmt_elf.c
+++ b/fs/compat_binfmt_elf.c
@@ -131,9 +131,14 @@ static void cputime_to_compat_timeval(const cputime_t cputime,
* might make some debugging less confusing not to duplicate them.
*/
#define elf_format compat_elf_format
+#define elf_format_name compat_elf_format_name
#define init_elf_binfmt init_compat_elf_binfmt
#define exit_elf_binfmt exit_compat_elf_binfmt
+#ifndef ELF_FORMAT_NAME
+#define ELF_FORMAT_NAME "ELF compat"
+#endif
+
/*
* We share all the actual code with the native (64-bit) version.
*/
diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
index 70cf138..402a74a 100644
--- a/include/linux/binfmts.h
+++ b/include/linux/binfmts.h
@@ -70,6 +70,7 @@ struct coredump_params {
struct linux_binfmt {
struct list_head lh;
struct module *module;
+ const char *name;
int (*load_binary)(struct linux_binprm *);
int (*load_shlib)(struct file *);
int (*core_dump)(struct coredump_params *cprm);
--
1.7.1
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v5 2/3] fs/binfmts: Better handling of binfmt loops
2013-07-25 15:40 [PATCH 1/3] fs/binfmts: Add a name field to the binfmt struct Zach Levis
` (15 preceding siblings ...)
2013-08-15 16:20 ` [PATCH v5 1/3] fs/binfmts: Add a name field to the binfmt struct Zach Levis
@ 2013-08-15 16:20 ` Zach Levis
2013-08-16 13:15 ` Oleg Nesterov
2013-08-15 16:20 ` [PATCH v5 3/3] fs/binfmts: Whitespace fixes with scripts/cleanfile Zach Levis
17 siblings, 1 reply; 35+ messages in thread
From: Zach Levis @ 2013-08-15 16:20 UTC (permalink / raw)
To: akpm, oleg
Cc: viro, linux-kernel, linux-fsdevel, dan.carpenter, keescook, cody,
zml, Zach Levis
With these changes, when a binfmt loop is encountered,
the ELOOP will propogate back to the 0 depth. At this point the
binprm's state will be reset to what it was originally and an
attempt is made to continue with the following binfmt handlers.
Example: a qemu is configured to run 64-bit ELFs on an otherwise 32-bit
system. The system's owner switches to running with 64-bit executables,
but forgets to disable the binfmt_misc option that redirects 64bit ELFs
to qemu. Since the qemu executable is a 64-bit ELF now, binfmt_misc
keeps on matching it with the qemu rule, preventing the execution of any
64-bit binary.
With these changes, an error is printed and search_binary_handler()
continues on to the next handler, allowing the original executable to
run normally so the user can (hopefully) fix their misconfiguration more
easily.
Caveats:
- binfmt_misc is skipped as a whole. To allow for a more generic
solution that works for any binfmt without a lot of duplicated code
and added complexity, the error detection code is applied on the
binfmt level.
- This is a fallback solution. It attempts to restore the state to allow
executing most binaries without side effects, but it may not work for
everything and should not be depended on for regular usage.
Tested with:
- binfmt_misc line ":Testing:E::bft::/home/zlevis/test.bft:"
and script at /home/zlevis/test.bft
#!/bin/sh
echo "hi"
echo "$@"
- binfmt_script
script at /home/zlevis/test
echo "hi"
echo "$@"
Signed-off-by: Zach Levis <zach@zachsthings.com>
---
fs/exec.c | 192 ++++++++++++++++++++++++++++++++++-------------
include/linux/binfmts.h | 5 +
2 files changed, 145 insertions(+), 52 deletions(-)
diff --git a/fs/exec.c b/fs/exec.c
index eb229e1..aff9be2 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1365,6 +1365,111 @@ out:
}
EXPORT_SYMBOL(remove_arg_zero);
+static void bprm_close_file(struct linux_binprm *bprm)
+{
+ if (bprm->mm) {
+ acct_arg_size(bprm, 0);
+ mmput(bprm->mm);
+ bprm->mm = NULL;
+ }
+
+ if (bprm->file) {
+ allow_write_access(bprm->file);
+ fput(bprm->file);
+ bprm->file = NULL;
+ }
+}
+
+static int init_bprm(struct linux_binprm *bprm,
+ const char *filename,
+ struct user_arg_ptr *argv,
+ struct user_arg_ptr *envp)
+{
+ int retval;
+ struct file *file;
+
+ file = open_exec(filename);
+ retval = PTR_ERR(file);
+ if (IS_ERR(file))
+ return retval;
+
+ if (!bprm->file)
+ sched_exec();
+
+ bprm->file = file;
+ bprm->filename = filename;
+ bprm->interp = filename;
+
+ retval = bprm_mm_init(bprm);
+ if (retval)
+ goto err;
+
+ retval = bprm->argc = count(*argv, MAX_ARG_STRINGS);
+ if (retval < 0)
+ goto err;
+
+ retval = bprm->envc = count(*envp, MAX_ARG_STRINGS);
+ if (retval < 0)
+ goto err;
+
+ retval = prepare_binprm(bprm);
+ if (retval < 0)
+ goto err;
+
+ retval = copy_strings_kernel(1, &bprm->filename, bprm);
+ if (retval < 0)
+ goto err;
+
+ bprm->argv_orig = argv;
+ bprm->envp_orig = envp;
+ bprm->exec = bprm->p;
+ retval = copy_strings(bprm->envc, *envp, bprm);
+ if (retval < 0)
+ goto err;
+
+ retval = copy_strings(bprm->argc, *argv, bprm);
+ if (retval < 0)
+ goto err;
+
+out:
+ return retval;
+err:
+ bprm_close_file(bprm);
+ goto out;
+}
+
+
+static bool update_prev_binfmts(struct linux_binprm *bprm,
+ struct linux_binfmt *cur_fmt)
+{
+
+ if (!try_module_get(cur_fmt->module))
+ return false;
+ if (bprm->previous_binfmts[1])
+ put_binfmt(bprm->previous_binfmts[1]);
+ bprm->previous_binfmts[1] = bprm->previous_binfmts[0];
+ bprm->previous_binfmts[0] = cur_fmt;
+ return true;
+}
+
+static void put_binfmts(struct linux_binprm *bprm)
+{
+ if (bprm->previous_binfmts[1]) {
+ put_binfmt(bprm->previous_binfmts[1]);
+ bprm->previous_binfmts[1] = NULL;
+ }
+ if (bprm->previous_binfmts[0]) {
+ put_binfmt(bprm->previous_binfmts[0]);
+ bprm->previous_binfmts[0] = NULL;
+ }
+}
+
+/* Get name null-safely */
+static inline const char *binfmt_name(struct linux_binfmt *fmt)
+{
+ return fmt ? fmt->name : "NULL";
+}
+
#define printable(c) (((c)=='\t') || ((c)=='\n') || (0x20<=(c) && (c)<=0x7e))
/*
* cycle the list of binary formats handler, until one recognizes the image
@@ -1393,15 +1498,38 @@ int search_binary_handler(struct linux_binprm *bprm)
list_for_each_entry(fmt, &formats, lh) {
if (!try_module_get(fmt->module))
continue;
+
+ if (!update_prev_binfmts(bprm, fmt))
+ continue;
+
read_unlock(&binfmt_lock);
+
bprm->recursion_depth++;
retval = fmt->load_binary(bprm);
bprm->recursion_depth--;
- if (retval >= 0 || retval != -ENOEXEC ||
- bprm->mm == NULL || bprm->file == NULL) {
+ if (retval == -ELOOP
+ && bprm->recursion_depth == 0) { /* cur, previous */
+ pr_err("Too much recursion with binfmts (0:%s, -1:%s) in file %s, skipping (base %s).\n",
+ binfmt_name(bprm->previous_binfmts[0]),
+ binfmt_name(bprm->previous_binfmts[1]),
+ bprm->filename,
+ fmt->name);
+
+ free_arg_pages(bprm);
+ if (bprm->interp != bprm->filename)
+ kfree(bprm->interp);
+ bprm_close_file(bprm);
+ if (init_bprm(bprm, bprm->filename,
+ bprm->argv_orig, bprm->envp_orig) < 0) {
+ put_binfmt(fmt);
+ return retval;
+ }
+ } else if (retval >= 0 || retval != -ENOEXEC ||
+ bprm->mm == NULL || bprm->file == NULL) {
put_binfmt(fmt);
return retval;
}
+
read_lock(&binfmt_lock);
put_binfmt(fmt);
}
@@ -1433,9 +1561,15 @@ static int exec_binprm(struct linux_binprm *bprm)
rcu_read_unlock();
ret = search_binary_handler(bprm);
+ put_binfmts(bprm);
if (ret >= 0) {
trace_sched_process_exec(current, old_pid, bprm);
ptrace_event(PTRACE_EVENT_EXEC, old_vpid);
+ /* Successful execution, now null out the cached argv
+ * (we don't want anyone to access it later)
+ * */
+ bprm->argv_orig = NULL;
+ bprm->envp_orig = NULL;
current->did_exec = 1;
proc_exec_connector(current);
@@ -1448,6 +1582,7 @@ static int exec_binprm(struct linux_binprm *bprm)
return ret;
}
+
/*
* sys_execve() executes a new program.
*/
@@ -1456,7 +1591,6 @@ static int do_execve_common(const char *filename,
struct user_arg_ptr envp)
{
struct linux_binprm *bprm;
- struct file *file;
struct files_struct *displaced;
bool clear_in_exec;
int retval;
@@ -1497,45 +1631,9 @@ static int do_execve_common(const char *filename,
clear_in_exec = retval;
current->in_execve = 1;
- file = open_exec(filename);
- retval = PTR_ERR(file);
- if (IS_ERR(file))
- goto out_unmark;
-
- sched_exec();
-
- bprm->file = file;
- bprm->filename = filename;
- bprm->interp = filename;
-
- retval = bprm_mm_init(bprm);
- if (retval)
- goto out_file;
-
- bprm->argc = count(argv, MAX_ARG_STRINGS);
- if ((retval = bprm->argc) < 0)
- goto out;
-
- bprm->envc = count(envp, MAX_ARG_STRINGS);
- if ((retval = bprm->envc) < 0)
- goto out;
-
- retval = prepare_binprm(bprm);
+ retval = init_bprm(bprm, filename, &argv, &envp);
if (retval < 0)
- goto out;
-
- retval = copy_strings_kernel(1, &bprm->filename, bprm);
- if (retval < 0)
- goto out;
-
- bprm->exec = bprm->p;
- retval = copy_strings(bprm->envc, envp, bprm);
- if (retval < 0)
- goto out;
-
- retval = copy_strings(bprm->argc, argv, bprm);
- if (retval < 0)
- goto out;
+ goto out_unmark;
retval = exec_binprm(bprm);
if (retval < 0)
@@ -1551,17 +1649,7 @@ static int do_execve_common(const char *filename,
return retval;
out:
- if (bprm->mm) {
- acct_arg_size(bprm, 0);
- mmput(bprm->mm);
- }
-
-out_file:
- if (bprm->file) {
- allow_write_access(bprm->file);
- fput(bprm->file);
- }
-
+ bprm_close_file(bprm);
out_unmark:
if (clear_in_exec)
current->fs->in_exec = 0;
diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
index 402a74a..3451db8 100644
--- a/include/linux/binfmts.h
+++ b/include/linux/binfmts.h
@@ -8,6 +8,8 @@
#define CORENAME_MAX_SIZE 128
+struct user_arg_ptr; /* Struct from fs/exec.c, for linux_binprm->argv_orig */
+
/*
* This structure is used to hold the arguments that are used when loading binaries.
*/
@@ -32,11 +34,14 @@ struct linux_binprm {
unsigned int taso:1;
#endif
unsigned int recursion_depth;
+ /* current binfmt at 0 and previous binfmt at 1 */
+ struct linux_binfmt *previous_binfmts[2];
struct file * file;
struct cred *cred; /* new credentials */
int unsafe; /* how unsafe this exec is (mask of LSM_UNSAFE_*) */
unsigned int per_clear; /* bits to clear in current->personality */
int argc, envc;
+ struct user_arg_ptr *argv_orig, *envp_orig;
const char * filename; /* Name of binary as seen by procps */
const char * interp; /* Name of the binary really executed. Most
of the time same as filename, but could be
--
1.7.1
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v5 3/3] fs/binfmts: Whitespace fixes with scripts/cleanfile
2013-07-25 15:40 [PATCH 1/3] fs/binfmts: Add a name field to the binfmt struct Zach Levis
` (16 preceding siblings ...)
2013-08-15 16:20 ` [PATCH v5 2/3] fs/binfmts: Better handling of binfmt loops Zach Levis
@ 2013-08-15 16:20 ` Zach Levis
17 siblings, 0 replies; 35+ messages in thread
From: Zach Levis @ 2013-08-15 16:20 UTC (permalink / raw)
To: akpm, oleg
Cc: viro, linux-kernel, linux-fsdevel, dan.carpenter, keescook, cody,
zml, Zach Levis
Obligatory first-patchset whitespace commit
Signed-off-by: Zach Levis <zach@zachsthings.com>
---
fs/binfmt_aout.c | 8 ++++----
fs/binfmt_elf.c | 38 +++++++++++++++++++-------------------
fs/binfmt_elf_fdpic.c | 8 ++++----
fs/binfmt_em86.c | 9 ++++++---
fs/binfmt_flat.c | 26 +++++++++++++-------------
fs/binfmt_misc.c | 24 ++++++++++++------------
fs/binfmt_script.c | 11 +++++++----
fs/binfmt_som.c | 2 +-
fs/exec.c | 10 +++++-----
9 files changed, 71 insertions(+), 65 deletions(-)
diff --git a/fs/binfmt_aout.c b/fs/binfmt_aout.c
index f6bed04..c0a458b 100644
--- a/fs/binfmt_aout.c
+++ b/fs/binfmt_aout.c
@@ -62,7 +62,7 @@ static int aout_core_dump(struct coredump_params *cprm)
fs = get_fs();
set_fs(KERNEL_DS);
has_dumped = 1;
- strncpy(dump.u_comm, current->comm, sizeof(dump.u_comm));
+ strncpy(dump.u_comm, current->comm, sizeof(dump.u_comm));
dump.u_ar0 = offsetof(struct user, regs);
dump.signal = cprm->siginfo->si_signo;
aout_dump_thread(cprm->regs, &dump);
@@ -302,7 +302,7 @@ static int load_aout_binary(struct linux_binprm * bprm)
if ((fd_offset & ~PAGE_MASK) != 0 && printk_ratelimit())
{
- printk(KERN_WARNING
+ printk(KERN_WARNING
"fd_offset is not page aligned. Please convert program: %s\n",
bprm->file->f_path.dentry->d_name.name);
}
@@ -391,12 +391,12 @@ static int load_aout_library(struct file *file)
if ((N_TXTOFF(ex) & ~PAGE_MASK) != 0) {
if (printk_ratelimit())
{
- printk(KERN_WARNING
+ printk(KERN_WARNING
"N_TXTOFF is not page aligned. Please convert library: %s\n",
file->f_path.dentry->d_name.name);
}
vm_brk(start_addr, ex.a_text + ex.a_data + ex.a_bss);
-
+
read_code(file, start_addr, N_TXTOFF(ex),
ex.a_text + ex.a_data);
retval = 0;
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 864208b..44dfe4e 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -222,7 +222,7 @@ create_elf_tables(struct linux_binprm *bprm, struct elfhdr *exec,
} while (0)
#ifdef ARCH_DLINFO
- /*
+ /*
* ARCH_DLINFO must come first so PPC can do its special alignment of
* AUXV.
* update AT_VECTOR_SIZE_ARCH if the number of NEW_AUX_ENT() in
@@ -243,7 +243,7 @@ create_elf_tables(struct linux_binprm *bprm, struct elfhdr *exec,
NEW_AUX_ENT(AT_EUID, from_kuid_munged(cred->user_ns, cred->euid));
NEW_AUX_ENT(AT_GID, from_kgid_munged(cred->user_ns, cred->gid));
NEW_AUX_ENT(AT_EGID, from_kgid_munged(cred->user_ns, cred->egid));
- NEW_AUX_ENT(AT_SECURE, security_bprm_secureexec(bprm));
+ NEW_AUX_ENT(AT_SECURE, security_bprm_secureexec(bprm));
NEW_AUX_ENT(AT_RANDOM, (elf_addr_t)(unsigned long)u_rand_bytes);
#ifdef ELF_HWCAP2
NEW_AUX_ENT(AT_HWCAP2, ELF_HWCAP2);
@@ -437,7 +437,7 @@ static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex,
error = -EIO;
if (retval != size) {
if (retval < 0)
- error = retval;
+ error = retval;
goto out_close;
}
@@ -456,7 +456,7 @@ static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex,
unsigned long k, map_addr;
if (eppnt->p_flags & PF_R)
- elf_prot = PROT_READ;
+ elf_prot = PROT_READ;
if (eppnt->p_flags & PF_W)
elf_prot |= PROT_WRITE;
if (eppnt->p_flags & PF_X)
@@ -574,7 +574,7 @@ static unsigned long randomize_stack_top(unsigned long stack_top)
static int load_elf_binary(struct linux_binprm *bprm)
{
struct file *interpreter = NULL; /* to shut gcc up */
- unsigned long load_addr = 0, load_bias = 0;
+ unsigned long load_addr = 0, load_bias = 0;
int load_addr_set = 0;
char * elf_interpreter = NULL;
unsigned long error;
@@ -599,7 +599,7 @@ static int load_elf_binary(struct linux_binprm *bprm)
retval = -ENOMEM;
goto out_ret;
}
-
+
/* Get the exec-header */
loc->elf_ex = *((struct elfhdr *)bprm->buf);
@@ -619,7 +619,7 @@ static int load_elf_binary(struct linux_binprm *bprm)
if (loc->elf_ex.e_phentsize != sizeof(struct elf_phdr))
goto out;
if (loc->elf_ex.e_phnum < 1 ||
- loc->elf_ex.e_phnum > 65536U / sizeof(struct elf_phdr))
+ loc->elf_ex.e_phnum > 65536U / sizeof(struct elf_phdr))
goto out;
size = loc->elf_ex.e_phnum * sizeof(struct elf_phdr);
retval = -ENOMEM;
@@ -651,7 +651,7 @@ static int load_elf_binary(struct linux_binprm *bprm)
* is an a.out format binary
*/
retval = -ENOEXEC;
- if (elf_ppnt->p_filesz > PATH_MAX ||
+ if (elf_ppnt->p_filesz > PATH_MAX ||
elf_ppnt->p_filesz < 2)
goto out_free_ph;
@@ -751,7 +751,7 @@ static int load_elf_binary(struct linux_binprm *bprm)
send_sig(SIGKILL, current, 0);
goto out_free_dentry;
}
-
+
current->mm->start_stack = bprm->p;
/* Now we do a little grungy work by mmapping the ELF image into
@@ -766,7 +766,7 @@ static int load_elf_binary(struct linux_binprm *bprm)
if (unlikely (elf_brk > elf_bss)) {
unsigned long nbyte;
-
+
/* There was a PT_LOAD segment with p_memsz > p_filesz
before this one. Map anonymous pages, if needed,
and clear the area. */
@@ -1298,7 +1298,7 @@ static void fill_elf_note_phdr(struct elf_phdr *phdr, int sz, loff_t offset)
return;
}
-static void fill_note(struct memelfnote *note, const char *name, int type,
+static void fill_note(struct memelfnote *note, const char *name, int type,
unsigned int sz, void *data)
{
note->name = name;
@@ -1350,7 +1350,7 @@ static int fill_psinfo(struct elf_prpsinfo *psinfo, struct task_struct *p,
{
const struct cred *cred;
unsigned int i, len;
-
+
/* first copy the parameters from user space */
memset(psinfo, 0, sizeof(struct elf_prpsinfo));
@@ -1384,7 +1384,7 @@ static int fill_psinfo(struct elf_prpsinfo *psinfo, struct task_struct *p,
SET_GID(psinfo->pr_gid, from_kgid_munged(cred->user_ns, cred->gid));
rcu_read_unlock();
strncpy(psinfo->pr_fname, p->comm, sizeof(psinfo->pr_fname));
-
+
return 0;
}
@@ -1785,8 +1785,8 @@ static int elf_dump_thread_status(long signr, struct elf_thread_status *t)
t->num_notes = 0;
fill_prstatus(&t->prstatus, p, signr);
- elf_core_copy_task_regs(p, &t->prstatus.pr_reg);
-
+ elf_core_copy_task_regs(p, &t->prstatus.pr_reg);
+
fill_note(&t->notes[0], "CORE", NT_PRSTATUS, sizeof(t->prstatus),
&(t->prstatus));
t->num_notes++;
@@ -1807,7 +1807,7 @@ static int elf_dump_thread_status(long signr, struct elf_thread_status *t)
t->num_notes++;
sz += notesize(&t->notes[2]);
}
-#endif
+#endif
return sz;
}
@@ -2059,7 +2059,7 @@ static int elf_core_dump(struct coredump_params *cprm)
/*
* We no longer stop all VM operations.
- *
+ *
* This is because those proceses that could possibly change map_count
* or the mmap / vma pages are now blocked in do_exit on current
* finishing this core dump.
@@ -2068,7 +2068,7 @@ static int elf_core_dump(struct coredump_params *cprm)
* the map_count or the pages allocated. So no possibility of crashing
* exists while dumping the mm->vm_next areas to the core file.
*/
-
+
/* alloc memory for large data structures: too large to be on stack */
elf = kmalloc(sizeof(*elf), GFP_KERNEL);
if (!elf)
@@ -2174,7 +2174,7 @@ static int elf_core_dump(struct coredump_params *cprm)
if (!elf_core_write_extra_phdrs(cprm->file, offset, &size, cprm->limit))
goto end_coredump;
- /* write out the notes section */
+ /* write out the notes section */
if (!write_note_info(&info, cprm->file, &foffset))
goto end_coredump;
diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c
index 6be4448..799791b 100644
--- a/fs/binfmt_elf_fdpic.c
+++ b/fs/binfmt_elf_fdpic.c
@@ -1595,8 +1595,8 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm)
struct memelfnote *notes = NULL;
struct elf_prstatus *prstatus = NULL; /* NT_PRSTATUS */
struct elf_prpsinfo *psinfo = NULL; /* NT_PRPSINFO */
- LIST_HEAD(thread_list);
- struct list_head *t;
+ LIST_HEAD(thread_list);
+ struct list_head *t;
elf_fpregset_t *fpu = NULL;
#ifdef ELF_CORE_COPY_XFPREGS
elf_fpxregset_t *xfpu = NULL;
@@ -1705,7 +1705,7 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm)
fill_note(¬es[numnote++], "CORE", NT_AUXV,
i * sizeof(elf_addr_t), auxv);
- /* Try to dump the FPU. */
+ /* Try to dump the FPU. */
if ((prstatus->pr_fpvalid =
elf_core_copy_task_fpregs(current, cprm->regs, fpu)))
fill_note(notes + numnote++,
@@ -1795,7 +1795,7 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm)
if (!elf_core_write_extra_phdrs(cprm->file, offset, &size, cprm->limit))
goto end_coredump;
- /* write out the notes section */
+ /* write out the notes section */
for (i = 0; i < numnote; i++)
if (!writenote(notes + i, cprm->file, &foffset))
goto end_coredump;
diff --git a/fs/binfmt_em86.c b/fs/binfmt_em86.c
index 8404cdd..7398f7e 100644
--- a/fs/binfmt_em86.c
+++ b/fs/binfmt_em86.c
@@ -63,15 +63,18 @@ static int load_em86(struct linux_binprm *bprm)
*/
remove_arg_zero(bprm);
retval = copy_strings_kernel(1, &bprm->filename, bprm);
- if (retval < 0) return retval;
+ if (retval < 0)
+ return retval;
bprm->argc++;
if (i_arg) {
retval = copy_strings_kernel(1, &i_arg, bprm);
- if (retval < 0) return retval;
+ if (retval < 0)
+ return retval;
bprm->argc++;
}
retval = copy_strings_kernel(1, &i_name, bprm);
- if (retval < 0) return retval;
+ if (retval < 0)
+ return retval;
bprm->argc++;
/*
diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
index a7641e1..13e7ad0 100644
--- a/fs/binfmt_flat.c
+++ b/fs/binfmt_flat.c
@@ -388,7 +388,7 @@ void old_reloc(unsigned long rl)
#endif
flat_v2_reloc_t r;
unsigned long *ptr;
-
+
r.value = rl;
#if defined(CONFIG_COLDFIRE)
ptr = (unsigned long *) (current->mm->start_code + r.reloc.offset);
@@ -401,7 +401,7 @@ void old_reloc(unsigned long rl)
"(address %p, currently %x) into segment %s\n",
r.reloc.offset, ptr, (int)*ptr, segment[r.reloc.type]);
#endif
-
+
switch (r.reloc.type) {
case OLD_FLAT_RELOC_TYPE_TEXT:
*ptr += current->mm->start_code;
@@ -420,7 +420,7 @@ void old_reloc(unsigned long rl)
#ifdef DEBUG
printk("Relocation became %x\n", (int)*ptr);
#endif
-}
+}
/****************************************************************************/
@@ -479,7 +479,7 @@ static int load_flat_file(struct linux_binprm * bprm,
ret = -ENOEXEC;
goto err;
}
-
+
/* Don't allow old format executables to use shared libraries */
if (rev == OLD_FLAT_VERSION && id != 0) {
printk("BINFMT_FLAT: shared libraries are not available before rev 0x%x\n",
@@ -581,7 +581,7 @@ static int load_flat_file(struct linux_binprm * bprm,
fpos = ntohl(hdr->data_start);
#ifdef CONFIG_BINFMT_ZFLAT
if (flags & FLAT_FLAG_GZDATA) {
- result = decompress_exec(bprm, fpos, (char *) datapos,
+ result = decompress_exec(bprm, fpos, (char *) datapos,
full_data, 0);
} else
#endif
@@ -704,7 +704,7 @@ static int load_flat_file(struct linux_binprm * bprm,
libinfo->lib_list[id].loaded = 1;
libinfo->lib_list[id].entry = (0x00ffffff & ntohl(hdr->entry)) + textpos;
libinfo->lib_list[id].build_date = ntohl(hdr->build_date);
-
+
/*
* We just load the allocations into some temporary memory to
* help simplify all this mumbo jumbo
@@ -784,11 +784,11 @@ static int load_flat_file(struct linux_binprm * bprm,
for (i=0; i < relocs; i++)
old_reloc(ntohl(reloc[i]));
}
-
+
flush_icache_range(start_code, end_code);
/* zero the BSS, BRK and stack areas */
- memset((void*)(datapos + data_len), 0, bss_len +
+ memset((void *)(datapos + data_len), 0, bss_len +
(memp + memp_size - stack_len - /* end brk */
libinfo->lib_list[id].start_brk) + /* start brk */
stack_len);
@@ -882,11 +882,11 @@ static int load_flat_binary(struct linux_binprm * bprm)
stack_len += (bprm->argc + 1) * sizeof(char *); /* the argv array */
stack_len += (bprm->envc + 1) * sizeof(char *); /* the envp array */
stack_len += FLAT_STACK_ALIGN - 1; /* reserve for upcoming alignment */
-
+
res = load_flat_file(bprm, &libinfo, 0, &stack_len);
if (IS_ERR_VALUE(res))
return res;
-
+
/* Update data segment pointers for all libraries */
for (i=0; i<MAX_SHARED_LIBS; i++)
if (libinfo.lib_list[i].loaded)
@@ -908,7 +908,7 @@ static int load_flat_binary(struct linux_binprm * bprm)
((char *) page_address(bprm->page[i/PAGE_SIZE]))[i % PAGE_SIZE];
sp = (unsigned long *) create_flat_tables(p, bprm);
-
+
/* Fake some return addresses to ensure the call chain will
* initialise library in order for us. We are required to call
* lib 1 first, then 2, ... and finally the main program (id 0).
@@ -924,7 +924,7 @@ static int load_flat_binary(struct linux_binprm * bprm)
}
}
#endif
-
+
/* Stash our initial stack pointer into the mm structure */
current->mm->start_stack = (unsigned long )sp;
@@ -933,7 +933,7 @@ static int load_flat_binary(struct linux_binprm * bprm)
#endif
DBG_FLT("start_thread(regs=0x%x, entry=0x%x, start_stack=0x%x)\n",
(int)regs, (int)start_addr, (int)current->mm->start_stack);
-
+
start_thread(regs, start_addr, current->mm->start_stack);
return 0;
diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c
index d74afc0..8b9d701 100644
--- a/fs/binfmt_misc.c
+++ b/fs/binfmt_misc.c
@@ -62,7 +62,7 @@ static struct file_system_type bm_fs_type;
static struct vfsmount *bm_mnt;
static int entry_count;
-/*
+/*
* Check if we support the binfmt
* if we do, return the node, else NULL
* locking is done in load_misc_binary
@@ -138,12 +138,12 @@ static int load_misc_binary(struct linux_binprm *bprm)
/* if the binary should be opened on behalf of the
* interpreter than keep it open and assign descriptor
* to it */
- fd_binary = get_unused_fd();
- if (fd_binary < 0) {
- retval = fd_binary;
- goto _ret;
- }
- fd_install(fd_binary, bprm->file);
+ fd_binary = get_unused_fd();
+ if (fd_binary < 0) {
+ retval = fd_binary;
+ goto _ret;
+ }
+ fd_install(fd_binary, bprm->file);
/* if the binary is not readable than enforce mm->dumpable=0
regardless of the interpreter's permissions */
@@ -156,11 +156,11 @@ static int load_misc_binary(struct linux_binprm *bprm)
bprm->interp_flags |= BINPRM_FLAGS_EXECFD;
bprm->interp_data = fd_binary;
- } else {
- allow_write_access(bprm->file);
- fput(bprm->file);
- bprm->file = NULL;
- }
+ } else {
+ allow_write_access(bprm->file);
+ fput(bprm->file);
+ bprm->file = NULL;
+ }
/* make argv[1] be the path to the binary */
retval = copy_strings_kernel (1, &bprm->interp, bprm);
if (retval < 0)
diff --git a/fs/binfmt_script.c b/fs/binfmt_script.c
index 53f3c17..901da7e 100644
--- a/fs/binfmt_script.c
+++ b/fs/binfmt_script.c
@@ -45,7 +45,7 @@ static int load_script(struct linux_binprm *bprm)
break;
}
for (cp = bprm->buf+2; (*cp == ' ') || (*cp == '\t'); cp++);
- if (*cp == '\0')
+ if (*cp == '\0')
return -ENOEXEC; /* No interpreter name found */
i_name = cp;
i_arg = NULL;
@@ -70,15 +70,18 @@ static int load_script(struct linux_binprm *bprm)
if (retval)
return retval;
retval = copy_strings_kernel(1, &bprm->interp, bprm);
- if (retval < 0) return retval;
+ if (retval < 0)
+ return retval;
bprm->argc++;
if (i_arg) {
retval = copy_strings_kernel(1, &i_arg, bprm);
- if (retval < 0) return retval;
+ if (retval < 0)
+ return retval;
bprm->argc++;
}
retval = copy_strings_kernel(1, &i_name, bprm);
- if (retval) return retval;
+ if (retval)
+ return retval;
bprm->argc++;
retval = bprm_change_interp(interp, bprm);
if (retval < 0)
diff --git a/fs/binfmt_som.c b/fs/binfmt_som.c
index 1c3ccc5..88af0d1 100644
--- a/fs/binfmt_som.c
+++ b/fs/binfmt_som.c
@@ -2,7 +2,7 @@
* linux/fs/binfmt_som.c
*
* These are the functions used to load SOM format executables as used
- * by HP-UX.
+ * by HP-UX.
*
* Copyright 1999 Matthew Wilcox <willy@bofh.ai>
* based on binfmt_elf which is
diff --git a/fs/exec.c b/fs/exec.c
index aff9be2..1bce345 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -19,7 +19,7 @@
* current->executable is only used by the procfs. This allows a dispatch
* table to check for several different types of binary formats. We keep
* trying until we recognize the file or we run out of supported binary
- * formats.
+ * formats.
*/
#include <linux/slab.h>
@@ -156,7 +156,7 @@ SYSCALL_DEFINE1(uselib, const char __user *, library)
exit:
fput(file);
out:
- return error;
+ return error;
}
#ifdef CONFIG_MMU
@@ -1141,7 +1141,7 @@ void setup_new_exec(struct linux_binprm * bprm)
group */
current->self_exec_id++;
-
+
flush_signal_handlers(current, 0);
do_close_on_exec(current->files);
}
@@ -1267,8 +1267,8 @@ static int check_unsafe_exec(struct linux_binprm *bprm)
return res;
}
-/*
- * Fill the binprm structure from the inode.
+/*
+ * Fill the binprm structure from the inode.
* Check permissions, then read the first 128 (BINPRM_BUF_SIZE) bytes
*
* This may be called multiple times for binary chains (scripts for example).
--
1.7.1
^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: [PATCH v4 2/3] fs/binfmts: Better handling of binfmt loops
2013-08-14 17:50 ` Oleg Nesterov
@ 2013-08-15 16:26 ` Zach L
2013-08-16 12:23 ` Oleg Nesterov
0 siblings, 1 reply; 35+ messages in thread
From: Zach L @ 2013-08-15 16:26 UTC (permalink / raw)
To: Oleg Nesterov
Cc: akpm, viro, linux-kernel, linux-fsdevel, dan.carpenter, keescook,
cody, zml
On 08/14/2013 10:50 AM, Oleg Nesterov wrote:
> On 08/14, Zach Levis wrote:
>>
>> Example: a qemu is configured to run 64-bit ELFs on an otherwise 32-bit
>> system. The system's owner switches to running with 64-bit executables,
>> but forgets to disable the binfmt_misc option that redirects 64bit ELFs
>> to qemu. Since the qemu executable is a 64-bit ELF now, binfmt_misc
>> keeps on matching it with the qemu rule, preventing the execution of any
>> 64-bit binary.
>
> Honestly, I dislike this version even more, sorry. The patch becomes
> much more complex, and and it is still not clear to me why do we want
> these complications.
>
It's a larger patch but the majority of the increase is from is
splitting the binfmt initialization code into a separate function to
address the issue you brought up where the state of the binprm was not
entirely restored
>> My (rough, but functional) test scripts for this issue are available at:
>> https://gist.github.com/zml2008/6075418
>
> Well, suppose that someone tries to read this changelog in 2014 to
> understand the code. Are you sure this link will be still alive?
fairly likely github will be around for a while, but will put the
contents in the commit msg to be safer
>
> It would be better to have everything in the changelog, if possible.
>
>> +static void put_binfmts(struct linux_binprm *bprm, struct linux_binfmt *cur_fmt)
>> +{
>> + if (bprm->previous_binfmts[1])
>> + put_binfmt(bprm->previous_binfmts[1]);
>> + if (bprm->previous_binfmts[0])
>> + put_binfmt(bprm->previous_binfmts[0]);
>> + if (cur_fmt)
>> + put_binfmt(cur_fmt);
>> +}
>
> I didn't actually read this patch, but at first glance this doesn't look
> right. Just suppose that ->load_binary() succeeds at depth = N, this will
> be called N times.
ok, I've moved things around so this is only called once
>
[snip]
>
> And btw, if we want this, then why we only do this if recursion_depth == 0?
> Just condider '#!/path-to-the-binary-which-wants-this-patch".
Unless recursion_depth is 0, there could be a binfmt in between that
would expect its changes to the binprm to remain in effect in lower
handlers, so even with your example
>
> And again, the patch (afaics) translates -ELOOP into -ENOEXEC on failure,
> not good.
it doesn't do that, but init_binprm reset the return value to success,
which is worse. going to be fixed in the next revision
>
> Oleg.
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v5 1/3] fs/binfmts: Add a name field to the binfmt struct
2013-08-15 16:20 ` [PATCH v5 1/3] fs/binfmts: Add a name field to the binfmt struct Zach Levis
@ 2013-08-15 17:06 ` Kees Cook
0 siblings, 0 replies; 35+ messages in thread
From: Kees Cook @ 2013-08-15 17:06 UTC (permalink / raw)
To: Zach Levis
Cc: Andrew Morton, Oleg Nesterov, Al Viro, LKML, linux-fsdevel,
Dan Carpenter, cody, Zach Levis
On Thu, Aug 15, 2013 at 9:20 AM, Zach Levis <zach@zachsthings.com> wrote:
> Adding the name field helps when printing error messages referring to
> specific binfmts
>
> Signed-off-by: Zach Levis <zach@zachsthings.com>
> ---
> fs/binfmt_aout.c | 1 +
> fs/binfmt_elf.c | 5 +++++
> fs/binfmt_elf_fdpic.c | 1 +
> fs/binfmt_em86.c | 1 +
> fs/binfmt_flat.c | 1 +
> fs/binfmt_misc.c | 1 +
> fs/binfmt_script.c | 1 +
> fs/binfmt_som.c | 1 +
> fs/compat_binfmt_elf.c | 5 +++++
> include/linux/binfmts.h | 1 +
> 10 files changed, 18 insertions(+), 0 deletions(-)
>
> diff --git a/fs/binfmt_aout.c b/fs/binfmt_aout.c
> index bce8769..f6bed04 100644
> --- a/fs/binfmt_aout.c
> +++ b/fs/binfmt_aout.c
> @@ -115,6 +115,7 @@ end_coredump:
> #endif
>
> static struct linux_binfmt aout_format = {
> + .name = "a.out",
> .module = THIS_MODULE,
> .load_binary = load_aout_binary,
> .load_shlib = load_aout_library,
> diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
> index f8a0b0e..864208b 100644
> --- a/fs/binfmt_elf.c
> +++ b/fs/binfmt_elf.c
> @@ -74,7 +74,12 @@ static int elf_core_dump(struct coredump_params *cprm);
> #define ELF_PAGEOFFSET(_v) ((_v) & (ELF_MIN_ALIGN-1))
> #define ELF_PAGEALIGN(_v) (((_v) + ELF_MIN_ALIGN - 1) & ~(ELF_MIN_ALIGN - 1))
>
> +#ifndef ELF_FORMAT_NAME
> +#define ELF_FORMAT_NAME "ELF"
> +#endif
> +
> static struct linux_binfmt elf_format = {
> + .name = ELF_FORMAT_NAME,
> .module = THIS_MODULE,
> .load_binary = load_elf_binary,
> .load_shlib = load_elf_library,
> diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c
> index c166f32..6be4448 100644
> --- a/fs/binfmt_elf_fdpic.c
> +++ b/fs/binfmt_elf_fdpic.c
> @@ -81,6 +81,7 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm);
> #endif
>
> static struct linux_binfmt elf_fdpic_format = {
> + .name = "FDPIC ELF",
> .module = THIS_MODULE,
> .load_binary = load_elf_fdpic_binary,
> #ifdef CONFIG_ELF_CORE
> diff --git a/fs/binfmt_em86.c b/fs/binfmt_em86.c
> index 037a3e2..8404cdd 100644
> --- a/fs/binfmt_em86.c
> +++ b/fs/binfmt_em86.c
> @@ -93,6 +93,7 @@ static int load_em86(struct linux_binprm *bprm)
> }
>
> static struct linux_binfmt em86_format = {
> + .name = "em86",
> .module = THIS_MODULE,
> .load_binary = load_em86,
> };
> diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
> index d50bbe5..a7641e1 100644
> --- a/fs/binfmt_flat.c
> +++ b/fs/binfmt_flat.c
> @@ -92,6 +92,7 @@ static int load_flat_binary(struct linux_binprm *);
> static int flat_core_dump(struct coredump_params *cprm);
>
> static struct linux_binfmt flat_format = {
> + .name = "flat",
> .module = THIS_MODULE,
> .load_binary = load_flat_binary,
> .core_dump = flat_core_dump,
> diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c
> index 1c740e1..d74afc0 100644
> --- a/fs/binfmt_misc.c
> +++ b/fs/binfmt_misc.c
> @@ -694,6 +694,7 @@ static struct dentry *bm_mount(struct file_system_type *fs_type,
> }
>
> static struct linux_binfmt misc_format = {
> + .name = "misc",
> .module = THIS_MODULE,
> .load_binary = load_misc_binary,
> };
> diff --git a/fs/binfmt_script.c b/fs/binfmt_script.c
> index 5027a3e..53f3c17 100644
> --- a/fs/binfmt_script.c
> +++ b/fs/binfmt_script.c
> @@ -99,6 +99,7 @@ static int load_script(struct linux_binprm *bprm)
> }
>
> static struct linux_binfmt script_format = {
> + .name = "script",
> .module = THIS_MODULE,
> .load_binary = load_script,
> };
> diff --git a/fs/binfmt_som.c b/fs/binfmt_som.c
> index 4e00ed6..1c3ccc5 100644
> --- a/fs/binfmt_som.c
> +++ b/fs/binfmt_som.c
> @@ -53,6 +53,7 @@ static int som_core_dump(struct coredump_params *cprm);
> #define SOM_PAGEALIGN(_v) (((_v) + SOM_PAGESIZE - 1) & ~(SOM_PAGESIZE - 1))
>
> static struct linux_binfmt som_format = {
> + .name = "SOM",
> .module = THIS_MODULE,
> .load_binary = load_som_binary,
> .load_shlib = load_som_library,
> diff --git a/fs/compat_binfmt_elf.c b/fs/compat_binfmt_elf.c
> index a81147e..96330f3 100644
> --- a/fs/compat_binfmt_elf.c
> +++ b/fs/compat_binfmt_elf.c
> @@ -131,9 +131,14 @@ static void cputime_to_compat_timeval(const cputime_t cputime,
> * might make some debugging less confusing not to duplicate them.
> */
> #define elf_format compat_elf_format
> +#define elf_format_name compat_elf_format_name
This function doesn't exist yet, so I would suggest either moving this
change to a later patch, or moving the function and its callers
forward to this patch.
-Kees
> #define init_elf_binfmt init_compat_elf_binfmt
> #define exit_elf_binfmt exit_compat_elf_binfmt
>
> +#ifndef ELF_FORMAT_NAME
> +#define ELF_FORMAT_NAME "ELF compat"
> +#endif
> +
> /*
> * We share all the actual code with the native (64-bit) version.
> */
> diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
> index 70cf138..402a74a 100644
> --- a/include/linux/binfmts.h
> +++ b/include/linux/binfmts.h
> @@ -70,6 +70,7 @@ struct coredump_params {
> struct linux_binfmt {
> struct list_head lh;
> struct module *module;
> + const char *name;
> int (*load_binary)(struct linux_binprm *);
> int (*load_shlib)(struct file *);
> int (*core_dump)(struct coredump_params *cprm);
> --
> 1.7.1
>
--
Kees Cook
Chrome OS Security
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v4 2/3] fs/binfmts: Better handling of binfmt loops
2013-08-15 16:26 ` Zach L
@ 2013-08-16 12:23 ` Oleg Nesterov
0 siblings, 0 replies; 35+ messages in thread
From: Oleg Nesterov @ 2013-08-16 12:23 UTC (permalink / raw)
To: Zach L
Cc: akpm, viro, linux-kernel, linux-fsdevel, dan.carpenter, keescook,
cody, zml
On 08/15, Zach L wrote:
>
> On 08/14/2013 10:50 AM, Oleg Nesterov wrote:
> > On 08/14, Zach Levis wrote:
> >>
> > Honestly, I dislike this version even more, sorry. The patch becomes
> > much more complex, and and it is still not clear to me why do we want
> > these complications.
> >
> It's a larger patch but the majority of the increase is from is
> splitting the binfmt initialization code into a separate function to
> address the issue you brought up where the state of the binprm was not
> entirely restored
I understand the reason. But I do not understand the value. IMHO, the
problem this patch tries to fix falls into the "don't do this" category
and doesn't worth the trouble.
> [snip]
This certainly answers my question you snipped ;)
> > And btw, if we want this, then why we only do this if recursion_depth == 0?
> > Just condider '#!/path-to-the-binary-which-wants-this-patch".
> Unless recursion_depth is 0, there could be a binfmt in between that
> would expect its changes to the binprm to remain in effect in lower
> handlers, so even with your example
My point was, this doesn't fix the same problem if depth != 0.
But yes, "depth > 0" can't simply do init_bprm().
> > And again, the patch (afaics) translates -ELOOP into -ENOEXEC on failure,
> > not good.
> it doesn't do that,
It does, afaics. Just suppose that -ELOOP comes from load_script(). We
restore everything and call the next handler which returns ENOEXEC.
And at first glance v5 does the same.
Oleg.
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v5 2/3] fs/binfmts: Better handling of binfmt loops
2013-08-15 16:20 ` [PATCH v5 2/3] fs/binfmts: Better handling of binfmt loops Zach Levis
@ 2013-08-16 13:15 ` Oleg Nesterov
0 siblings, 0 replies; 35+ messages in thread
From: Oleg Nesterov @ 2013-08-16 13:15 UTC (permalink / raw)
To: Zach Levis
Cc: akpm, viro, linux-kernel, linux-fsdevel, dan.carpenter, keescook,
cody, zml
On 08/15, Zach Levis wrote:
>
> +static bool update_prev_binfmts(struct linux_binprm *bprm,
> + struct linux_binfmt *cur_fmt)
> +{
> +
> + if (!try_module_get(cur_fmt->module))
> + return false;
> + if (bprm->previous_binfmts[1])
> + put_binfmt(bprm->previous_binfmts[1]);
> + bprm->previous_binfmts[1] = bprm->previous_binfmts[0];
> + bprm->previous_binfmts[0] = cur_fmt;
> + return true;
> +}
Still can't understand the logic behind this function and its usage.
IOW, what ->previous_binfmts[] actually means? previous_binfmts[1]
could be a caller or the previous fmt which was called at the same
depth.
> @@ -1393,15 +1498,38 @@ int search_binary_handler(struct linux_binprm *bprm)
> list_for_each_entry(fmt, &formats, lh) {
> if (!try_module_get(fmt->module))
> continue;
> +
> + if (!update_prev_binfmts(bprm, fmt))
> + continue;
> +
> read_unlock(&binfmt_lock);
> +
> bprm->recursion_depth++;
> retval = fmt->load_binary(bprm);
> bprm->recursion_depth--;
> - if (retval >= 0 || retval != -ENOEXEC ||
> - bprm->mm == NULL || bprm->file == NULL) {
> + if (retval == -ELOOP
> + && bprm->recursion_depth == 0) { /* cur, previous */
> + pr_err("Too much recursion with binfmts (0:%s, -1:%s) in file %s, skipping (base %s).\n",
> + binfmt_name(bprm->previous_binfmts[0]),
> + binfmt_name(bprm->previous_binfmts[1]),
> + bprm->filename,
> + fmt->name);
> +
> + free_arg_pages(bprm);
> + if (bprm->interp != bprm->filename)
> + kfree(bprm->interp);
this doesn't look safe too, kfree(interp) can be called twice.
and once again, we should not lose -ELOOP as an error code if the
next fmt returns ENOEXEC.
But the main problem (in my opinion) is that this doesn't worth the
trouble, sorry.
Oleg.
^ permalink raw reply [flat|nested] 35+ messages in thread
end of thread, other threads:[~2013-08-16 13:20 UTC | newest]
Thread overview: 35+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-07-25 15:40 [PATCH 1/3] fs/binfmts: Add a name field to the binfmt struct Zach Levis
2013-07-25 15:40 ` [PATCH 2/3] fs/binfmts: Better handling of binfmt loops Zach Levis
2013-07-30 21:04 ` Andrew Morton
2013-07-30 23:16 ` Zach Levis
2013-07-30 23:26 ` Andrew Morton
2013-07-31 16:17 ` Zach Levis
2013-07-25 15:40 ` [PATCH 3/3] fs/binfmts: Whitespace fixes with scripts/cleanfile Zach Levis
2013-08-02 22:12 ` [PATCH v2 0/3] fs/binfmts: Improve handling of loops Zach Levis
2013-08-02 22:12 ` [PATCH v2 1/3] fs/binfmts: Add a name field to the binfmt struct Zach Levis
2013-08-02 22:12 ` [PATCH v2 2/3] fs/binfmts: Better handling of binfmt loops Zach Levis
2013-08-02 22:49 ` Zach Levis
2013-08-02 22:12 ` [PATCH v2 3/3] fs/binfmts: Whitespace fixes with scripts/cleanfile Zach Levis
2013-08-02 23:21 ` [PATCH v3 0/3] fs/binfmts: Improve handling of loops Zach Levis
2013-08-06 21:11 ` Kees Cook
2013-08-07 23:30 ` Zach Levis
2013-08-02 23:21 ` [PATCH v3 1/3] fs/binfmts: Add a name field to the binfmt struct Zach Levis
2013-08-03 16:41 ` Oleg Nesterov
2013-08-02 23:21 ` [PATCH v3 2/3] fs/binfmts: Better handling of binfmt loops Zach Levis
2013-08-03 16:42 ` Oleg Nesterov
2013-08-02 23:21 ` [PATCH v3 3/3] fs/binfmts: Whitespace fixes with scripts/cleanfile Zach Levis
2013-08-03 16:51 ` Oleg Nesterov
2013-08-14 16:31 ` [PATCH v4 0/3] fs/binfmts: Improve handling of loops Zach Levis
2013-08-14 16:31 ` [PATCH v4 1/3] fs/binfmts: Add a name field to the binfmt struct Zach Levis
2013-08-14 16:31 ` [PATCH v4 2/3] fs/binfmts: Better handling of binfmt loops Zach Levis
2013-08-14 17:50 ` Oleg Nesterov
2013-08-15 16:26 ` Zach L
2013-08-16 12:23 ` Oleg Nesterov
2013-08-14 18:16 ` Oleg Nesterov
2013-08-14 16:31 ` [PATCH v4 3/3] fs/binfmts: Whitespace fixes with scripts/cleanfile Zach Levis
2013-08-15 16:20 ` [PATCH v5 0/3] fs/binfmts: Improve handling of loops Zach Levis
2013-08-15 16:20 ` [PATCH v5 1/3] fs/binfmts: Add a name field to the binfmt struct Zach Levis
2013-08-15 17:06 ` Kees Cook
2013-08-15 16:20 ` [PATCH v5 2/3] fs/binfmts: Better handling of binfmt loops Zach Levis
2013-08-16 13:15 ` Oleg Nesterov
2013-08-15 16:20 ` [PATCH v5 3/3] fs/binfmts: Whitespace fixes with scripts/cleanfile Zach Levis
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).