Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1449164 > unrolled thread

[PATCH v5 12/15] binfmt_flat: allow compressed flat binary format to work on MMU systems

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v5 12/15] binfmt_flat: allow compressed flat binary format to work on MMU systems Nicolas Pitre <nicolas.pitre@linaro.org> - 2016-07-24 17:40 +0200
    Re: [PATCH v5 12/15] binfmt_flat: allow compressed flat binary format to work on MMU systems Arnd Bergmann <arnd@arndb.de> - 2016-07-24 21:50 +0200
      Re: [PATCH v5 12/15] binfmt_flat: allow compressed flat binary format  to work on MMU systems Nicolas Pitre <nicolas.pitre@linaro.org> - 2016-07-24 22:30 +0200
        Re: [PATCH v5 12/15] binfmt_flat: allow compressed flat binary format to work on MMU systems Arnd Bergmann <arnd@arndb.de> - 2016-07-25 10:20 +0200

#1449164 — [PATCH v5 12/15] binfmt_flat: allow compressed flat binary format to work on MMU systems

Let's take the simple and obvious approach by decompressing the binary
into a kernel buffer and then copying it to user space.  Those who are
looking for top performance on an MMU system are unlikely to choose this
executable format anyway.

Signed-off-by: Nicolas Pitre <nico@linaro.org>
Reviewed-by: Greg Ungerer <gerg@linux-m68k.org>
---
 fs/binfmt_flat.c | 44 ++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 42 insertions(+), 2 deletions(-)

diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
index 7b999aebad..98cfefadb6 100644
--- a/fs/binfmt_flat.c
+++ b/fs/binfmt_flat.c
@@ -34,6 +34,7 @@
 #include <linux/init.h>
 #include <linux/flat.h>
 #include <linux/uaccess.h>
+#include <linux/vmalloc.h>
 
 #include <asm/byteorder.h>
 #include <asm/unaligned.h>
