All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] x86_64: Add safe check in a.out loaders and some coding style
@ 2013-09-18  2:54 Geyslan G. Bem
  2013-09-18  2:54 ` [PATCH 2/2] x86: Useless inode var and Coding style fixes Geyslan G. Bem
  0 siblings, 1 reply; 4+ messages in thread
From: Geyslan G. Bem @ 2013-09-18  2:54 UTC (permalink / raw)
  To: viro, linux-fsdevel; +Cc: linux-kernel, Geyslan G. Bem

ia32_aout had no safe checks concerning the mmap and f_op in this module.
It's not necessary to verify f_op in the load_aout_library, since the
prior kernel_read/vfs_read function already does.
Coding style and printk strings fixes.

Tested using qemu, a handcrafted a.out binary and a a.out linked with a
cross-compiled ld.

Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
---
 arch/x86/ia32/ia32_aout.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/arch/x86/ia32/ia32_aout.c b/arch/x86/ia32/ia32_aout.c
index bae3aba..15a8319 100644
--- a/arch/x86/ia32/ia32_aout.c
+++ b/arch/x86/ia32/ia32_aout.c
@@ -271,10 +271,17 @@ static int load_aout_binary(struct linux_binprm *bprm)
 	     N_MAGIC(ex) != QMAGIC && N_MAGIC(ex) != NMAGIC) ||
 	    N_TRSIZE(ex) || N_DRSIZE(ex) ||
 	    i_size_read(file_inode(bprm->file)) <
-	    ex.a_text+ex.a_data+N_SYMSIZE(ex)+N_TXTOFF(ex)) {
+	    ex.a_text + ex.a_data + N_SYMSIZE(ex) + N_TXTOFF(ex)) {
 		return -ENOEXEC;
 	}
 
+	/*
+	 * Requires a mmap handler. This prevents people from using a.out
+	 * as part of an exploit attack against /proc-related vulnerabilities.
+	 */
+	if (!bprm->file->f_op || !bprm->file->f_op->mmap)
+		return -ENOEXEC;
+
 	fd_offset = N_TXTOFF(ex);
 
 	/* Check initial limits. This avoids letting people circumvent
@@ -350,14 +357,13 @@ static int load_aout_binary(struct linux_binprm *bprm)
 		if ((fd_offset & ~PAGE_MASK) != 0 &&
 			    time_after(jiffies, error_time + 5*HZ)) {
 			printk(KERN_WARNING
-			       "fd_offset is not page aligned. Please convert "
-			       "program: %s\n",
+			       "fd_offset is not page aligned. Please convert program: %s\n",
 			       bprm->file->f_path.dentry->d_name.name);
 			error_time = jiffies;
 		}
 #endif
 
-		if (!bprm->file->f_op->mmap || (fd_offset & ~PAGE_MASK) != 0) {
+		if ((fd_offset & ~PAGE_MASK) != 0) {
 			vm_brk(N_TXTADDR(ex), ex.a_text+ex.a_data);
 			read_code(bprm->file, N_TXTADDR(ex), fd_offset,
 					ex.a_text+ex.a_data);
@@ -424,10 +430,17 @@ static int load_aout_library(struct file *file)
 	if ((N_MAGIC(ex) != ZMAGIC && N_MAGIC(ex) != QMAGIC) || N_TRSIZE(ex) ||
 	    N_DRSIZE(ex) || ((ex.a_entry & 0xfff) && N_MAGIC(ex) == ZMAGIC) ||
 	    i_size_read(file_inode(file)) <
-	    ex.a_text+ex.a_data+N_SYMSIZE(ex)+N_TXTOFF(ex)) {
+	    ex.a_text + ex.a_data + N_SYMSIZE(ex) + N_TXTOFF(ex)) {
 		goto out;
 	}
 
+	/*
+	 * Requires a mmap handler. This prevents people from using a.out
+	 * as part of an exploit attack against /proc-related vulnerabilities.
+	 */
+	if (!file->f_op->mmap)
+		goto out;
+
 	if (N_FLAGS(ex))
 		goto out;
 
@@ -441,8 +454,7 @@ static int load_aout_library(struct file *file)
 		static unsigned long error_time;
 		if (time_after(jiffies, error_time + 5*HZ)) {
 			printk(KERN_WARNING
-			       "N_TXTOFF is not page aligned. Please convert "
-			       "library: %s\n",
+			       "N_TXTOFF is not page aligned. Please convert library: %s\n",
 			       file->f_path.dentry->d_name.name);
 			error_time = jiffies;
 		}
-- 
1.8.4


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 2/2] x86: Useless inode var and Coding style fixes
  2013-09-18  2:54 [PATCH 1/2] x86_64: Add safe check in a.out loaders and some coding style Geyslan G. Bem
@ 2013-09-18  2:54 ` Geyslan G. Bem
  2013-09-18  5:25   ` Joe Perches
  0 siblings, 1 reply; 4+ messages in thread
From: Geyslan G. Bem @ 2013-09-18  2:54 UTC (permalink / raw)
  To: viro, linux-fsdevel; +Cc: linux-kernel, Geyslan G. Bem

file size read only one time: useless prior value allocation.
It's not necessary to verify f_op in the load_aout_library, since the
prior kernel_read/vfs_read function already does.
Coding style and printk strings fixes.

Tested using qemu, a handcrafted a.out binary and a a.out linked with a
cross-compiled ld.

Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
---
 fs/binfmt_aout.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/fs/binfmt_aout.c b/fs/binfmt_aout.c
index 89dec7f..a4af504 100644
--- a/fs/binfmt_aout.c
+++ b/fs/binfmt_aout.c
@@ -142,7 +142,8 @@ static int set_brk(unsigned long start, unsigned long end)
  * memory and creates the pointer tables from them, and puts their
  * addresses on the "stack", returning the new stack pointer value.
  */
-static unsigned long __user *create_aout_tables(char __user *p, struct linux_binprm * bprm)
+static unsigned long __user *create_aout_tables(char __user *p,
+						struct linux_binprm *bprm)
 {
 	char __user * __user *argv;
 	char __user * __user *envp;
@@ -213,7 +214,8 @@ static int load_aout_binary(struct linux_binprm * bprm)
 	if ((N_MAGIC(ex) != ZMAGIC && N_MAGIC(ex) != OMAGIC &&
 	     N_MAGIC(ex) != QMAGIC && N_MAGIC(ex) != NMAGIC) ||
 	    N_TRSIZE(ex) || N_DRSIZE(ex) ||
-	    i_size_read(file_inode(bprm->file)) < ex.a_text+ex.a_data+N_SYMSIZE(ex)+N_TXTOFF(ex)) {
+	    i_size_read(file_inode(bprm->file)) <
+	    ex.a_text + ex.a_data + N_SYMSIZE(ex) + N_TXTOFF(ex)) {
 		return -ENOEXEC;
 	}
 
@@ -299,12 +301,12 @@ 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);
 		}
 
-		if (!bprm->file->f_op->mmap||((fd_offset & ~PAGE_MASK) != 0)) {
+		if ((fd_offset & ~PAGE_MASK) != 0) {
 			vm_brk(N_TXTADDR(ex), ex.a_text+ex.a_data);
 			read_code(bprm->file, N_TXTADDR(ex), fd_offset,
 				  ex.a_text + ex.a_data);
@@ -350,14 +352,11 @@ beyond_if:
 
 static int load_aout_library(struct file *file)
 {
-	struct inode * inode;
 	unsigned long bss, start_addr, len;
 	unsigned long error;
 	int retval;
 	struct exec ex;
 
-	inode = file_inode(file);
-
 	retval = -ENOEXEC;
 	error = kernel_read(file, 0, (char *) &ex, sizeof(ex));
 	if (error != sizeof(ex))
@@ -366,7 +365,8 @@ static int load_aout_library(struct file *file)
 	/* We come in here for the regular a.out style of shared libraries */
 	if ((N_MAGIC(ex) != ZMAGIC && N_MAGIC(ex) != QMAGIC) || N_TRSIZE(ex) ||
 	    N_DRSIZE(ex) || ((ex.a_entry & 0xfff) && N_MAGIC(ex) == ZMAGIC) ||
-	    i_size_read(inode) < ex.a_text+ex.a_data+N_SYMSIZE(ex)+N_TXTOFF(ex)) {
+	    i_size_read(file_inode(file)) <
+	    ex.a_text + ex.a_data + N_SYMSIZE(ex) + N_TXTOFF(ex)) {
 		goto out;
 	}
 
@@ -374,7 +374,7 @@ static int load_aout_library(struct file *file)
 	 * Requires a mmap handler. This prevents people from using a.out
 	 * as part of an exploit attack against /proc-related vulnerabilities.
 	 */
-	if (!file->f_op || !file->f_op->mmap)
+	if (!file->f_op->mmap)
 		goto out;
 
 	if (N_FLAGS(ex))
@@ -388,12 +388,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;
-- 
1.8.4


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 2/2] x86: Useless inode var and Coding style fixes
  2013-09-18  2:54 ` [PATCH 2/2] x86: Useless inode var and Coding style fixes Geyslan G. Bem
