[PATCH] 9p: remove unused 'p9_fid' struct pointer

[PATCH] 9p: remove unused 'p9_fid' struct pointer

[Geyslan G. Bem]

Get rid of the useless '*fid' variable.

Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
---
 fs/9p/cache.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/9p/cache.c b/fs/9p/cache.c
index a9ea73d..50f9d9c 100644
--- a/fs/9p/cache.c
+++ b/fs/9p/cache.c
@@ -239,13 +239,12 @@ void v9fs_cache_inode_flush_cookie(struct inode *inode)
 void v9fs_cache_inode_set_cookie(struct inode *inode, struct file *filp)
 {
 	struct v9fs_inode *v9inode = V9FS_I(inode);
-	struct p9_fid *fid;
 
 	if (!v9inode->fscache)
 		return;
 
 	spin_lock(&v9inode->fscache_lock);
-	fid = filp->private_data;
+
 	if ((filp->f_flags & O_ACCMODE) != O_RDONLY)
 		v9fs_cache_inode_flush_cookie(inode);
 	else
-- 
1.8.4

[PATCH] 9p: remove unused 'p9_client' struct pointer

[Geyslan G. Bem]

Get rid of the useless '*clnt' variable.

Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
---
 fs/9p/vfs_file.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/fs/9p/vfs_file.c b/fs/9p/vfs_file.c
index aa5ecf4..42b7286 100644
--- a/fs/9p/vfs_file.c
+++ b/fs/9p/vfs_file.c
@@ -463,14 +463,12 @@ v9fs_file_write_internal(struct inode *inode, struct p9_fid *fid,
 	int n;
 	loff_t i_size;
 	size_t total = 0;
-	struct p9_client *clnt;
 	loff_t origin = *offset;
 	unsigned long pg_start, pg_end;
 
 	p9_debug(P9_DEBUG_VFS, "data %p count %d offset %x\n",
 		 data, (int)count, (int)*offset);
 
-	clnt = fid->clnt;
 	do {
 		n = p9_client_write(fid, NULL, data+total, origin+total, count);
 		if (n <= 0)
-- 
1.8.4

[PATCH] 9p: remove useless assignment

[Geyslan G. Bem]

There is no use of pointer 'fid' before the next assignment.

Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
---
 fs/9p/vfs_inode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c
index 94de6d1..7566477 100644
--- a/fs/9p/vfs_inode.c
+++ b/fs/9p/vfs_inode.c
@@ -867,7 +867,7 @@ v9fs_vfs_atomic_open(struct inode *dir, struct dentry *dentry,
 		return finish_no_open(file, res);
 
 	err = 0;
-	fid = NULL;
+
 	v9ses = v9fs_inode2v9ses(dir);
 	perm = unixmode2p9mode(v9ses, mode);
 	fid = v9fs_create(v9ses, dir, dentry, NULL, perm,
-- 
1.8.4

[PATCH] 9p: remove unused 'super_block' struct pointer

[Geyslan G. Bem]

Get rid of the useless '*sb' variable.

Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
---
 fs/9p/vfs_inode.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c
index 7566477..b352457 100644
--- a/fs/9p/vfs_inode.c
+++ b/fs/9p/vfs_inode.c
@@ -783,7 +783,6 @@ struct dentry *v9fs_vfs_lookup(struct inode *dir, struct dentry *dentry,
 				      unsigned int flags)
 {
 	struct dentry *res;
-	struct super_block *sb;
 	struct v9fs_session_info *v9ses;
 	struct p9_fid *dfid, *fid;
 	struct inode *inode;
@@ -795,7 +794,6 @@ struct dentry *v9fs_vfs_lookup(struct inode *dir, struct dentry *dentry,
 	if (dentry->d_name.len > NAME_MAX)
 		return ERR_PTR(-ENAMETOOLONG);
 
-	sb = dir->i_sb;
 	v9ses = v9fs_inode2v9ses(dir);
 	/* We can walk d_parent because we hold the dir->i_mutex */
 	dfid = v9fs_fid_lookup(dentry->d_parent);
-- 
1.8.4

[PATCH] 9p: fix return value in case of error in v9fs_fid_xattr_set

[Geyslan G. Bem]

In case of error in the p9_client_write, the function v9fs_fid_xattr_set
should return its negative value, what was never being done.

Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
---
 fs/9p/xattr.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/fs/9p/xattr.c b/fs/9p/xattr.c
index 3c28cdf..0788388 100644
--- a/fs/9p/xattr.c
+++ b/fs/9p/xattr.c
@@ -149,11 +149,10 @@ int v9fs_fid_xattr_set(struct p9_fid *fid, const char *name,
 			write_count = value_len;
 		write_count = p9_client_write(fid, ((char *)value)+offset,
 					NULL, offset, write_count);
-		if (write_count < 0) {
-			/* error in xattr write */
-			retval = write_count;
-			break;
-		}
+		/* error in xattr write */
+		if (write_count < 0)
+			return write_count;
+
 		offset += write_count;
 		value_len -= write_count;
 	}
-- 
1.8.4

Re: [PATCH] 9p: fix return value in case of error in v9fs_fid_xattr_set

[Geyslan Gregório Bem]

At first, thanks for reply.

2013/10/20 Eric Van Hensbergen <ericvh@gmail.com>:
> On Sat, Sep 28, 2013 at 6:32 PM, Geyslan G. Bem <geyslan@gmail.com> wrote:
>>
>> In case of error in the p9_client_write, the function v9fs_fid_xattr_set
>> should return its negative value, what was never being done.
>>
>> Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
>> ---
>>  fs/9p/xattr.c | 9 ++++-----
>>  1 file changed, 4 insertions(+), 5 deletions(-)
>>
>> diff --git a/fs/9p/xattr.c b/fs/9p/xattr.c
>> index 3c28cdf..0788388 100644
>> --- a/fs/9p/xattr.c
>> +++ b/fs/9p/xattr.c
>> @@ -149,11 +149,10 @@ int v9fs_fid_xattr_set(struct p9_fid *fid, const
>> char *name,
>>                         write_count = value_len;
>>                 write_count = p9_client_write(fid, ((char *)value)+offset,
>>                                         NULL, offset, write_count);
>> -               if (write_count < 0) {
>> -                       /* error in xattr write */
>> -                       retval = write_count;
>> -                       break;
>> -               }
>> +               /* error in xattr write */
>> +               if (write_count < 0)
>> +                       return write_count;
>> +
>>
>
>
> So, I'm convinced that there's a problem here, but I think the solution in
> the patch is incomplete.  Simply returning wouldn't clunk the fid.  I think
> the right approach is likely to keep the break, clunk and return an error if
> either the p9_client_write or the p9_client_clunk fails.
>
> I suppose you could make a claim that v9fs_fid_xattr_set shouldn't be
> clunking the fid -- but considering it's cloned the fid in its function
> body, it does seem like it shoudl also be cleaning up after itself.
>

Right. I'll centralize the exiting assuring that fid will be clunked
in case of fails.

>     -eric
>
>
>       -eric
>

Re: [PATCH] 9p: fix return value in case of error in v9fs_fid_xattr_set

[Geyslan Gregório Bem]

2013/10/21 Geyslan Gregório Bem <geyslan@gmail.com>:
> At first, thanks for reply.
>
> 2013/10/20 Eric Van Hensbergen <ericvh@gmail.com>:
>> On Sat, Sep 28, 2013 at 6:32 PM, Geyslan G. Bem <geyslan@gmail.com> wrote:
>>>
>>> In case of error in the p9_client_write, the function v9fs_fid_xattr_set
>>> should return its negative value, what was never being done.
>>>
>>> Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
>>> ---
>>>  fs/9p/xattr.c | 9 ++++-----
>>>  1 file changed, 4 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/fs/9p/xattr.c b/fs/9p/xattr.c
>>> index 3c28cdf..0788388 100644
>>> --- a/fs/9p/xattr.c
>>> +++ b/fs/9p/xattr.c
>>> @@ -149,11 +149,10 @@ int v9fs_fid_xattr_set(struct p9_fid *fid, const
>>> char *name,
>>>                         write_count = value_len;
>>>                 write_count = p9_client_write(fid, ((char *)value)+offset,
>>>                                         NULL, offset, write_count);
>>> -               if (write_count < 0) {
>>> -                       /* error in xattr write */
>>> -                       retval = write_count;
>>> -                       break;
>>> -               }
>>> +               /* error in xattr write */
>>> +               if (write_count < 0)
>>> +                       return write_count;
>>> +
>>>
>>
>>
>> So, I'm convinced that there's a problem here, but I think the solution in
>> the patch is incomplete.  Simply returning wouldn't clunk the fid.  I think
>> the right approach is likely to keep the break, clunk and return an error if
>> either the p9_client_write or the p9_client_clunk fails.
>>
>> I suppose you could make a claim that v9fs_fid_xattr_set shouldn't be
>> clunking the fid -- but considering it's cloned the fid in its function
>> body, it does seem like it shoudl also be cleaning up after itself.
>>
>
> Right. I'll centralize the exiting assuring that fid will be clunked
> in case of fails.
>
>>     -eric
>>
>>
>>       -eric
>>

New version sent:
 [PATCH] 9p: fix return value in case in v9fs_fid_xattr_set()

[PATCH] 9p: remove never used return variable

[Geyslan G. Bem]

Get rid of the useless 'err' variable, since the return is treated
farther down without the use of it.

Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
---
 fs/9p/vfs_inode_dotl.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/fs/9p/vfs_inode_dotl.c b/fs/9p/vfs_inode_dotl.c
index a7c4814..a892c2f 100644
--- a/fs/9p/vfs_inode_dotl.c
+++ b/fs/9p/vfs_inode_dotl.c
@@ -477,13 +477,11 @@ static int
 v9fs_vfs_getattr_dotl(struct vfsmount *mnt, struct dentry *dentry,
 		 struct kstat *stat)
 {
-	int err;
 	struct v9fs_session_info *v9ses;
 	struct p9_fid *fid;
 	struct p9_stat_dotl *st;
 
 	p9_debug(P9_DEBUG_VFS, "dentry: %p\n", dentry);
-	err = -EPERM;
 	v9ses = v9fs_dentry2v9ses(dentry);
 	if (v9ses->cache == CACHE_LOOSE || v9ses->cache == CACHE_FSCACHE) {
 		generic_fillattr(dentry->d_inode, stat);
-- 
1.8.4

[PATCH] 9p: remove useless variable and assignment

[Geyslan G. Bem]

There is no use of pointer 'v9ses'. Get rid of useless 'retval'
assignment.

Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
---
 fs/9p/vfs_inode_dotl.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/fs/9p/vfs_inode_dotl.c b/fs/9p/vfs_inode_dotl.c
index a892c2f..6803758 100644
--- a/fs/9p/vfs_inode_dotl.c
+++ b/fs/9p/vfs_inode_dotl.c
@@ -558,7 +558,6 @@ static int v9fs_mapped_iattr_valid(int iattr_valid)
 int v9fs_vfs_setattr_dotl(struct dentry *dentry, struct iattr *iattr)
 {
 	int retval;
-	struct v9fs_session_info *v9ses;
 	struct p9_fid *fid;
 	struct p9_iattr_dotl p9attr;
 	struct inode *inode = dentry->d_inode;
@@ -579,8 +578,6 @@ int v9fs_vfs_setattr_dotl(struct dentry *dentry, struct iattr *iattr)
 	p9attr.mtime_sec = iattr->ia_mtime.tv_sec;
 	p9attr.mtime_nsec = iattr->ia_mtime.tv_nsec;
 
-	retval = -EPERM;
-	v9ses = v9fs_dentry2v9ses(dentry);
 	fid = v9fs_fid_lookup(dentry);
 	if (IS_ERR(fid))
 		return PTR_ERR(fid);
-- 
1.8.4

[PATCH] 9p: proper use of the 'name' variable

[Geyslan G. Bem]

The 'name' variable was assigned but never used. Hence puts its
assignment to the top and makes proper use of its value.

Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
---
 fs/9p/vfs_inode_dotl.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/9p/vfs_inode_dotl.c b/fs/9p/vfs_inode_dotl.c
index 6803758..86b6944 100644
--- a/fs/9p/vfs_inode_dotl.c
+++ b/fs/9p/vfs_inode_dotl.c
@@ -772,8 +772,10 @@ v9fs_vfs_link_dotl(struct dentry *old_dentry, struct inode *dir,
 	struct p9_fid *dfid, *oldfid;
 	struct v9fs_session_info *v9ses;
 
+	name = (char *) dentry->d_name.name;
+
 	p9_debug(P9_DEBUG_VFS, "dir ino: %lu, old_name: %s, new_name: %s\n",
-		 dir->i_ino, old_dentry->d_name.name, dentry->d_name.name);
+		 dir->i_ino, old_dentry->d_name.name, name);
 
 	v9ses = v9fs_inode2v9ses(dir);
 	dir_dentry = dentry->d_parent;
@@ -785,9 +787,7 @@ v9fs_vfs_link_dotl(struct dentry *old_dentry, struct inode *dir,
 	if (IS_ERR(oldfid))
 		return PTR_ERR(oldfid);
 
-	name = (char *) dentry->d_name.name;
-
-	err = p9_client_link(dfid, oldfid, (char *)dentry->d_name.name);
+	err = p9_client_link(dfid, oldfid, name);
 
 	if (err < 0) {
 		p9_debug(P9_DEBUG_VFS, "p9_client_link failed %d\n", err);
-- 
1.8.4

Re: [PATCH] 9p: proper use of the 'name' variable

[Geyslan Gregório Bem]

2013/10/20 Eric Van Hensbergen <ericvh@gmail.com>:
> I reckon we should probably just get rid of name if its not being used.  I
> doubt the indirection is going to hurt anything here.
>
>      -eric
>

Eric, you're right. Once that there's not assignment to name, the
cycles are the same.
I'll get rid of name var.

>
>
> On Sat, Sep 28, 2013 at 6:32 PM, Geyslan G. Bem <geyslan@gmail.com> wrote:
>>
>> The 'name' variable was assigned but never used. Hence puts its
>> assignment to the top and makes proper use of its value.
>>
>> Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
>> ---
>>  fs/9p/vfs_inode_dotl.c | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/9p/vfs_inode_dotl.c b/fs/9p/vfs_inode_dotl.c
>> index 6803758..86b6944 100644
>> --- a/fs/9p/vfs_inode_dotl.c
>> +++ b/fs/9p/vfs_inode_dotl.c
>> @@ -772,8 +772,10 @@ v9fs_vfs_link_dotl(struct dentry *old_dentry, struct
>> inode *dir,
>>         struct p9_fid *dfid, *oldfid;
>>         struct v9fs_session_info *v9ses;
>>
>> +       name = (char *) dentry->d_name.name;
>> +
>>         p9_debug(P9_DEBUG_VFS, "dir ino: %lu, old_name: %s, new_name:
>> %s\n",
>> -                dir->i_ino, old_dentry->d_name.name,
>> dentry->d_name.name);
>> +                dir->i_ino, old_dentry->d_name.name, name);
>>
>>         v9ses = v9fs_inode2v9ses(dir);
>>         dir_dentry = dentry->d_parent;
>> @@ -785,9 +787,7 @@ v9fs_vfs_link_dotl(struct dentry *old_dentry, struct
>> inode *dir,
>>         if (IS_ERR(oldfid))
>>                 return PTR_ERR(oldfid);
>>
>> -       name = (char *) dentry->d_name.name;
>> -
>> -       err = p9_client_link(dfid, oldfid, (char *)dentry->d_name.name);
>> +       err = p9_client_link(dfid, oldfid, name);
>>
>>         if (err < 0) {
>>                 p9_debug(P9_DEBUG_VFS, "p9_client_link failed %d\n", err);
>> --
>> 1.8.4
>>
>

Re: [PATCH] 9p: proper use of the 'name' variable

[Geyslan Gregório Bem]

2013/10/21 Geyslan Gregório Bem <geyslan@gmail.com>:
> 2013/10/20 Eric Van Hensbergen <ericvh@gmail.com>:
>> I reckon we should probably just get rid of name if its not being used.  I
>> doubt the indirection is going to hurt anything here.
>>
>>      -eric
>>
>
> Eric, you're right. Once that there's not assignment to name, the
> cycles are the same.
> I'll get rid of name var.
>

New patch sent.
[PATCH] 9p: remove useless 'name' variable and assignment

>>
>>
>> On Sat, Sep 28, 2013 at 6:32 PM, Geyslan G. Bem <geyslan@gmail.com> wrote:
>>>
>>> The 'name' variable was assigned but never used. Hence puts its
>>> assignment to the top and makes proper use of its value.
>>>
>>> Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
>>> ---
>>>  fs/9p/vfs_inode_dotl.c | 8 ++++----
>>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/fs/9p/vfs_inode_dotl.c b/fs/9p/vfs_inode_dotl.c
>>> index 6803758..86b6944 100644
>>> --- a/fs/9p/vfs_inode_dotl.c
>>> +++ b/fs/9p/vfs_inode_dotl.c
>>> @@ -772,8 +772,10 @@ v9fs_vfs_link_dotl(struct dentry *old_dentry, struct
>>> inode *dir,
>>>         struct p9_fid *dfid, *oldfid;
>>>         struct v9fs_session_info *v9ses;
>>>
>>> +       name = (char *) dentry->d_name.name;
>>> +
>>>         p9_debug(P9_DEBUG_VFS, "dir ino: %lu, old_name: %s, new_name:
>>> %s\n",
>>> -                dir->i_ino, old_dentry->d_name.name,
>>> dentry->d_name.name);
>>> +                dir->i_ino, old_dentry->d_name.name, name);
>>>
>>>         v9ses = v9fs_inode2v9ses(dir);
>>>         dir_dentry = dentry->d_parent;
>>> @@ -785,9 +787,7 @@ v9fs_vfs_link_dotl(struct dentry *old_dentry, struct
>>> inode *dir,
>>>         if (IS_ERR(oldfid))
>>>                 return PTR_ERR(oldfid);
>>>
>>> -       name = (char *) dentry->d_name.name;
>>> -
>>> -       err = p9_client_link(dfid, oldfid, (char *)dentry->d_name.name);
>>> +       err = p9_client_link(dfid, oldfid, name);
>>>
>>>         if (err < 0) {
>>>                 p9_debug(P9_DEBUG_VFS, "p9_client_link failed %d\n", err);
>>> --
>>> 1.8.4
>>>
>>

[PATCH v3 1/2] binfmt_aout: x86: Useless inode var, printks coding style fixes

[Geyslan G. Bem]

file size used only once, so removed due its useless prior allocation.
It's not necessary to verify f_op in the load_aout_library, since the
prior kernel_read/vfs_read function already does.
Made coding style fixes and printk replacements.

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

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

diff --git a/fs/binfmt_aout.c b/fs/binfmt_aout.c
index 89dec7f..c732b8e 100644
--- a/fs/binfmt_aout.c
+++ b/fs/binfmt_aout.c
@@ -25,13 +25,14 @@
 #include <linux/init.h>
 #include <linux/coredump.h>
 #include <linux/slab.h>
+#include <linux/ratelimit.h>
+#include <linux/uaccess.h>
 
-#include <asm/uaccess.h>
 #include <asm/cacheflush.h>
 #include <asm/a.out-core.h>
 
 static int load_aout_binary(struct linux_binprm *);
-static int load_aout_library(struct file*);
+static int load_aout_library(struct file *);
 
 #ifdef CONFIG_COREDUMP
 /*
@@ -62,7 +63,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);
@@ -78,9 +79,11 @@ static int aout_core_dump(struct coredump_params *cprm)
 
 /* make sure we actually have a data and stack area to dump */
 	set_fs(USER_DS);
-	if (!access_ok(VERIFY_READ, START_DATA(dump), dump.u_dsize << PAGE_SHIFT))
+	if (!access_ok(VERIFY_READ, START_DATA(dump),
+		       dump.u_dsize << PAGE_SHIFT))
 		dump.u_dsize = 0;
-	if (!access_ok(VERIFY_READ, START_STACK(dump), dump.u_ssize << PAGE_SHIFT))
+	if (!access_ok(VERIFY_READ, START_STACK(dump),
+		       dump.u_ssize << PAGE_SHIFT))
 		dump.u_ssize = 0;
 
 	set_fs(KERNEL_DS);
@@ -142,7 +145,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;
@@ -150,7 +154,8 @@ static unsigned long __user *create_aout_tables(char __user *p, struct linux_bin
 	int argc = bprm->argc;
 	int envc = bprm->envc;
 
-	sp = (void __user *)((-(unsigned long)sizeof(char *)) & (unsigned long) p);
+	sp = (void __user *) ((-(unsigned long) sizeof(char *))
+			      & (unsigned long) p);
 #ifdef __alpha__
 /* whee.. test-programs are so much fun. */
 	put_user(0, --sp);
@@ -169,28 +174,28 @@ static unsigned long __user *create_aout_tables(char __user *p, struct linux_bin
 	sp -= argc+1;
 	argv = (char __user * __user *) sp;
 #ifndef __alpha__
-	put_user((unsigned long) envp,--sp);
-	put_user((unsigned long) argv,--sp);
+	put_user((unsigned long) envp, --sp);
+	put_user((unsigned long) argv, --sp);
 #endif
-	put_user(argc,--sp);
+	put_user(argc, --sp);
 	current->mm->arg_start = (unsigned long) p;
-	while (argc-->0) {
+	while (argc-- > 0) {
 		char c;
-		put_user(p,argv++);
+		put_user(p, argv++);
 		do {
-			get_user(c,p++);
+			get_user(c, p++);
 		} while (c);
 	}
-	put_user(NULL,argv);
+	put_user(NULL, argv);
 	current->mm->arg_end = current->mm->env_start = (unsigned long) p;
-	while (envc-->0) {
+	while (envc-- > 0) {
 		char c;
-		put_user(p,envp++);
+		put_user(p, envp++);
 		do {
-			get_user(c,p++);
+			get_user(c, p++);
 		} while (c);
 	}
-	put_user(NULL,envp);
+	put_user(NULL, envp);
 	current->mm->env_end = (unsigned long) p;
 	return sp;
 }
@@ -200,7 +205,7 @@ static unsigned long __user *create_aout_tables(char __user *p, struct linux_bin
  * libraries.  There is no binary dependent code anywhere else.
  */
 
-static int load_aout_binary(struct linux_binprm * bprm)
+static int load_aout_binary(struct linux_binprm *bprm)
 {
 	struct pt_regs *regs = current_pt_regs();
 	struct exec ex;
@@ -213,7 +218,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;
 	}
 
@@ -292,19 +298,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");
-		}
-
-		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);
-		}
+		    (N_MAGIC(ex) != NMAGIC))
+			pr_notice_ratelimited("executable not page aligned\n");
 
-		if (!bprm->file->f_op->mmap||((fd_offset & ~PAGE_MASK) != 0)) {
+		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);
 			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);
@@ -312,9 +311,10 @@ static int load_aout_binary(struct linux_binprm * bprm)
 		}
 
 		error = vm_mmap(bprm->file, N_TXTADDR(ex), ex.a_text,
-			PROT_READ | PROT_EXEC,
-			MAP_FIXED | MAP_PRIVATE | MAP_DENYWRITE | MAP_EXECUTABLE,
-			fd_offset);
+				PROT_READ | PROT_EXEC,
+				(MAP_FIXED | MAP_PRIVATE
+				 | MAP_DENYWRITE | MAP_EXECUTABLE),
+				fd_offset);
 
 		if (error != N_TXTADDR(ex)) {
 			send_sig(SIGKILL, current, 0);
@@ -323,8 +323,10 @@ static int load_aout_binary(struct linux_binprm * bprm)
 
 		error = vm_mmap(bprm->file, N_DATADDR(ex), ex.a_data,
 				PROT_READ | PROT_WRITE | PROT_EXEC,
-				MAP_FIXED | MAP_PRIVATE | MAP_DENYWRITE | MAP_EXECUTABLE,
+				(MAP_FIXED | MAP_PRIVATE
+				 | MAP_DENYWRITE | MAP_EXECUTABLE),
 				fd_offset + ex.a_text);
+
 		if (error != N_DATADDR(ex)) {
 			send_sig(SIGKILL, current, 0);
 			return error;
@@ -340,7 +342,8 @@ beyond_if:
 	}
 
 	current->mm->start_stack =
-		(unsigned long) create_aout_tables((char __user *) bprm->p, bprm);
+		(unsigned long) create_aout_tables((char __user *) bprm->p,
+						   bprm);
 #ifdef __alpha__
 	regs->gp = ex.a_gpvalue;
 #endif
@@ -350,14 +353,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 +366,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 +375,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))
@@ -383,17 +384,14 @@ static int load_aout_library(struct file *file)
 	/* For  QMAGIC, the starting address is 0x20 into the page.  We mask
 	   this off to get the starting address for the page */
 
-	start_addr =  ex.a_entry & 0xfffff000;
+	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),
 			  ex.a_text + ex.a_data);
 		retval = 0;
-- 
1.8.4

Re: [PATCH v3 1/2] binfmt_aout: x86: Useless inode var, printks coding style fixes

[Geyslan Gregório Bem]

Please, disconsider this e-mail.

Geyslan Gregório Bem
hackingbits.com


2013/9/28 Geyslan G. Bem <geyslan@gmail.com>:
> file size used only once, so removed due its useless prior allocation.
> It's not necessary to verify f_op in the load_aout_library, since the
> prior kernel_read/vfs_read function already does.
> Made coding style fixes and printk replacements.
>
> Tested using qemu, a handcrafted a.out binary and an a.out linked with a
> cross-compiled ld.
>
> Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
> ---
>  fs/binfmt_aout.c | 98 +++++++++++++++++++++++++++-----------------------------
>  1 file changed, 48 insertions(+), 50 deletions(-)
>
> diff --git a/fs/binfmt_aout.c b/fs/binfmt_aout.c
> index 89dec7f..c732b8e 100644
> --- a/fs/binfmt_aout.c
> +++ b/fs/binfmt_aout.c
> @@ -25,13 +25,14 @@
>  #include <linux/init.h>
>  #include <linux/coredump.h>
>  #include <linux/slab.h>
> +#include <linux/ratelimit.h>
> +#include <linux/uaccess.h>
>
> -#include <asm/uaccess.h>
>  #include <asm/cacheflush.h>
>  #include <asm/a.out-core.h>
>
>  static int load_aout_binary(struct linux_binprm *);
> -static int load_aout_library(struct file*);
> +static int load_aout_library(struct file *);
>
>  #ifdef CONFIG_COREDUMP
>  /*
> @@ -62,7 +63,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);
> @@ -78,9 +79,11 @@ static int aout_core_dump(struct coredump_params *cprm)
>
>  /* make sure we actually have a data and stack area to dump */
>         set_fs(USER_DS);
> -       if (!access_ok(VERIFY_READ, START_DATA(dump), dump.u_dsize << PAGE_SHIFT))
> +       if (!access_ok(VERIFY_READ, START_DATA(dump),
> +                      dump.u_dsize << PAGE_SHIFT))
>                 dump.u_dsize = 0;
> -       if (!access_ok(VERIFY_READ, START_STACK(dump), dump.u_ssize << PAGE_SHIFT))
> +       if (!access_ok(VERIFY_READ, START_STACK(dump),
> +                      dump.u_ssize << PAGE_SHIFT))
>                 dump.u_ssize = 0;
>
>         set_fs(KERNEL_DS);
> @@ -142,7 +145,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;
> @@ -150,7 +154,8 @@ static unsigned long __user *create_aout_tables(char __user *p, struct linux_bin
>         int argc = bprm->argc;
>         int envc = bprm->envc;
>
> -       sp = (void __user *)((-(unsigned long)sizeof(char *)) & (unsigned long) p);
> +       sp = (void __user *) ((-(unsigned long) sizeof(char *))
> +                             & (unsigned long) p);
>  #ifdef __alpha__
>  /* whee.. test-programs are so much fun. */
>         put_user(0, --sp);
> @@ -169,28 +174,28 @@ static unsigned long __user *create_aout_tables(char __user *p, struct linux_bin
>         sp -= argc+1;
>         argv = (char __user * __user *) sp;
>  #ifndef __alpha__
> -       put_user((unsigned long) envp,--sp);
> -       put_user((unsigned long) argv,--sp);
> +       put_user((unsigned long) envp, --sp);
> +       put_user((unsigned long) argv, --sp);
>  #endif
> -       put_user(argc,--sp);
> +       put_user(argc, --sp);
>         current->mm->arg_start = (unsigned long) p;
> -       while (argc-->0) {
> +       while (argc-- > 0) {
>                 char c;
> -               put_user(p,argv++);
> +               put_user(p, argv++);
>                 do {
> -                       get_user(c,p++);
> +                       get_user(c, p++);
>                 } while (c);
>         }
> -       put_user(NULL,argv);
> +       put_user(NULL, argv);
>         current->mm->arg_end = current->mm->env_start = (unsigned long) p;
> -       while (envc-->0) {
> +       while (envc-- > 0) {
>                 char c;
> -               put_user(p,envp++);
> +               put_user(p, envp++);
>                 do {
> -                       get_user(c,p++);
> +                       get_user(c, p++);
>                 } while (c);
>         }
> -       put_user(NULL,envp);
> +       put_user(NULL, envp);
>         current->mm->env_end = (unsigned long) p;
>         return sp;
>  }
> @@ -200,7 +205,7 @@ static unsigned long __user *create_aout_tables(char __user *p, struct linux_bin
>   * libraries.  There is no binary dependent code anywhere else.
>   */
>
> -static int load_aout_binary(struct linux_binprm * bprm)
> +static int load_aout_binary(struct linux_binprm *bprm)
>  {
>         struct pt_regs *regs = current_pt_regs();
>         struct exec ex;
> @@ -213,7 +218,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;
>         }
>
> @@ -292,19 +298,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");
> -               }
> -
> -               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);
> -               }
> +                   (N_MAGIC(ex) != NMAGIC))
> +                       pr_notice_ratelimited("executable not page aligned\n");
>
> -               if (!bprm->file->f_op->mmap||((fd_offset & ~PAGE_MASK) != 0)) {
> +               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);
>                         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);
> @@ -312,9 +311,10 @@ static int load_aout_binary(struct linux_binprm * bprm)
>                 }
>
>                 error = vm_mmap(bprm->file, N_TXTADDR(ex), ex.a_text,
> -                       PROT_READ | PROT_EXEC,
> -                       MAP_FIXED | MAP_PRIVATE | MAP_DENYWRITE | MAP_EXECUTABLE,
> -                       fd_offset);
> +                               PROT_READ | PROT_EXEC,
> +                               (MAP_FIXED | MAP_PRIVATE
> +                                | MAP_DENYWRITE | MAP_EXECUTABLE),
> +                               fd_offset);
>
>                 if (error != N_TXTADDR(ex)) {
>                         send_sig(SIGKILL, current, 0);
> @@ -323,8 +323,10 @@ static int load_aout_binary(struct linux_binprm * bprm)
>
>                 error = vm_mmap(bprm->file, N_DATADDR(ex), ex.a_data,
>                                 PROT_READ | PROT_WRITE | PROT_EXEC,
> -                               MAP_FIXED | MAP_PRIVATE | MAP_DENYWRITE | MAP_EXECUTABLE,
> +                               (MAP_FIXED | MAP_PRIVATE
> +                                | MAP_DENYWRITE | MAP_EXECUTABLE),
>                                 fd_offset + ex.a_text);
> +
>                 if (error != N_DATADDR(ex)) {
>                         send_sig(SIGKILL, current, 0);
>                         return error;
> @@ -340,7 +342,8 @@ beyond_if:
>         }
>
>         current->mm->start_stack =
> -               (unsigned long) create_aout_tables((char __user *) bprm->p, bprm);
> +               (unsigned long) create_aout_tables((char __user *) bprm->p,
> +                                                  bprm);
>  #ifdef __alpha__
>         regs->gp = ex.a_gpvalue;
>  #endif
> @@ -350,14 +353,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 +366,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 +375,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))
> @@ -383,17 +384,14 @@ static int load_aout_library(struct file *file)
>         /* For  QMAGIC, the starting address is 0x20 into the page.  We mask
>            this off to get the starting address for the page */
>
> -       start_addr =  ex.a_entry & 0xfffff000;
> +       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),
>                           ex.a_text + ex.a_data);
>                 retval = 0;
> --
> 1.8.4
>

[PATCH 1/1] fs/binfmt_elf.c: fill_note_info: Reduce scope of a variable

[Geyslan G. Bem]

The *t variable in the fill_note_info function is only used if
siginfo->si_signo isn't 0. Moving "t" pointer to that inner scope.

Tested.

Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
---
 fs/binfmt_elf.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 100edcc..d0ac15b 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -1848,12 +1848,11 @@ static int fill_note_info(struct elfhdr *elf, int phdrs,
 			  struct elf_note_info *info,
 			  siginfo_t *siginfo, struct pt_regs *regs)
 {
-	struct list_head *t;
-
 	if (!elf_note_info_init(info))
 		return 0;
 
 	if (siginfo->si_signo) {
+		struct list_head *t;
 		struct core_thread *ct;
 		struct elf_thread_status *ets;
 
-- 
1.8.4

Re: [PATCH 1/1] fs/binfmt_elf.c: fill_note_info: Reduce scope of a variable

[Geyslan Gregório Bem]

Please, disconsider this e-mail.

Geyslan Gregório Bem
hackingbits.com


2013/9/28 Geyslan G. Bem <geyslan@gmail.com>:
> The *t variable in the fill_note_info function is only used if
> siginfo->si_signo isn't 0. Moving "t" pointer to that inner scope.
>
> Tested.
>
> Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
> ---
>  fs/binfmt_elf.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
> index 100edcc..d0ac15b 100644
> --- a/fs/binfmt_elf.c
> +++ b/fs/binfmt_elf.c
> @@ -1848,12 +1848,11 @@ static int fill_note_info(struct elfhdr *elf, int phdrs,
>                           struct elf_note_info *info,
>                           siginfo_t *siginfo, struct pt_regs *regs)
>  {
> -       struct list_head *t;
> -
>         if (!elf_note_info_init(info))
>                 return 0;
>
>         if (siginfo->si_signo) {
> +               struct list_head *t;
>                 struct core_thread *ct;
>                 struct elf_thread_status *ets;
>
> --
> 1.8.4
>

[PATCH] fs: exec.c: Coding style sanitization

[Geyslan G. Bem]

Adjustment based on the checkpatch.pl.

Tested.

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

diff --git a/fs/exec.c b/fs/exec.c
index 8875dd1..b5c6086 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>
@@ -55,8 +55,8 @@
 #include <linux/pipe_fs_i.h>
 #include <linux/oom.h>
 #include <linux/compat.h>
+#include <linux/uaccess.h>
 
-#include <asm/uaccess.h>
 #include <asm/mmu_context.h>
 #include <asm/tlb.h>
 
@@ -71,7 +71,7 @@ int suid_dumpable = 0;
 static LIST_HEAD(formats);
 static DEFINE_RWLOCK(binfmt_lock);
 
-void __register_binfmt(struct linux_binfmt * fmt, int insert)
+void __register_binfmt(struct linux_binfmt *fmt, int insert)
 {
 	BUG_ON(!fmt);
 	if (WARN_ON(!fmt->load_binary))
@@ -81,19 +81,17 @@ void __register_binfmt(struct linux_binfmt * fmt, int insert)
 		 list_add_tail(&fmt->lh, &formats);
 	write_unlock(&binfmt_lock);
 }
-
 EXPORT_SYMBOL(__register_binfmt);
 
-void unregister_binfmt(struct linux_binfmt * fmt)
+void unregister_binfmt(struct linux_binfmt *fmt)
 {
 	write_lock(&binfmt_lock);
 	list_del(&fmt->lh);
 	write_unlock(&binfmt_lock);
 }
-
 EXPORT_SYMBOL(unregister_binfmt);
 
-static inline void put_binfmt(struct linux_binfmt * fmt)
+static inline void put_binfmt(struct linux_binfmt *fmt)
 {
 	module_put(fmt->module);
 }
@@ -136,8 +134,8 @@ SYSCALL_DEFINE1(uselib, const char __user *, library)
 	fsnotify_open(file);
 
 	error = -ENOEXEC;
-	if(file->f_op) {
-		struct linux_binfmt * fmt;
+	if (file->f_op) {
+		struct linux_binfmt *fmt;
 
 		read_lock(&binfmt_lock);
 		list_for_each_entry(fmt, &formats, lh) {
@@ -154,10 +152,11 @@ SYSCALL_DEFINE1(uselib, const char __user *, library)
 		}
 		read_unlock(&binfmt_lock);
 	}
+
 exit:
 	fput(file);
 out:
-  	return error;
+	return error;
 }
 
 #ifdef CONFIG_MMU
@@ -180,7 +179,7 @@ static void acct_arg_size(struct linux_binprm *bprm, unsigned long pages)
 }
 
 static struct page *get_arg_page(struct linux_binprm *bprm, unsigned long pos,
-		int write)
+				 int write)
 {
 	struct page *page;
 	int ret;
@@ -241,7 +240,7 @@ static void free_arg_pages(struct linux_binprm *bprm)
 }
 
 static void flush_arg_page(struct linux_binprm *bprm, unsigned long pos,
-		struct page *page)
+			   struct page *page)
 {
 	flush_cache_page(bprm->vma, pos, page_to_pfn(page));
 }
@@ -268,7 +267,9 @@ static int __bprm_mm_init(struct linux_binprm *bprm)
 	BUILD_BUG_ON(VM_STACK_FLAGS & VM_STACK_INCOMPLETE_SETUP);
 	vma->vm_end = STACK_TOP_MAX;
 	vma->vm_start = vma->vm_end - PAGE_SIZE;
-	vma->vm_flags = VM_SOFTDIRTY | VM_STACK_FLAGS | VM_STACK_INCOMPLETE_SETUP;
+	vma->vm_flags = (VM_SOFTDIRTY |
+			 VM_STACK_FLAGS |
+			 VM_STACK_INCOMPLETE_SETUP);
 	vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
 	INIT_LIST_HEAD(&vma->anon_vma_chain);
 
@@ -299,7 +300,7 @@ static inline void acct_arg_size(struct linux_binprm *bprm, unsigned long pages)
 }
 
 static struct page *get_arg_page(struct linux_binprm *bprm, unsigned long pos,
-		int write)
+				 int write)
 {
 	struct page *page;
 
@@ -335,7 +336,7 @@ static void free_arg_pages(struct linux_binprm *bprm)
 }
 
 static void flush_arg_page(struct linux_binprm *bprm, unsigned long pos,
-		struct page *page)
+			   struct page *page)
 {
 }
 
@@ -616,7 +617,9 @@ static int shift_arg_pages(struct vm_area_struct *vma, unsigned long shift)
 		 * when the old and new regions overlap clear from new_end.
 		 */
 		free_pgd_range(&tlb, new_end, old_end, new_end,
-			vma->vm_next ? vma->vm_next->vm_start : USER_PGTABLES_CEILING);
+			       vma->vm_next
+			       ? vma->vm_next->vm_start
+			       : USER_PGTABLES_CEILING);
 	} else {
 		/*
 		 * otherwise, clean from old_start; this is done to not touch
@@ -625,7 +628,9 @@ static int shift_arg_pages(struct vm_area_struct *vma, unsigned long shift)
 		 * for the others its just a little faster.
 		 */
 		free_pgd_range(&tlb, old_start, old_end, new_end,
-			vma->vm_next ? vma->vm_next->vm_start : USER_PGTABLES_CEILING);
+			       vma->vm_next
+			       ? vma->vm_next->vm_start
+			       : USER_PGTABLES_CEILING);
 	}
 	tlb_finish_mmu(&tlb, old_start, old_end);
 
@@ -803,7 +808,6 @@ int kernel_read(struct file *file, loff_t offset,
 	set_fs(old_fs);
 	return result;
 }
-
 EXPORT_SYMBOL(kernel_read);
 
 ssize_t read_code(struct file *file, unsigned long addr, loff_t pos, size_t len)
@@ -818,7 +822,7 @@ EXPORT_SYMBOL(read_code);
 static int exec_mmap(struct mm_struct *mm)
 {
 	struct task_struct *tsk;
-	struct mm_struct * old_mm, *active_mm;
+	struct mm_struct *old_mm, *active_mm;
 
 	/* Notify parent that we're no longer interested in the old VM */
 	tsk = current;
@@ -1064,7 +1068,7 @@ static void filename_to_taskname(char *tcomm, const char *fn, unsigned int len)
 	tcomm[i] = '\0';
 }
 
-int flush_old_exec(struct linux_binprm * bprm)
+int flush_old_exec(struct linux_binprm *bprm)
 {
 	int retval;
 
@@ -1109,14 +1113,15 @@ void would_dump(struct linux_binprm *bprm, struct file *file)
 }
 EXPORT_SYMBOL(would_dump);
 
-void setup_new_exec(struct linux_binprm * bprm)
+void setup_new_exec(struct linux_binprm *bprm)
 {
 	arch_pick_mmap_layout(current->mm);
 
 	/* This is the point of no return */
 	current->sas_ss_sp = current->sas_ss_size = 0;
 
-	if (uid_eq(current_euid(), current_uid()) && gid_eq(current_egid(), current_gid()))
+	if (uid_eq(current_euid(), current_uid()) &&
+	    gid_eq(current_egid(), current_gid()))
 		set_dumpable(current->mm, SUID_DUMP_USER);
 	else
 		set_dumpable(current->mm, suid_dumpable);
@@ -1143,7 +1148,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);
 }