@@ -628,6 +629,7 @@ static int load_flat_file(struct linux_binprm *bprm,
 		 * load it all in and treat it like a RAM load from now on
 		 */
 		if (flags & FLAT_FLAG_GZIP) {
+#ifndef CONFIG_MMU
 			result = decompress_exec(bprm, sizeof(struct flat_hdr),
 					 (((char *)textpos) + sizeof(struct flat_hdr)),
 					 (text_len + full_data
@@ -635,13 +637,51 @@ static int load_flat_file(struct linux_binprm *bprm,
 					 0);
 			memmove((void *) datapos, (void *) realdatastart,
 					full_data);
+#else
+			/*
+			 * This is used on MMU systems mainly for testing.
+			 * Let's use a kernel buffer to simplify things.
+			 */
+			long unz_text_len = text_len - sizeof(struct flat_hdr);
+			long unz_len = unz_text_len + full_data;
+			char *unz_data = vmalloc(unz_len);
+			if (!unz_data) {
+				result = -ENOMEM;
+			} else {
+				result = decompress_exec(bprm, sizeof(struct flat_hdr),
+							 unz_data, unz_len, 0);
+				if (result == 0 &&
+				    (copy_to_user((void __user *)textpos + sizeof(struct flat_hdr),
+						  unz_data, unz_text_len) ||
+				     copy_to_user((void __user *)datapos,
+						  unz_data + unz_text_len, full_data)))
+					result = -EFAULT;
+				vfree(unz_data);
+			}
+#endif
 		} else if (flags & FLAT_FLAG_GZDATA) {
 			result = read_code(bprm->file, textpos, 0, text_len);
-			if (!IS_ERR_VALUE(result))
+			if (!IS_ERR_VALUE(result)) {
+#ifndef CONFIG_MMU
 				result = decompress_exec(bprm, text_len, (char *) datapos,
 						 full_data, 0);
-		} else
+#else
+				char *unz_data = vmalloc(full_data);
+				if (!unz_data) {
+					result = -ENOMEM;
+				} else {
+					result = decompress_exec(bprm, text_len,
+						       unz_data, full_data, 0);
+					if (result == 0 &&
+					    copy_to_user((void __user *)datapos,
+							 unz_data, full_data))
+						result = -EFAULT;
+					vfree(unz_data);
+				}
 #endif
+			}
+		} else
+#endif /* CONFIG_BINFMT_ZFLAT */
 		{
 			result = read_code(bprm->file, textpos, 0, text_len);
 			if (!IS_ERR_VALUE(result))
-- 
2.7.4

[toc] | [next] | [standalone]

#1449194

On Sunday, July 24, 2016 11:30:26 AM CEST Nicolas Pitre wrote:
> +#else
> +                       /*
> +                        * This is used on MMU systems mainly for testing.
> +                        * Let's use a kernel buffer to simplify things.
> +                        */
> +                       long unz_text_len = text_len - sizeof(struct flat_hdr);
> +                       long unz_len = unz_text_len + full_data;
> +                       char *unz_data = vmalloc(unz_len);
> +                       if (!unz_data) {
> +                               result = -ENOMEM;
> 

Is there a risk of a malicious user exhausting vmalloc space with a
binary that has forged headers? If there is, maybe put an upper bound on
the size of allocation.

More broadly speaking, are there any other attacks that may get enabled
through forged binaries? We've had a couple of vulnerabilities in
binfmt_elf over the years, and I wonder how dangerous it might be
if distros turn on binfmt_flat support by default.

	Arnd

[toc] | [prev] | [next] | [standalone]

#1449200 — Re: [PATCH v5 12/15] binfmt_flat: allow compressed flat binary format to work on MMU systems

On Sun, 24 Jul 2016, Arnd Bergmann wrote:

> On Sunday, July 24, 2016 11:30:26 AM CEST Nicolas Pitre wrote:
> > +#else
> > +                       /*
> > +                        * This is used on MMU systems mainly for testing.
> > +                        * Let's use a kernel buffer to simplify things.
> > +                        */
> > +                       long unz_text_len = text_len - sizeof(struct flat_hdr);
> > +                       long unz_len = unz_text_len + full_data;
> > +                       char *unz_data = vmalloc(unz_len);
> > +                       if (!unz_data) {
> > +                               result = -ENOMEM;
> > 
> 
> Is there a risk of a malicious user exhausting vmalloc space with a
> binary that has forged headers? If there is, maybe put an upper bound on
> the size of allocation.

Patch #3 enforces a cap on all parameters to avoid overflows and 
unreasonable section sizes.

Then vmalloc space is used here only for decompressing the binary into, 
after which the whole thing is copied to user space and the vmalloc area 
is freed right away.

> More broadly speaking, are there any other attacks that may get enabled
> through forged binaries? We've had a couple of vulnerabilities in
> binfmt_elf over the years, and I wonder how dangerous it might be
> if distros turn on binfmt_flat support by default.

That was Alan's concern too which prompted patch #3. But with a clamp on 
all parameters, everything else is done via user accessors.  So an 
executable still can crap onto itself or generate a segfault but I doubt 
we really care at that point.


Nicolas

[toc] | [prev] | [next] | [standalone]

#1449350

On Sunday, July 24, 2016 4:25:16 PM CEST Nicolas Pitre wrote:
> On Sun, 24 Jul 2016, Arnd Bergmann wrote:
> 
> > On Sunday, July 24, 2016 11:30:26 AM CEST Nicolas Pitre wrote:
> > > +#else
> > > +                       /*
> > > +                        * This is used on MMU systems mainly for testing.
> > > +                        * Let's use a kernel buffer to simplify things.
> > > +                        */
> > > +                       long unz_text_len = text_len - sizeof(struct flat_hdr);
> > > +                       long unz_len = unz_text_len + full_data;
> > > +                       char *unz_data = vmalloc(unz_len);
> > > +                       if (!unz_data) {
> > > +                               result = -ENOMEM;
> > > 
> > 
> > Is there a risk of a malicious user exhausting vmalloc space with a
> > binary that has forged headers? If there is, maybe put an upper bound on
> > the size of allocation.
> 
> Patch #3 enforces a cap on all parameters to avoid overflows and 
> unreasonable section sizes.
> 
> Then vmalloc space is used here only for decompressing the binary into, 
> after which the whole thing is copied to user space and the vmalloc area 
> is freed right away.
> 
> > More broadly speaking, are there any other attacks that may get enabled
> > through forged binaries? We've had a couple of vulnerabilities in
> > binfmt_elf over the years, and I wonder how dangerous it might be
> > if distros turn on binfmt_flat support by default.
> 
> That was Alan's concern too which prompted patch #3. But with a clamp on 
> all parameters, everything else is done via user accessors.  So an 
> executable still can crap onto itself or generate a segfault but I doubt 
> we really care at that point.
> 

Ok, sounds good.

	Arnd

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web