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

Groups > linux.kernel > #1445181 > unrolled thread

[PATCH v2 10/10] 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 v2 10/10] binfmt_flat: allow compressed flat binary format to work on MMU systems Nicolas Pitre <nicolas.pitre@linaro.org> - 2016-07-18 05:40 +0200
    Re: [PATCH v2 10/10] binfmt_flat: allow compressed flat binary  format to work on MMU systems One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk> - 2016-07-18 13:50 +0200
      Re: [PATCH v2 10/10] binfmt_flat: allow compressed flat binary format  to work on MMU systems Nicolas Pitre <nicolas.pitre@linaro.org> - 2016-07-18 17:50 +0200
        Re: [PATCH v2 10/10] binfmt_flat: allow compressed flat binary  format to work on MMU systems One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk> - 2016-07-18 19:00 +0200
          Re: [PATCH v2 10/10] binfmt_flat: allow compressed flat binary format  to work on MMU systems Nicolas Pitre <nicolas.pitre@linaro.org> - 2016-07-18 19:00 +0200

#1445181 — [PATCH v2 10/10] 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 more performance on a MMU system are unlikely to choose this
executable format anyway.

Signed-off-by: Nicolas Pitre <nico@linaro.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 4cb0c4b6ae..24deae4dcb 100644
--- a/fs/binfmt_flat.c
+++ b/fs/binfmt_flat.c
@@ -35,6 +35,7 @@
 #include <linux/init.h>
 #include <linux/flat.h>
 #include <linux/syscalls.h>
+#include <linux/vmalloc.h>
 
 #include <asm/byteorder.h>
 #include <asm/uaccess.h>