@@ -1269,8 +1274,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).
@@ -1278,7 +1283,7 @@ static int check_unsafe_exec(struct linux_binprm *bprm)
 int prepare_binprm(struct linux_binprm *bprm)
 {
 	umode_t mode;
-	struct inode * inode = file_inode(bprm->file);
+	struct inode *inode = file_inode(bprm->file);
 	int retval;
 
 	mode = inode->i_mode;
@@ -1320,7 +1325,6 @@ int prepare_binprm(struct linux_binprm *bprm)
 	memset(bprm->buf, 0, BINPRM_BUF_SIZE);
 	return kernel_read(bprm->file, 0, bprm->buf, BINPRM_BUF_SIZE);
 }
-
 EXPORT_SYMBOL(prepare_binprm);
 
 /*
@@ -1367,7 +1371,9 @@ out:
 }
 EXPORT_SYMBOL(remove_arg_zero);
 
-#define printable(c) (((c)=='\t') || ((c)=='\n') || (0x20<=(c) && (c)<=0x7e))
+#define printable(c) (((c) == '\t') ||			\
+		      ((c) == '\n') ||			\
+		      (0x20 <= (c) && (c) <= 0x7e))
 /*
  * cycle the list of binary formats handler, until one recognizes the image
  */
@@ -1413,7 +1419,8 @@ int search_binary_handler(struct linux_binprm *bprm)
 		if (printable(bprm->buf[0]) && printable(bprm->buf[1]) &&
 		    printable(bprm->buf[2]) && printable(bprm->buf[3]))
 			return retval;