@ 2013-09-18  5:25   ` Joe Perches
  0 siblings, 0 replies; 4+ messages in thread
From: Joe Perches @ 2013-09-18  5:25 UTC (permalink / raw)
  To: Geyslan G. Bem; +Cc: viro, linux-fsdevel, linux-kernel

On Tue, 2013-09-17 at 23:54 -0300, Geyslan G. Bem wrote:
> Coding style and printk strings fixes.
[]
> diff --git a/fs/binfmt_aout.c b/fs/binfmt_aout.c
[]
> @@ -299,12 +301,12 @@ 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);
[]
> @@ -388,12 +388,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);

I'd rather see these printks were fixed like this:
---
 fs/binfmt_aout.c | 24 ++++++++----------------
 1 file changed, 8 insertions(+), 16 deletions(-)

diff --git a/fs/binfmt_aout.c b/fs/binfmt_aout.c
index 89dec7f..3c9ec06 100644
--- a/fs/binfmt_aout.c
+++ b/fs/binfmt_aout.c
@@ -25,6 +25,7 @@
 #include <linux/init.h>
 #include <linux/coredump.h>
 #include <linux/slab.h>
+#include <linux/ratelimit.h>
 
 #include <asm/uaccess.h>
 #include <asm/cacheflush.h>
@@ -292,17 +293,12 @@ static int load_aout_binary(struct linux_binprm * bprm)
 		}
 	} else {
 		if ((ex.a_text & 0xfff || ex.a_data & 0xfff) &&
-		    (N_MAGIC(ex) != NMAGIC) && printk_ratelimit())
-		{
-			printk(KERN_NOTICE "executable not page aligned\n");
-		}
+		    (N_MAGIC(ex) != NMAGIC))
+			pr_notice_ratelimited("executable not page aligned\n");
 
-		if ((fd_offset & ~PAGE_MASK) != 0 && printk_ratelimit())
-		{
-			printk(KERN_WARNING 
-			       "fd_offset is not page aligned. Please convert program: %s\n",
-			       bprm->file->f_path.dentry->d_name.name);
-		}
+		if ((fd_offset & ~PAGE_MASK) != 0)
+			pr_warn_ratelimited("fd_offset is not page aligned. Please convert program: %s\n",
+					    bprm->file->f_path.dentry->d_name.name);
 
 		if (!bprm->file->f_op->mmap||((fd_offset & ~PAGE_MASK) != 0)) {
 			vm_brk(N_TXTADDR(ex), ex.a_text+ex.a_data);
@@ -386,12 +382,8 @@ static int load_aout_library(struct file *file)
 	start_addr =  ex.a_entry & 0xfffff000;
 
 	if ((N_TXTOFF(ex) & ~PAGE_MASK) != 0) {
-		if (printk_ratelimit())
-		{
-			printk(KERN_WARNING 
-			       "N_TXTOFF is not page aligned. Please convert library: %s\n",
-			       file->f_path.dentry->d_name.name);
-		}
+		pr_warn_ratelimited("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),




^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 1/2] x86_64: Add safe check in a.out loaders and some coding style
@ 2013-09-18  2:21 Geyslan G. Bem
  0 siblings, 0 replies; 4+ messages in thread
From: Geyslan G. Bem @ 2013-09-18  2:21 UTC (permalink / raw)
  To: tglx, mingo, hpa, x86; +Cc: linux-kernel, Geyslan G. Bem

ia32_aout had no safe checks concerning the mmap and f_op in this module.
It's not necessary to verify f_op in the load_aout_library, since the
prior kernel_read/vfs_read function already does.
Coding style and printk strings fixes.

Tested using qemu, a handcrafted a.out binary and a a.out linked with a
cross-compiled ld.

Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
---
 arch/x86/ia32/ia32_aout.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/arch/x86/ia32/ia32_aout.c b/arch/x86/ia32/ia32_aout.c
index bae3aba..15a8319 100644
--- a/arch/x86/ia32/ia32_aout.c
+++ b/arch/x86/ia32/ia32_aout.c
@@ -271,10 +271,17 @@ static int load_aout_binary(struct linux_binprm *bprm)
 	     N_MAGIC(ex) != QMAGIC && N_MAGIC(ex) != NMAGIC) ||
 	    N_TRSIZE(ex) || N_DRSIZE(ex) ||
 	    i_size_read(file_inode(bprm->file)) <
-	    ex.a_text+ex.a_data+N_SYMSIZE(ex)+N_TXTOFF(ex)) {
+	    ex.a_text + ex.a_data + N_SYMSIZE(ex) + N_TXTOFF(ex)) {
 		return -ENOEXEC;
 	}
 
+	/*
+	 * Requires a mmap handler. This prevents people from using a.out
+	 * as part of an exploit attack against /proc-related vulnerabilities.
+	 */
+	if (!bprm->file->f_op || !bprm->file->f_op->mmap)
+		return -ENOEXEC;
+
 	fd_offset = N_TXTOFF(ex);
 
 	/* Check initial limits. This avoids letting people circumvent
@@ -350,14 +357,13 @@ static int load_aout_binary(struct linux_binprm *bprm)
 		if ((fd_offset & ~PAGE_MASK) != 0 &&
 			    time_after(jiffies, error_time + 5*HZ)) {
 			printk(KERN_WARNING
-			       "fd_offset is not page aligned. Please convert "
-			       "program: %s\n",
+			       "fd_offset is not page aligned. Please convert program: %s\n",
 			       bprm->file->f_path.dentry->d_name.name);
 			error_time = jiffies;
 		}
 #endif
 
-		if (!bprm->file->f_op->mmap || (fd_offset & ~PAGE_MASK) != 0) {
+		if ((fd_offset & ~PAGE_MASK) != 0) {
 			vm_brk(N_TXTADDR(ex), ex.a_text+ex.a_data);
 			read_code(bprm->file, N_TXTADDR(ex), fd_offset,
 					ex.a_text+ex.a_data);
@@ -424,10 +430,17 @@ static int load_aout_library(struct file *file)
 	if ((N_MAGIC(ex) != ZMAGIC && N_MAGIC(ex) != QMAGIC) || N_TRSIZE(ex) ||
 	    N_DRSIZE(ex) || ((ex.a_entry & 0xfff) && N_MAGIC(ex) == ZMAGIC) ||
 	    i_size_read(file_inode(file)) <
-	    ex.a_text+ex.a_data+N_SYMSIZE(ex)+N_TXTOFF(ex)) {
+	    ex.a_text + ex.a_data + N_SYMSIZE(ex) + N_TXTOFF(ex)) {
 		goto out;
 	}
 
+	/*
+	 * Requires a mmap handler. This prevents people from using a.out
+	 * as part of an exploit attack against /proc-related vulnerabilities.
+	 */
+	if (!file->f_op->mmap)
+		goto out;
+
 	if (N_FLAGS(ex))
 		goto out;
 
@@ -441,8 +454,7 @@ static int load_aout_library(struct file *file)
 		static unsigned long error_time;
 		if (time_after(jiffies, error_time + 5*HZ)) {
 			printk(KERN_WARNING
-			       "N_TXTOFF is not page aligned. Please convert "
-			       "library: %s\n",
+			       "N_TXTOFF is not page aligned. Please convert library: %s\n",
 			       file->f_path.dentry->d_name.name);
 			error_time = jiffies;
 		}
-- 
1.8.4


^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2013-09-18  5:25 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-09-18  2:54 [PATCH 1/2] x86_64: Add safe check in a.out loaders and some coding style Geyslan G. Bem
2013-09-18  2:54 ` [PATCH 2/2] x86: Useless inode var and Coding style fixes Geyslan G. Bem
2013-09-18  5:25   ` Joe Perches
  -- strict thread matches above, loose matches on Subject: below --
2013-09-18  2:21 [PATCH 1/2] x86_64: Add safe check in a.out loaders and some coding style Geyslan G. Bem

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.