@@ -637,6 +638,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
@@ -644,14 +646,52 @@ 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
+				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
+#endif /* CONFIG_BINFMT_ZFLAT */
 		{
 			result = read_code(bprm->file, textpos, 0, text_len);
 			if (!IS_ERR_VALUE(result))
-- 
2.7.4

[toc] | [next] | [standalone]

#1445412 — Re: [PATCH v2 10/10] binfmt_flat: allow compressed flat binary format to work on MMU systems

On Sun, 17 Jul 2016 23:31:56 -0400
Nicolas Pitre <nicolas.pitre@linaro.org> wrote:

> 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 more performance on a MMU system are unlikely to choose this
> executable format anyway.

The flat loader takes a very casual attitude to overruns and corrupted
binaries. It's after all MMUless so has no real security model. If you
enable flat for an MMU system then IMHO those all need to be fixed
including all the missing overflow checks on the maths on textlen and the
like.

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

#1445596 — Re: [PATCH v2 10/10] binfmt_flat: allow compressed flat binary format to work on MMU systems

On Mon, 18 Jul 2016, One Thousand Gnomes wrote:

> On Sun, 17 Jul 2016 23:31:56 -0400
> Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> 
> > 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 more performance on a MMU system are unlikely to choose this
> > executable format anyway.
> 
> The flat loader takes a very casual attitude to overruns and corrupted
> binaries. It's after all MMUless so has no real security model. If you
> enable flat for an MMU system then IMHO those all need to be fixed
> including all the missing overflow checks on the maths on textlen and the
> like.

What about the following patch?  This with existing user accessors and 
allocation error checks should cover it all.

----- >8
commit cc1051c9c57202772568600e96b75229a2a7cf19
Author: Nicolas Pitre <nicolas.pitre@linaro.org>
Date:   Mon Jul 18 11:28:57 2016 -0400

    binfmt_flat: prevent kernel dammage from corrupted executable headers
    
    Signed-off-by: Nicolas Pitre <nico@linaro.org>

diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
index 24deae4dcb..fa0054c1c3 100644
--- a/fs/binfmt_flat.c
+++ b/fs/binfmt_flat.c
@@ -498,6 +498,17 @@ static int load_flat_file(struct linux_binprm * bprm,
 	}
 
 	/*
+	 * Make sure the header params are sane.
+	 * 28 bits (256 MB) is way more than reasonable in this case.
+	 * If some top bits are set we have probable binary corruption.
+	*/
+	if ((text_len | data_len | bss_len | stack_len | full_data) >> 28) {
+		printk("BINFMT_FLAT: bad header\n");
+		ret = -ENOEXEC;
+		goto err;
+	}
+
+	/*
 	 * fix up the flags for the older format,  there were all kinds
 	 * of endian hacks,  this only works for the simple cases
 	 */




> 

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

#1445642 — Re: [PATCH v2 10/10] binfmt_flat: allow compressed flat binary format to work on MMU systems

On Mon, 18 Jul 2016 11:45:53 -0400 (EDT)
Nicolas Pitre <nicolas.pitre@linaro.org> wrote:

> On Mon, 18 Jul 2016, One Thousand Gnomes wrote:
> 
> > On Sun, 17 Jul 2016 23:31:56 -0400
> > Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> >   
> > > 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 more performance on a MMU system are unlikely to choose this
> > > executable format anyway.  
> > 
> > The flat loader takes a very casual attitude to overruns and corrupted
> > binaries. It's after all MMUless so has no real security model. If you
> > enable flat for an MMU system then IMHO those all need to be fixed
> > including all the missing overflow checks on the maths on textlen and the
> > like.  
> 
> What about the following patch?  This with existing user accessors and 
> allocation error checks should cover it all.
> 
> ----- >8  
> commit cc1051c9c57202772568600e96b75229a2a7cf19
> Author: Nicolas Pitre <nicolas.pitre@linaro.org>
> Date:   Mon Jul 18 11:28:57 2016 -0400
> 
>     binfmt_flat: prevent kernel dammage from corrupted executable headers
>     
>     Signed-off-by: Nicolas Pitre <nico@linaro.org>
> 
> diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
> index 24deae4dcb..fa0054c1c3 100644
> --- a/fs/binfmt_flat.c
> +++ b/fs/binfmt_flat.c
> @@ -498,6 +498,17 @@ static int load_flat_file(struct linux_binprm * bprm,
>  	}
>  
>  	/*
> +	 * Make sure the header params are sane.
> +	 * 28 bits (256 MB) is way more than reasonable in this case.
> +	 * If some top bits are set we have probable binary corruption.
> +	*/
> +	if ((text_len | data_len | bss_len | stack_len | full_data) >> 28) {
> +		printk("BINFMT_FLAT: bad header\n");

Apart from the printk that looks good for the header but I think the rest
could do with a fair bit more review (eg relocations in range checks).


Alan

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

#1445643 — Re: [PATCH v2 10/10] binfmt_flat: allow compressed flat binary format to work on MMU systems

On Mon, 18 Jul 2016, One Thousand Gnomes wrote:

> On Mon, 18 Jul 2016 11:45:53 -0400 (EDT)
> Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> 
> > On Mon, 18 Jul 2016, One Thousand Gnomes wrote:
> > 
> > > On Sun, 17 Jul 2016 23:31:56 -0400
> > > Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> > >   
> > > > 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 more performance on a MMU system are unlikely to choose this
> > > > executable format anyway.  
> > > 
> > > The flat loader takes a very casual attitude to overruns and corrupted
> > > binaries. It's after all MMUless so has no real security model. If you
> > > enable flat for an MMU system then IMHO those all need to be fixed
> > > including all the missing overflow checks on the maths on textlen and the
> > > like.  
> > 
> > What about the following patch?  This with existing user accessors and 
> > allocation error checks should cover it all.
> > 
> > ----- >8  
> > commit cc1051c9c57202772568600e96b75229a2a7cf19
> > Author: Nicolas Pitre <nicolas.pitre@linaro.org>
> > Date:   Mon Jul 18 11:28:57 2016 -0400
> > 
> >     binfmt_flat: prevent kernel dammage from corrupted executable headers
> >     
> >     Signed-off-by: Nicolas Pitre <nico@linaro.org>
> > 
> > diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
> > index 24deae4dcb..fa0054c1c3 100644
> > --- a/fs/binfmt_flat.c
> > +++ b/fs/binfmt_flat.c
> > @@ -498,6 +498,17 @@ static int load_flat_file(struct linux_binprm * bprm,
> >  	}
> >  
> >  	/*
> > +	 * Make sure the header params are sane.
> > +	 * 28 bits (256 MB) is way more than reasonable in this case.
> > +	 * If some top bits are set we have probable binary corruption.
> > +	*/
> > +	if ((text_len | data_len | bss_len | stack_len | full_data) >> 28) {
> > +		printk("BINFMT_FLAT: bad header\n");
> 
> Apart from the printk that looks good for the header but I think the rest
> could do with a fair bit more review (eg relocations in range checks).

Given that they all go through put_user() now, the worst that could 
happen is an executable that craps onto itself.  I don't think there is 
much we can do here besides letting the user task crash.


Nicolas

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web