-		if (request_module("binfmt-%04x", *(ushort *)(bprm->buf + 2)) < 0)
+		if (request_module("binfmt-%04x",
+				   *(ushort *)(bprm->buf + 2)) < 0)
 			return retval;
 		need_retry = false;
 		goto retry;
@@ -1455,8 +1462,8 @@ static int exec_binprm(struct linux_binprm *bprm)
  * sys_execve() executes a new program.
  */
 static int do_execve_common(const char *filename,
-				struct user_arg_ptr argv,
-				struct user_arg_ptr envp)
+			    struct user_arg_ptr argv,
+			    struct user_arg_ptr envp)
 {
 	struct linux_binprm *bprm;
 	struct file *file;
@@ -1514,12 +1521,12 @@ static int do_execve_common(const char *filename,
 	if (retval)
 		goto out_file;
 
-	bprm->argc = count(argv, MAX_ARG_STRINGS);
-	if ((retval = bprm->argc) < 0)
+	retval = bprm->argc = count(argv, MAX_ARG_STRINGS);
+	if (retval < 0)
 		goto out;
 
-	bprm->envc = count(envp, MAX_ARG_STRINGS);
-	if ((retval = bprm->envc) < 0)
+	retval = bprm->envc = count(envp, MAX_ARG_STRINGS);
+	if (retval < 0)
 		goto out;
 
 	retval = prepare_binprm(bprm);
@@ -1580,8 +1587,8 @@ out_ret:
 }
 
 int do_execve(const char *filename,
-	const char __user *const __user *__argv,
-	const char __user *const __user *__envp)
+	      const char __user *const __user *__argv,
+	      const char __user *const __user *__envp)
 {
 	struct user_arg_ptr argv = { .ptr.native = __argv };
 	struct user_arg_ptr envp = { .ptr.native = __envp };
@@ -1590,8 +1597,8 @@ int do_execve(const char *filename,
 
 #ifdef CONFIG_COMPAT
 static int compat_do_execve(const char *filename,
-	const compat_uptr_t __user *__argv,
-	const compat_uptr_t __user *__envp)
+			    const compat_uptr_t __user *__argv,
+			    const compat_uptr_t __user *__envp)
 {
 	struct user_arg_ptr argv = {
 		.is_compat = true,
@@ -1616,7 +1623,6 @@ void set_binfmt(struct linux_binfmt *new)
 	if (new)
 		__module_get(new->module);
 }
-
 EXPORT_SYMBOL(set_binfmt);
 
 /*
@@ -1687,9 +1693,9 @@ SYSCALL_DEFINE3(execve,
 	return error;
 }
 #ifdef CONFIG_COMPAT
-asmlinkage long compat_sys_execve(const char __user * filename,
-	const compat_uptr_t __user * argv,
-	const compat_uptr_t __user * envp)
+asmlinkage long compat_sys_execve(const char __user *filename,
+				  const compat_uptr_t __user *argv,
+				  const compat_uptr_t __user *envp)
 {
 	struct filename *path = getname(filename);
 	int error = PTR_ERR(path);
-- 
1.8.4

Re: [PATCH] fs: exec.c: Coding style sanitization

[Geyslan Gregório Bem]

Please, disconsider this e-mail.

Geyslan Gregório Bem
hackingbits.com


2013/9/28 Geyslan G. Bem <geyslan@gmail.com>:
> Adjustment based on the checkpatch.pl.
>
> Tested.
>
> Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
> ---
>  fs/exec.c | 92 ++++++++++++++++++++++++++++++++++-----------------------------
>  1 file changed, 49 insertions(+), 43 deletions(-)
>
> diff --git a/fs/exec.c b/fs/exec.c
> index 8875dd1..b5c6086 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>
> @@ -55,8 +55,8 @@
>  #include <linux/pipe_fs_i.h>
>  #include <linux/oom.h>
>  #include <linux/compat.h>
> +#include <linux/uaccess.h>
>
> -#include <asm/uaccess.h>
>  #include <asm/mmu_context.h>
>  #include <asm/tlb.h>
>
> @@ -71,7 +71,7 @@ int suid_dumpable = 0;
>  static LIST_HEAD(formats);
>  static DEFINE_RWLOCK(binfmt_lock);
>
> -void __register_binfmt(struct linux_binfmt * fmt, int insert)
> +void __register_binfmt(struct linux_binfmt *fmt, int insert)
>  {
>         BUG_ON(!fmt);
>         if (WARN_ON(!fmt->load_binary))
> @@ -81,19 +81,17 @@ void __register_binfmt(struct linux_binfmt * fmt, int insert)
>                  list_add_tail(&fmt->lh, &formats);
>         write_unlock(&binfmt_lock);
>  }
> -
>  EXPORT_SYMBOL(__register_binfmt);
>
> -void unregister_binfmt(struct linux_binfmt * fmt)
> +void unregister_binfmt(struct linux_binfmt *fmt)
>  {
>         write_lock(&binfmt_lock);
>         list_del(&fmt->lh);
>         write_unlock(&binfmt_lock);
>  }
> -
>  EXPORT_SYMBOL(unregister_binfmt);
>
> -static inline void put_binfmt(struct linux_binfmt * fmt)
> +static inline void put_binfmt(struct linux_binfmt *fmt)
>  {
>         module_put(fmt->module);
>  }
> @@ -136,8 +134,8 @@ SYSCALL_DEFINE1(uselib, const char __user *, library)
>         fsnotify_open(file);
>
>         error = -ENOEXEC;
> -       if(file->f_op) {
> -               struct linux_binfmt * fmt;
> +       if (file->f_op) {
> +               struct linux_binfmt *fmt;
>
>                 read_lock(&binfmt_lock);
>                 list_for_each_entry(fmt, &formats, lh) {
> @@ -154,10 +152,11 @@ SYSCALL_DEFINE1(uselib, const char __user *, library)
>                 }
>                 read_unlock(&binfmt_lock);
>         }
> +
>  exit:
>         fput(file);
>  out:
> -       return error;
> +       return error;
>  }
>
>  #ifdef CONFIG_MMU
> @@ -180,7 +179,7 @@ static void acct_arg_size(struct linux_binprm *bprm, unsigned long pages)
>  }
>
>  static struct page *get_arg_page(struct linux_binprm *bprm, unsigned long pos,
> -               int write)
> +                                int write)
>  {
>         struct page *page;
>         int ret;
> @@ -241,7 +240,7 @@ static void free_arg_pages(struct linux_binprm *bprm)
>  }
>
>  static void flush_arg_page(struct linux_binprm *bprm, unsigned long pos,
> -               struct page *page)
> +                          struct page *page)
>  {
>         flush_cache_page(bprm->vma, pos, page_to_pfn(page));
>  }
> @@ -268,7 +267,9 @@ static int __bprm_mm_init(struct linux_binprm *bprm)
>         BUILD_BUG_ON(VM_STACK_FLAGS & VM_STACK_INCOMPLETE_SETUP);
>         vma->vm_end = STACK_TOP_MAX;
>         vma->vm_start = vma->vm_end - PAGE_SIZE;
> -       vma->vm_flags = VM_SOFTDIRTY | VM_STACK_FLAGS | VM_STACK_INCOMPLETE_SETUP;
> +       vma->vm_flags = (VM_SOFTDIRTY |
> +                        VM_STACK_FLAGS |
> +                        VM_STACK_INCOMPLETE_SETUP);
>         vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
>         INIT_LIST_HEAD(&vma->anon_vma_chain);
>
> @@ -299,7 +300,7 @@ static inline void acct_arg_size(struct linux_binprm *bprm, unsigned long pages)
>  }
>
>  static struct page *get_arg_page(struct linux_binprm *bprm, unsigned long pos,
> -               int write)
> +                                int write)
>  {
>         struct page *page;
>
> @@ -335,7 +336,7 @@ static void free_arg_pages(struct linux_binprm *bprm)
>  }
>
>  static void flush_arg_page(struct linux_binprm *bprm, unsigned long pos,
> -               struct page *page)
> +                          struct page *page)
>  {
>  }
>
> @@ -616,7 +617,9 @@ static int shift_arg_pages(struct vm_area_struct *vma, unsigned long shift)
>                  * when the old and new regions overlap clear from new_end.
>                  */
>                 free_pgd_range(&tlb, new_end, old_end, new_end,
> -                       vma->vm_next ? vma->vm_next->vm_start : USER_PGTABLES_CEILING);
> +                              vma->vm_next
> +                              ? vma->vm_next->vm_start
> +                              : USER_PGTABLES_CEILING);
>         } else {
>                 /*
>                  * otherwise, clean from old_start; this is done to not touch
> @@ -625,7 +628,9 @@ static int shift_arg_pages(struct vm_area_struct *vma, unsigned long shift)
>                  * for the others its just a little faster.
>                  */
>                 free_pgd_range(&tlb, old_start, old_end, new_end,
> -                       vma->vm_next ? vma->vm_next->vm_start : USER_PGTABLES_CEILING);
> +                              vma->vm_next
> +                              ? vma->vm_next->vm_start
> +                              : USER_PGTABLES_CEILING);
>         }
>         tlb_finish_mmu(&tlb, old_start, old_end);
>
> @@ -803,7 +808,6 @@ int kernel_read(struct file *file, loff_t offset,
>         set_fs(old_fs);
>         return result;
>  }
> -
>  EXPORT_SYMBOL(kernel_read);
>
>  ssize_t read_code(struct file *file, unsigned long addr, loff_t pos, size_t len)
> @@ -818,7 +822,7 @@ EXPORT_SYMBOL(read_code);
>  static int exec_mmap(struct mm_struct *mm)
>  {
>         struct task_struct *tsk;
> -       struct mm_struct * old_mm, *active_mm;
> +       struct mm_struct *old_mm, *active_mm;
>
>         /* Notify parent that we're no longer interested in the old VM */
>         tsk = current;
> @@ -1064,7 +1068,7 @@ static void filename_to_taskname(char *tcomm, const char *fn, unsigned int len)
>         tcomm[i] = '\0';
>  }
>
> -int flush_old_exec(struct linux_binprm * bprm)
> +int flush_old_exec(struct linux_binprm *bprm)
>  {
>         int retval;
>
> @@ -1109,14 +1113,15 @@ void would_dump(struct linux_binprm *bprm, struct file *file)
>  }
>  EXPORT_SYMBOL(would_dump);
>
> -void setup_new_exec(struct linux_binprm * bprm)
> +void setup_new_exec(struct linux_binprm *bprm)
>  {
>         arch_pick_mmap_layout(current->mm);
>
>         /* This is the point of no return */
>         current->sas_ss_sp = current->sas_ss_size = 0;
>
> -       if (uid_eq(current_euid(), current_uid()) && gid_eq(current_egid(), current_gid()))
> +       if (uid_eq(current_euid(), current_uid()) &&
> +           gid_eq(current_egid(), current_gid()))
>                 set_dumpable(current->mm, SUID_DUMP_USER);
>         else
>                 set_dumpable(current->mm, suid_dumpable);
> @@ -1143,7 +1148,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);
>  }
> @@ -1269,8 +1274,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).
> @@ -1278,7 +1283,7 @@ static int check_unsafe_exec(struct linux_binprm *bprm)
>  int prepare_binprm(struct linux_binprm *bprm)
>  {
>         umode_t mode;
> -       struct inode * inode = file_inode(bprm->file);
> +       struct inode *inode = file_inode(bprm->file);
>         int retval;
>
>         mode = inode->i_mode;
> @@ -1320,7 +1325,6 @@ int prepare_binprm(struct linux_binprm *bprm)
>         memset(bprm->buf, 0, BINPRM_BUF_SIZE);
>         return kernel_read(bprm->file, 0, bprm->buf, BINPRM_BUF_SIZE);
>  }
> -
>  EXPORT_SYMBOL(prepare_binprm);
>
>  /*
> @@ -1367,7 +1371,9 @@ out:
>  }
>  EXPORT_SYMBOL(remove_arg_zero);
>
> -#define printable(c) (((c)=='\t') || ((c)=='\n') || (0x20<=(c) && (c)<=0x7e))
> +#define printable(c) (((c) == '\t') ||                 \
> +                     ((c) == '\n') ||                  \
> +                     (0x20 <= (c) && (c) <= 0x7e))
>  /*
>   * cycle the list of binary formats handler, until one recognizes the image
>   */
> @@ -1413,7 +1419,8 @@ int search_binary_handler(struct linux_binprm *bprm)
>                 if (printable(bprm->buf[0]) && printable(bprm->buf[1]) &&
>                     printable(bprm->buf[2]) && printable(bprm->buf[3]))
>                         return retval;
> -               if (request_module("binfmt-%04x", *(ushort *)(bprm->buf + 2)) < 0)
> +               if (request_module("binfmt-%04x",
> +                                  *(ushort *)(bprm->buf + 2)) < 0)
>                         return retval;
>                 need_retry = false;
>                 goto retry;
> @@ -1455,8 +1462,8 @@ static int exec_binprm(struct linux_binprm *bprm)
>   * sys_execve() executes a new program.
>   */
>  static int do_execve_common(const char *filename,
> -                               struct user_arg_ptr argv,
> -                               struct user_arg_ptr envp)
> +                           struct user_arg_ptr argv,
> +                           struct user_arg_ptr envp)
>  {
>         struct linux_binprm *bprm;
>         struct file *file;
> @@ -1514,12 +1521,12 @@ static int do_execve_common(const char *filename,
>         if (retval)
>                 goto out_file;
>
> -       bprm->argc = count(argv, MAX_ARG_STRINGS);
> -       if ((retval = bprm->argc) < 0)
> +       retval = bprm->argc = count(argv, MAX_ARG_STRINGS);
> +       if (retval < 0)
>                 goto out;
>
> -       bprm->envc = count(envp, MAX_ARG_STRINGS);
> -       if ((retval = bprm->envc) < 0)
> +       retval = bprm->envc = count(envp, MAX_ARG_STRINGS);
> +       if (retval < 0)
>                 goto out;
>
>         retval = prepare_binprm(bprm);
> @@ -1580,8 +1587,8 @@ out_ret:
>  }
>
>  int do_execve(const char *filename,
> -       const char __user *const __user *__argv,
> -       const char __user *const __user *__envp)
> +             const char __user *const __user *__argv,
> +             const char __user *const __user *__envp)
>  {
>         struct user_arg_ptr argv = { .ptr.native = __argv };
>         struct user_arg_ptr envp = { .ptr.native = __envp };
> @@ -1590,8 +1597,8 @@ int do_execve(const char *filename,
>
>  #ifdef CONFIG_COMPAT
>  static int compat_do_execve(const char *filename,
> -       const compat_uptr_t __user *__argv,
> -       const compat_uptr_t __user *__envp)
> +                           const compat_uptr_t __user *__argv,
> +                           const compat_uptr_t __user *__envp)
>  {
>         struct user_arg_ptr argv = {
>                 .is_compat = true,
> @@ -1616,7 +1623,6 @@ void set_binfmt(struct linux_binfmt *new)
>         if (new)
>                 __module_get(new->module);
>  }
> -
>  EXPORT_SYMBOL(set_binfmt);
>
>  /*
> @@ -1687,9 +1693,9 @@ SYSCALL_DEFINE3(execve,
>         return error;
>  }
>  #ifdef CONFIG_COMPAT
> -asmlinkage long compat_sys_execve(const char __user * filename,
> -       const compat_uptr_t __user * argv,
> -       const compat_uptr_t __user * envp)
> +asmlinkage long compat_sys_execve(const char __user *filename,
> +                                 const compat_uptr_t __user *argv,
> +                                 const compat_uptr_t __user *envp)
>  {
>         struct filename *path = getname(filename);
>         int error = PTR_ERR(path);
> --
> 1.8.4
>

[PATCH v3 2/2] ia32_aout: x86_64: Add safe check in a.out loaders, printks, conding style fixes

[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.
Made coding style fixes and printks replacements.

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

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

diff --git a/arch/x86/ia32/ia32_aout.c b/arch/x86/ia32/ia32_aout.c
index bae3aba..87d5114 100644
--- a/arch/x86/ia32/ia32_aout.c
+++ b/arch/x86/ia32/ia32_aout.c
@@ -24,9 +24,9 @@
 #include <linux/binfmts.h>
 #include <linux/personality.h>
 #include <linux/init.h>
-#include <linux/jiffies.h>
+#include <linux/ratelimit.h>
+#include <linux/uaccess.h>
 
-#include <asm/uaccess.h>
 #include <asm/pgalloc.h>
 #include <asm/cacheflush.h>
 #include <asm/user32.h>
@@ -224,9 +224,9 @@ static u32 __user *create_aout_tables(char __user *p, struct linux_binprm *bprm)
 	int argc = bprm->argc, envc = bprm->envc;
 
 	sp = (u32 __user *) ((-(unsigned long)sizeof(u32)) & (unsigned long) p);
-	sp -= envc+1;
+	sp -= envc + 1;
 	envp = sp;
-	sp -= argc+1;
+	sp -= argc + 1;
 	argv = sp;
 	put_user((unsigned long) envp, --sp);
 	put_user((unsigned long) argv, --sp);
@@ -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
@@ -322,7 +329,7 @@ static int load_aout_binary(struct linux_binprm *bprm)
 		unsigned long text_addr, map_size;
 
 		text_addr = N_TXTADDR(ex);
-		map_size = ex.a_text+ex.a_data;
+		map_size = ex.a_text + ex.a_data;
 
 		error = vm_brk(text_addr & PAGE_MASK, map_size);
 
@@ -339,28 +346,19 @@ static int load_aout_binary(struct linux_binprm *bprm)
 		}
 	} else {
 #ifdef WARN_OLD
-		static unsigned long error_time, error_time2;
 		if ((ex.a_text & 0xfff || ex.a_data & 0xfff) &&
-		    (N_MAGIC(ex) != NMAGIC) &&
-				time_after(jiffies, error_time2 + 5*HZ)) {
-			printk(KERN_NOTICE "executable not page aligned\n");
-			error_time2 = jiffies;
-		}
+		    (N_MAGIC(ex) != NMAGIC))
+			pr_notice_ratelimited("executable not page aligned\n");
 
-		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",
-			       bprm->file->f_path.dentry->d_name.name);
-			error_time = jiffies;
-		}
+		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);
 #endif
 
-		if (!bprm->file->f_op->mmap || (fd_offset & ~PAGE_MASK) != 0) {
-			vm_brk(N_TXTADDR(ex), ex.a_text+ex.a_data);
+		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);
+					ex.a_text + ex.a_data);
 			goto beyond_if;
 		}
 
@@ -424,10 +422,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;
 
@@ -438,14 +443,8 @@ static int load_aout_library(struct file *file)
 
 	if ((N_TXTOFF(ex) & ~PAGE_MASK) != 0) {
 #ifdef WARN_OLD
-		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",
-			       file->f_path.dentry->d_name.name);
-			error_time = jiffies;
-		}
+		pr_warn_ratelimited("N_TXTOFF is not page aligned. Please convert library: %s\n",
+				    file->f_path.dentry->d_name.name);
 #endif
 		vm_brk(start_addr, ex.a_text + ex.a_data + ex.a_bss);
 
-- 
1.8.4

Re: [PATCH v3 2/2] ia32_aout: x86_64: Add safe check in a.out loaders, printks, conding style fixes

[Geyslan Gregório Bem]

Please, disconsider this e-mail.

Geyslan Gregório Bem
hackingbits.com


2013/9/28 Geyslan G. Bem <geyslan@gmail.com>:
> 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.
> Made coding style fixes and printks replacements.
>
> Tested using qemu, a handcrafted a.out binary and an a.out linked with a
> cross-compiled ld.
>
> Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
> ---
>  arch/x86/ia32/ia32_aout.c | 63 +++++++++++++++++++++++------------------------
>  1 file changed, 31 insertions(+), 32 deletions(-)
>
> diff --git a/arch/x86/ia32/ia32_aout.c b/arch/x86/ia32/ia32_aout.c
> index bae3aba..87d5114 100644
> --- a/arch/x86/ia32/ia32_aout.c
> +++ b/arch/x86/ia32/ia32_aout.c
> @@ -24,9 +24,9 @@
>  #include <linux/binfmts.h>
>  #include <linux/personality.h>
>  #include <linux/init.h>
> -#include <linux/jiffies.h>
> +#include <linux/ratelimit.h>
> +#include <linux/uaccess.h>
>
> -#include <asm/uaccess.h>
>  #include <asm/pgalloc.h>
>  #include <asm/cacheflush.h>
>  #include <asm/user32.h>
> @@ -224,9 +224,9 @@ static u32 __user *create_aout_tables(char __user *p, struct linux_binprm *bprm)
>         int argc = bprm->argc, envc = bprm->envc;
>
>         sp = (u32 __user *) ((-(unsigned long)sizeof(u32)) & (unsigned long) p);
> -       sp -= envc+1;
> +       sp -= envc + 1;
>         envp = sp;
> -       sp -= argc+1;
> +       sp -= argc + 1;
>         argv = sp;
>         put_user((unsigned long) envp, --sp);
>         put_user((unsigned long) argv, --sp);
> @@ -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
> @@ -322,7 +329,7 @@ static int load_aout_binary(struct linux_binprm *bprm)
>                 unsigned long text_addr, map_size;
>
>                 text_addr = N_TXTADDR(ex);
> -               map_size = ex.a_text+ex.a_data;
> +               map_size = ex.a_text + ex.a_data;
>
>                 error = vm_brk(text_addr & PAGE_MASK, map_size);
>
> @@ -339,28 +346,19 @@ static int load_aout_binary(struct linux_binprm *bprm)
>                 }
>         } else {
>  #ifdef WARN_OLD
> -               static unsigned long error_time, error_time2;
>                 if ((ex.a_text & 0xfff || ex.a_data & 0xfff) &&
> -                   (N_MAGIC(ex) != NMAGIC) &&
> -                               time_after(jiffies, error_time2 + 5*HZ)) {
> -                       printk(KERN_NOTICE "executable not page aligned\n");
> -                       error_time2 = jiffies;
> -               }
> +                   (N_MAGIC(ex) != NMAGIC))
> +                       pr_notice_ratelimited("executable not page aligned\n");
>
> -               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",
> -                              bprm->file->f_path.dentry->d_name.name);
> -                       error_time = jiffies;
> -               }
> +               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);
>  #endif
>
> -               if (!bprm->file->f_op->mmap || (fd_offset & ~PAGE_MASK) != 0) {
> -                       vm_brk(N_TXTADDR(ex), ex.a_text+ex.a_data);
> +               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);
> +                                       ex.a_text + ex.a_data);
>                         goto beyond_if;
>                 }
>
> @@ -424,10 +422,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;
>
> @@ -438,14 +443,8 @@ static int load_aout_library(struct file *file)
>
>         if ((N_TXTOFF(ex) & ~PAGE_MASK) != 0) {
>  #ifdef WARN_OLD
> -               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",
> -                              file->f_path.dentry->d_name.name);
> -                       error_time = jiffies;
> -               }
> +               pr_warn_ratelimited("N_TXTOFF is not page aligned. Please convert library: %s\n",
> +                                   file->f_path.dentry->d_name.name);
>  #endif
>                 vm_brk(start_addr, ex.a_text + ex.a_data + ex.a_bss);
>
> --
> 1.8.4
>

[PATCH 1/1] fs/binfmt_elf.c: fill_note_info: Reduce scope of a variable

[Geyslan G. Bem]

The *t variable in the fill_note_info function is only used if
siginfo->si_signo isn't 0. Moving "t" pointer to that inner scope.

Tested.

Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
---
 fs/binfmt_elf.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 100edcc..d0ac15b 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -1848,12 +1848,11 @@ static int fill_note_info(struct elfhdr *elf, int phdrs,
 			  struct elf_note_info *info,
 			  siginfo_t *siginfo, struct pt_regs *regs)
 {
-	struct list_head *t;
-
 	if (!elf_note_info_init(info))
 		return 0;
 
 	if (siginfo->si_signo) {
+		struct list_head *t;
 		struct core_thread *ct;
 		struct elf_thread_status *ets;
 
-- 
1.8.4