* [PATCH] 9p: remove unused 'p9_fid' struct pointer
@ 2013-09-28 23:32 Geyslan G. Bem
2013-09-28 23:32 ` [PATCH] 9p: remove unused 'p9_client' " Geyslan G. Bem
` (10 more replies)
0 siblings, 11 replies; 20+ messages in thread
From: Geyslan G. Bem @ 2013-09-28 23:32 UTC (permalink / raw)
To: ericvh, rminnich, lucho; +Cc: v9fs-developer, linux-kernel, 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
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH] 9p: remove unused 'p9_client' struct pointer
2013-09-28 23:32 [PATCH] 9p: remove unused 'p9_fid' struct pointer Geyslan G. Bem
@ 2013-09-28 23:32 ` Geyslan G. Bem
2013-09-28 23:32 ` [PATCH] 9p: remove useless assignment Geyslan G. Bem
` (9 subsequent siblings)
10 siblings, 0 replies; 20+ messages in thread
From: Geyslan G. Bem @ 2013-09-28 23:32 UTC (permalink / raw)
To: ericvh, rminnich, lucho; +Cc: v9fs-developer, linux-kernel, 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
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH] 9p: remove useless assignment
2013-09-28 23:32 [PATCH] 9p: remove unused 'p9_fid' struct pointer Geyslan G. Bem
2013-09-28 23:32 ` [PATCH] 9p: remove unused 'p9_client' " Geyslan G. Bem
@ 2013-09-28 23:32 ` Geyslan G. Bem
2013-09-28 23:32 ` [PATCH] 9p: remove unused 'super_block' struct pointer Geyslan G. Bem
` (8 subsequent siblings)
10 siblings, 0 replies; 20+ messages in thread
From: Geyslan G. Bem @ 2013-09-28 23:32 UTC (permalink / raw)
To: ericvh, rminnich, lucho; +Cc: v9fs-developer, linux-kernel, 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
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH] 9p: remove unused 'super_block' struct pointer
2013-09-28 23:32 [PATCH] 9p: remove unused 'p9_fid' struct pointer Geyslan G. Bem
2013-09-28 23:32 ` [PATCH] 9p: remove unused 'p9_client' " Geyslan G. Bem
2013-09-28 23:32 ` [PATCH] 9p: remove useless assignment Geyslan G. Bem
@ 2013-09-28 23:32 ` Geyslan G. Bem
2013-09-28 23:32 ` [PATCH] 9p: fix return value in case of error in v9fs_fid_xattr_set Geyslan G. Bem
` (7 subsequent siblings)
10 siblings, 0 replies; 20+ messages in thread
From: Geyslan G. Bem @ 2013-09-28 23:32 UTC (permalink / raw)
To: ericvh, rminnich, lucho; +Cc: v9fs-developer, linux-kernel, 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
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH] 9p: fix return value in case of error in v9fs_fid_xattr_set
2013-09-28 23:32 [PATCH] 9p: remove unused 'p9_fid' struct pointer Geyslan G. Bem
` (2 preceding siblings ...)
2013-09-28 23:32 ` [PATCH] 9p: remove unused 'super_block' struct pointer Geyslan G. Bem
@ 2013-09-28 23:32 ` Geyslan G. Bem
[not found] ` <CAFkjPTkKeEFmVWoPFXhAkBwwO67M1K-c1Q2OUvajSArTSgrFvg@mail.gmail.com>
2013-09-28 23:32 ` [PATCH] 9p: remove never used return variable Geyslan G. Bem
` (6 subsequent siblings)
10 siblings, 1 reply; 20+ messages in thread
From: Geyslan G. Bem @ 2013-09-28 23:32 UTC (permalink / raw)
To: ericvh, rminnich, lucho; +Cc: v9fs-developer, linux-kernel, 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
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH] 9p: remove never used return variable
2013-09-28 23:32 [PATCH] 9p: remove unused 'p9_fid' struct pointer Geyslan G. Bem
` (3 preceding siblings ...)
2013-09-28 23:32 ` [PATCH] 9p: fix return value in case of error in v9fs_fid_xattr_set Geyslan G. Bem
@ 2013-09-28 23:32 ` Geyslan G. Bem
2013-09-28 23:32 ` [PATCH] 9p: remove useless variable and assignment Geyslan G. Bem
` (5 subsequent siblings)
10 siblings, 0 replies; 20+ messages in thread
From: Geyslan G. Bem @ 2013-09-28 23:32 UTC (permalink / raw)
To: ericvh, rminnich, lucho; +Cc: v9fs-developer, linux-kernel, 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
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH] 9p: remove useless variable and assignment
2013-09-28 23:32 [PATCH] 9p: remove unused 'p9_fid' struct pointer Geyslan G. Bem
` (4 preceding siblings ...)
2013-09-28 23:32 ` [PATCH] 9p: remove never used return variable Geyslan G. Bem
@ 2013-09-28 23:32 ` Geyslan G. Bem
2013-09-28 23:32 ` [PATCH] 9p: proper use of the 'name' variable Geyslan G. Bem
` (4 subsequent siblings)
10 siblings, 0 replies; 20+ messages in thread
From: Geyslan G. Bem @ 2013-09-28 23:32 UTC (permalink / raw)
To: ericvh, rminnich, lucho; +Cc: v9fs-developer, linux-kernel, 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
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH] 9p: proper use of the 'name' variable
2013-09-28 23:32 [PATCH] 9p: remove unused 'p9_fid' struct pointer Geyslan G. Bem
` (5 preceding siblings ...)
2013-09-28 23:32 ` [PATCH] 9p: remove useless variable and assignment Geyslan G. Bem
@ 2013-09-28 23:32 ` Geyslan G. Bem
[not found] ` <CAFkjPTmg26H0RGdHmR2u-wuJF=Jhd4PbbG1hVrBucuWyZDgNAw@mail.gmail.com>
2013-09-28 23:32 ` [PATCH v3 1/2] binfmt_aout: x86: Useless inode var, printks coding style fixes Geyslan G. Bem
` (3 subsequent siblings)
10 siblings, 1 reply; 20+ messages in thread
From: Geyslan G. Bem @ 2013-09-28 23:32 UTC (permalink / raw)
To: ericvh, rminnich, lucho; +Cc: v9fs-developer, linux-kernel, 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
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v3 1/2] binfmt_aout: x86: Useless inode var, printks coding style fixes
2013-09-28 23:32 [PATCH] 9p: remove unused 'p9_fid' struct pointer Geyslan G. Bem
` (6 preceding siblings ...)
2013-09-28 23:32 ` [PATCH] 9p: proper use of the 'name' variable Geyslan G. Bem
@ 2013-09-28 23:32 ` Geyslan G. Bem
2013-09-28 23:41 ` Geyslan Gregório Bem
2013-09-28 23:32 ` [PATCH 1/1] fs/binfmt_elf.c: fill_note_info: Reduce scope of a variable Geyslan G. Bem
` (2 subsequent siblings)
10 siblings, 1 reply; 20+ messages in thread
From: Geyslan G. Bem @ 2013-09-28 23:32 UTC (permalink / raw)
To: ericvh, rminnich, lucho; +Cc: v9fs-developer, linux-kernel, 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
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 1/1] fs/binfmt_elf.c: fill_note_info: Reduce scope of a variable
2013-09-28 23:32 [PATCH] 9p: remove unused 'p9_fid' struct pointer Geyslan G. Bem
` (7 preceding siblings ...)
2013-09-28 23:32 ` [PATCH v3 1/2] binfmt_aout: x86: Useless inode var, printks coding style fixes Geyslan G. Bem
@ 2013-09-28 23:32 ` Geyslan G. Bem
2013-09-28 23:42 ` Geyslan Gregório Bem
2013-09-28 23:32 ` [PATCH] fs: exec.c: Coding style sanitization Geyslan G. Bem
2013-09-28 23:32 ` [PATCH v3 2/2] ia32_aout: x86_64: Add safe check in a.out loaders, printks, conding style fixes Geyslan G. Bem
10 siblings, 1 reply; 20+ messages in thread
From: Geyslan G. Bem @ 2013-09-28 23:32 UTC (permalink / raw)
To: ericvh, rminnich, lucho; +Cc: v9fs-developer, linux-kernel, 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
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH] fs: exec.c: Coding style sanitization
2013-09-28 23:32 [PATCH] 9p: remove unused 'p9_fid' struct pointer Geyslan G. Bem
` (8 preceding siblings ...)
2013-09-28 23:32 ` [PATCH 1/1] fs/binfmt_elf.c: fill_note_info: Reduce scope of a variable Geyslan G. Bem
@ 2013-09-28 23:32 ` Geyslan G. Bem
2013-09-28 23:41 ` Geyslan Gregório Bem
2013-09-28 23:32 ` [PATCH v3 2/2] ia32_aout: x86_64: Add safe check in a.out loaders, printks, conding style fixes Geyslan G. Bem
10 siblings, 1 reply; 20+ messages in thread
From: Geyslan G. Bem @ 2013-09-28 23:32 UTC (permalink / raw)
To: ericvh, rminnich, lucho; +Cc: v9fs-developer, linux-kernel, 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
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v3 2/2] ia32_aout: x86_64: Add safe check in a.out loaders, printks, conding style fixes
2013-09-28 23:32 [PATCH] 9p: remove unused 'p9_fid' struct pointer Geyslan G. Bem
` (9 preceding siblings ...)
2013-09-28 23:32 ` [PATCH] fs: exec.c: Coding style sanitization Geyslan G. Bem
@ 2013-09-28 23:32 ` Geyslan G. Bem
2013-09-28 23:41 ` Geyslan Gregório Bem
10 siblings, 1 reply; 20+ messages in thread
From: Geyslan G. Bem @ 2013-09-28 23:32 UTC (permalink / raw)
To: ericvh, rminnich, lucho; +Cc: v9fs-developer, 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.
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
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v3 1/2] binfmt_aout: x86: Useless inode var, printks coding style fixes
2013-09-28 23:32 ` [PATCH v3 1/2] binfmt_aout: x86: Useless inode var, printks coding style fixes Geyslan G. Bem
@ 2013-09-28 23:41 ` Geyslan Gregório Bem
0 siblings, 0 replies; 20+ messages in thread
From: Geyslan Gregório Bem @ 2013-09-28 23:41 UTC (permalink / raw)
To: ericvh, rminnich, lucho; +Cc: v9fs-developer, linux-kernel, Geyslan G. 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
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3 2/2] ia32_aout: x86_64: Add safe check in a.out loaders, printks, conding style fixes
2013-09-28 23:32 ` [PATCH v3 2/2] ia32_aout: x86_64: Add safe check in a.out loaders, printks, conding style fixes Geyslan G. Bem
@ 2013-09-28 23:41 ` Geyslan Gregório Bem
0 siblings, 0 replies; 20+ messages in thread
From: Geyslan Gregório Bem @ 2013-09-28 23:41 UTC (permalink / raw)
To: ericvh, rminnich, lucho; +Cc: v9fs-developer, linux-kernel, Geyslan G. 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
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] fs: exec.c: Coding style sanitization
2013-09-28 23:32 ` [PATCH] fs: exec.c: Coding style sanitization Geyslan G. Bem
@ 2013-09-28 23:41 ` Geyslan Gregório Bem
0 siblings, 0 replies; 20+ messages in thread
From: Geyslan Gregório Bem @ 2013-09-28 23:41 UTC (permalink / raw)
To: ericvh, rminnich, lucho; +Cc: v9fs-developer, linux-kernel, Geyslan G. 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
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/1] fs/binfmt_elf.c: fill_note_info: Reduce scope of a variable
2013-09-28 23:32 ` [PATCH 1/1] fs/binfmt_elf.c: fill_note_info: Reduce scope of a variable Geyslan G. Bem
@ 2013-09-28 23:42 ` Geyslan Gregório Bem
0 siblings, 0 replies; 20+ messages in thread
From: Geyslan Gregório Bem @ 2013-09-28 23:42 UTC (permalink / raw)
To: ericvh, rminnich, lucho; +Cc: v9fs-developer, linux-kernel, Geyslan G. 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
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] 9p: fix return value in case of error in v9fs_fid_xattr_set
[not found] ` <CAFkjPTkKeEFmVWoPFXhAkBwwO67M1K-c1Q2OUvajSArTSgrFvg@mail.gmail.com>
@ 2013-10-21 10:47 ` Geyslan Gregório Bem
2013-10-21 19:52 ` Geyslan Gregório Bem
0 siblings, 1 reply; 20+ messages in thread
From: Geyslan Gregório Bem @ 2013-10-21 10:47 UTC (permalink / raw)
To: Eric Van Hensbergen
Cc: rminnich, Latchesar Ionkov, V9FS Developers, linux-kernel
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
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] 9p: proper use of the 'name' variable
[not found] ` <CAFkjPTmg26H0RGdHmR2u-wuJF=Jhd4PbbG1hVrBucuWyZDgNAw@mail.gmail.com>
@ 2013-10-21 10:53 ` Geyslan Gregório Bem
2013-10-21 19:00 ` Geyslan Gregório Bem
0 siblings, 1 reply; 20+ messages in thread
From: Geyslan Gregório Bem @ 2013-10-21 10:53 UTC (permalink / raw)
To: Eric Van Hensbergen
Cc: rminnich, Latchesar Ionkov, V9FS Developers, linux-kernel
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
>>
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] 9p: proper use of the 'name' variable
2013-10-21 10:53 ` Geyslan Gregório Bem
@ 2013-10-21 19:00 ` Geyslan Gregório Bem
0 siblings, 0 replies; 20+ messages in thread
From: Geyslan Gregório Bem @ 2013-10-21 19:00 UTC (permalink / raw)
To: Eric Van Hensbergen
Cc: rminnich, Latchesar Ionkov, V9FS Developers, linux-kernel
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
>>>
>>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] 9p: fix return value in case of error in v9fs_fid_xattr_set
2013-10-21 10:47 ` Geyslan Gregório Bem
@ 2013-10-21 19:52 ` Geyslan Gregório Bem
0 siblings, 0 replies; 20+ messages in thread
From: Geyslan Gregório Bem @ 2013-10-21 19:52 UTC (permalink / raw)
To: Eric Van Hensbergen
Cc: Ron Minnich, Latchesar Ionkov, V9FS Developers, linux-kernel
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()
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2013-10-21 19:53 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-09-28 23:32 [PATCH] 9p: remove unused 'p9_fid' struct pointer Geyslan G. Bem
2013-09-28 23:32 ` [PATCH] 9p: remove unused 'p9_client' " Geyslan G. Bem
2013-09-28 23:32 ` [PATCH] 9p: remove useless assignment Geyslan G. Bem
2013-09-28 23:32 ` [PATCH] 9p: remove unused 'super_block' struct pointer Geyslan G. Bem
2013-09-28 23:32 ` [PATCH] 9p: fix return value in case of error in v9fs_fid_xattr_set Geyslan G. Bem
[not found] ` <CAFkjPTkKeEFmVWoPFXhAkBwwO67M1K-c1Q2OUvajSArTSgrFvg@mail.gmail.com>
2013-10-21 10:47 ` Geyslan Gregório Bem
2013-10-21 19:52 ` Geyslan Gregório Bem
2013-09-28 23:32 ` [PATCH] 9p: remove never used return variable Geyslan G. Bem
2013-09-28 23:32 ` [PATCH] 9p: remove useless variable and assignment Geyslan G. Bem
2013-09-28 23:32 ` [PATCH] 9p: proper use of the 'name' variable Geyslan G. Bem
[not found] ` <CAFkjPTmg26H0RGdHmR2u-wuJF=Jhd4PbbG1hVrBucuWyZDgNAw@mail.gmail.com>
2013-10-21 10:53 ` Geyslan Gregório Bem
2013-10-21 19:00 ` Geyslan Gregório Bem
2013-09-28 23:32 ` [PATCH v3 1/2] binfmt_aout: x86: Useless inode var, printks coding style fixes Geyslan G. Bem
2013-09-28 23:41 ` Geyslan Gregório Bem
2013-09-28 23:32 ` [PATCH 1/1] fs/binfmt_elf.c: fill_note_info: Reduce scope of a variable Geyslan G. Bem
2013-09-28 23:42 ` Geyslan Gregório Bem
2013-09-28 23:32 ` [PATCH] fs: exec.c: Coding style sanitization Geyslan G. Bem
2013-09-28 23:41 ` Geyslan Gregório Bem
2013-09-28 23:32 ` [PATCH v3 2/2] ia32_aout: x86_64: Add safe check in a.out loaders, printks, conding style fixes Geyslan G. Bem
2013-09-28 23:41 ` Geyslan Gregório 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.