Discussion:
[PATCH 2/2] binfmt_misc: clean up code style a bit
(too old to reply)
Mike Frysinger
2014-10-19 23:03:46 UTC
Permalink
Clean up various coding style issues that checkpatch complains about.
No functional changes here.

Signed-off-by: Mike Frysinger <***@gentoo.org>
---
fs/binfmt_misc.c | 295 +++++++++++++++++++++++++++--------------------=
--------
1 file changed, 146 insertions(+), 149 deletions(-)

diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c
index dbf0ac5..acd3245 100644
--- a/fs/binfmt_misc.c
+++ b/fs/binfmt_misc.c
@@ -1,19 +1,10 @@
/*
- * binfmt_misc.c
+ * binfmt_misc.c
*
- * Copyright (C) 1997 Richard G=C3=BCnther
+ * Copyright (C) 1997 Richard G=C3=BCnther
*
- * binfmt_misc detects binaries via a magic or filename extension and=
invokes
- * a specified wrapper. This should obsolete binfmt_java, binfmt_em86=
and
- * binfmt_mz.
- *
- * 1997-04-25 first version
- * [...]
- * 1997-05-19 cleanup
- * 1997-06-26 hpa: pass the real filename rather than argv[0]
- * 1997-06-30 minor cleanup
- * 1997-08-09 removed extension stripping, locking cleanup
- * 2001-02-28 AV: rewritten into something that resembles C. Original=
didn't.
+ * binfmt_misc detects binaries via a magic or filename extension and =
invokes
+ * a specified wrapper. See Documentation/binfmt_misc.txt for more det=
ails.
*/
=20
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
@@ -48,9 +39,9 @@ static LIST_HEAD(entries);
static int enabled =3D 1;
=20
enum {Enabled, Magic};
-#define MISC_FMT_PRESERVE_ARGV0 (1<<31)
-#define MISC_FMT_OPEN_BINARY (1<<30)
-#define MISC_FMT_CREDENTIALS (1<<29)
+#define MISC_FMT_PRESERVE_ARGV0 (1 << 31)
+#define MISC_FMT_OPEN_BINARY (1 << 30)
+#define MISC_FMT_CREDENTIALS (1 << 29)
=20
typedef struct {
struct list_head list;
@@ -134,7 +125,7 @@ static Node *check_file(struct linux_binprm *bprm)
static int load_misc_binary(struct linux_binprm *bprm)
{
Node *fmt;
- struct file * interp_file =3D NULL;
+ struct file *interp_file =3D NULL;
char iname[BINPRM_BUF_SIZE];
const char *iname_addr =3D iname;
int retval;
@@ -142,7 +133,7 @@ static int load_misc_binary(struct linux_binprm *bp=
rm)
=20
retval =3D -ENOEXEC;
if (!enabled)
- goto _ret;
+ goto ret;
=20
/* to keep locking time low, we copy the interpreter string */
read_lock(&entries_lock);
@@ -151,25 +142,26 @@ static int load_misc_binary(struct linux_binprm *=
bprm)
strlcpy(iname, fmt->interpreter, BINPRM_BUF_SIZE);
read_unlock(&entries_lock);
if (!fmt)
- goto _ret;
+ goto ret;
=20
if (!(fmt->flags & MISC_FMT_PRESERVE_ARGV0)) {
retval =3D remove_arg_zero(bprm);
if (retval)
- goto _ret;
+ goto ret;
}
=20
if (fmt->flags & MISC_FMT_OPEN_BINARY) {
=20
/* if the binary should be opened on behalf of the
* interpreter than keep it open and assign descriptor
- * to it */
- fd_binary =3D get_unused_fd();
- if (fd_binary < 0) {
- retval =3D fd_binary;
- goto _ret;
- }
- fd_install(fd_binary, bprm->file);
+ * to it
+ */
+ fd_binary =3D get_unused_fd();
+ if (fd_binary < 0) {
+ retval =3D fd_binary;
+ goto ret;
+ }
+ fd_install(fd_binary, bprm->file);
=20
/* if the binary is not readable than enforce mm->dumpable=3D0
regardless of the interpreter's permissions */
@@ -182,32 +174,32 @@ static int load_misc_binary(struct linux_binprm *=
bprm)
bprm->interp_flags |=3D BINPRM_FLAGS_EXECFD;
bprm->interp_data =3D fd_binary;
=20
- } else {
- allow_write_access(bprm->file);
- fput(bprm->file);
- bprm->file =3D NULL;
- }
+ } else {
+ allow_write_access(bprm->file);
+ fput(bprm->file);
+ bprm->file =3D NULL;
+ }
/* make argv[1] be the path to the binary */
- retval =3D copy_strings_kernel (1, &bprm->interp, bprm);
+ retval =3D copy_strings_kernel(1, &bprm->interp, bprm);
if (retval < 0)
- goto _error;
+ goto error;
bprm->argc++;
=20
/* add the interp as argv[0] */
- retval =3D copy_strings_kernel (1, &iname_addr, bprm);
+ retval =3D copy_strings_kernel(1, &iname_addr, bprm);
if (retval < 0)
- goto _error;
- bprm->argc ++;
+ goto error;
+ bprm->argc++;
=20
/* Update interp in case binfmt_script needs it. */
retval =3D bprm_change_interp(iname, bprm);
if (retval < 0)
- goto _error;
+ goto error;
=20
- interp_file =3D open_exec (iname);
- retval =3D PTR_ERR (interp_file);
- if (IS_ERR (interp_file))
- goto _error;
+ interp_file =3D open_exec(iname);
+ retval =3D PTR_ERR(interp_file);
+ if (IS_ERR(interp_file))
+ goto error;
=20
bprm->file =3D interp_file;
if (fmt->flags & MISC_FMT_CREDENTIALS) {
@@ -218,23 +210,23 @@ static int load_misc_binary(struct linux_binprm *=
bprm)
memset(bprm->buf, 0, BINPRM_BUF_SIZE);
retval =3D kernel_read(bprm->file, 0, bprm->buf, BINPRM_BUF_SIZE);
} else
- retval =3D prepare_binprm (bprm);
+ retval =3D prepare_binprm(bprm);
=20
if (retval < 0)
- goto _error;
+ goto error;
=20
retval =3D search_binary_handler(bprm);
if (retval < 0)
- goto _error;
+ goto error;
=20
-_ret:
+ret:
return retval;
-_error:
+error:
if (fd_binary > 0)
sys_close(fd_binary);
bprm->interp_flags =3D 0;
bprm->interp_data =3D 0;
- goto _ret;
+ goto ret;
}
=20
/* Command parsers */
@@ -261,39 +253,40 @@ static char *scanarg(char *s, char del)
return s;
}
=20
-static char * check_special_flags (char * sfs, Node * e)
+static char *check_special_flags(char *sfs, Node *e)
{
- char * p =3D sfs;
+ char *p =3D sfs;
int cont =3D 1;
=20
/* special flags */
while (cont) {
switch (*p) {
- case 'P':
- pr_debug("register: flag: P (preserve argv0)");
- p++;
- e->flags |=3D MISC_FMT_PRESERVE_ARGV0;
- break;
- case 'O':
- pr_debug("register: flag: O (open binary)");
- p++;
- e->flags |=3D MISC_FMT_OPEN_BINARY;
- break;
- case 'C':
- pr_debug("register: flag: C (preserve creds)");
- p++;
- /* this flags also implies the
- open-binary flag */
- e->flags |=3D (MISC_FMT_CREDENTIALS |
- MISC_FMT_OPEN_BINARY);
- break;
- default:
- cont =3D 0;
+ case 'P':
+ pr_debug("register: flag: P (preserve argv0)");
+ p++;
+ e->flags |=3D MISC_FMT_PRESERVE_ARGV0;
+ break;
+ case 'O':
+ pr_debug("register: flag: O (open binary)");
+ p++;
+ e->flags |=3D MISC_FMT_OPEN_BINARY;
+ break;
+ case 'C':
+ pr_debug("register: flag: C (preserve creds)");
+ p++;
+ /* this flags also implies the
+ open-binary flag */
+ e->flags |=3D (MISC_FMT_CREDENTIALS |
+ MISC_FMT_OPEN_BINARY);
+ break;
+ default:
+ cont =3D 0;
}
}
=20
return p;
}
+
/*
* This registers a new binary format, it recognises the syntax
* ':name:type:offset:magic:mask:interpreter:flags'
@@ -323,26 +316,26 @@ static Node *create_entry(const char __user *buff=
er, size_t count)
=20
memset(e, 0, sizeof(Node));
if (copy_from_user(buf, buffer, count))
- goto Efault;
+ goto efault;
=20
del =3D *p++; /* delimeter */
=20
pr_debug("register: delim: %#x {%c}", del, del);
=20
/* Pad the buffer with the delim to simplify parsing below. */
- memset(buf+count, del, 8);
+ memset(buf + count, del, 8);
=20
/* Parse the 'name' field. */
e->name =3D p;
p =3D strchr(p, del);
if (!p)
- goto Einval;
+ goto einval;
*p++ =3D '\0';
if (!e->name[0] ||
!strcmp(e->name, ".") ||
!strcmp(e->name, "..") ||
strchr(e->name, '/'))
- goto Einval;
+ goto einval;
=20
pr_debug("register: name: {%s}", e->name);
=20
@@ -357,10 +350,10 @@ static Node *create_entry(const char __user *buff=
er, size_t count)
e->flags =3D (1 << Enabled) | (1 << Magic);
break;
default:
- goto Einval;
+ goto einval;
}
if (*p++ !=3D del)
- goto Einval;
+ goto einval;
=20
if (test_bit(Magic, &e->flags)) {
/* Handle the 'M' (magic) format. */
@@ -369,21 +362,21 @@ static Node *create_entry(const char __user *buff=
er, size_t count)
/* Parse the 'offset' field. */
s =3D strchr(p, del);
if (!s)
- goto Einval;
+ goto einval;
*s++ =3D '\0';
e->offset =3D simple_strtoul(p, &p, 10);
if (*p++)
- goto Einval;
+ goto einval;
pr_debug("register: offset: %#x", e->offset);
=20
/* Parse the 'magic' field. */
e->magic =3D p;
p =3D scanarg(p, del);
if (!p)
- goto Einval;
+ goto einval;
p[-1] =3D '\0';
if (p =3D=3D e->magic)
- goto Einval;
+ goto einval;
if (USE_DEBUG)
print_hex_dump_bytes(
KBUILD_MODNAME ": register: magic[raw]: ",
@@ -393,7 +386,7 @@ static Node *create_entry(const char __user *buffer=
, size_t count)
e->mask =3D p;
p =3D scanarg(p, del);
if (!p)
- goto Einval;
+ goto einval;
p[-1] =3D '\0';
if (p =3D=3D e->mask) {
e->mask =3D NULL;
@@ -412,9 +405,9 @@ static Node *create_entry(const char __user *buffer=
, size_t count)
e->size =3D string_unescape_inplace(e->magic, UNESCAPE_HEX);
if (e->mask &&
string_unescape_inplace(e->mask, UNESCAPE_HEX) !=3D e->size)
- goto Einval;
+ goto einval;
if (e->size + e->offset > BINPRM_BUF_SIZE)
- goto Einval;
+ goto einval;
pr_debug("register: magic/mask length: %i", e->size);
if (USE_DEBUG) {
print_hex_dump_bytes(
@@ -446,23 +439,23 @@ static Node *create_entry(const char __user *buff=
er, size_t count)
/* Skip the 'offset' field. */
p =3D strchr(p, del);
if (!p)
- goto Einval;
+ goto einval;
*p++ =3D '\0';
=20
/* Parse the 'magic' field. */
e->magic =3D p;
p =3D strchr(p, del);
if (!p)
- goto Einval;
+ goto einval;
*p++ =3D '\0';
if (!e->magic[0] || strchr(e->magic, '/'))
- goto Einval;
+ goto einval;
pr_debug("register: extension: {%s}", e->magic);
=20
/* Skip the 'mask' field. */
p =3D strchr(p, del);
if (!p)
- goto Einval;
+ goto einval;
*p++ =3D '\0';
}
=20
@@ -470,27 +463,28 @@ static Node *create_entry(const char __user *buff=
er, size_t count)
e->interpreter =3D p;
p =3D strchr(p, del);
if (!p)
- goto Einval;
+ goto einval;
*p++ =3D '\0';
if (!e->interpreter[0])
- goto Einval;
+ goto einval;
pr_debug("register: interpreter: {%s}", e->interpreter);
=20
/* Parse the 'flags' field. */
- p =3D check_special_flags (p, e);
+ p =3D check_special_flags(p, e);
if (*p =3D=3D '\n')
p++;
if (p !=3D buf + count)
- goto Einval;
+ goto einval;
+
return e;
=20
out:
return ERR_PTR(err);
=20
-Efault:
+efault:
kfree(e);
return ERR_PTR(-EFAULT);
-Einval:
+einval:
kfree(e);
return ERR_PTR(-EINVAL);
}
@@ -509,7 +503,7 @@ static int parse_command(const char __user *buffer,=
size_t count)
return -EFAULT;
if (!count)
return 0;
- if (s[count-1] =3D=3D '\n')
+ if (s[count - 1] =3D=3D '\n')
count--;
if (count =3D=3D 1 && s[0] =3D=3D '0')
return 1;
@@ -526,7 +520,7 @@ static void entry_status(Node *e, char *page)
{
char *dp;
char *status =3D "disabled";
- const char * flags =3D "flags: ";
+ const char *flags =3D "flags: ";
=20
if (test_bit(Enabled, &e->flags))
status =3D "enabled";
@@ -540,19 +534,15 @@ static void entry_status(Node *e, char *page)
dp =3D page + strlen(page);
=20
/* print the special flags */
- sprintf (dp, "%s", flags);
- dp +=3D strlen (flags);
- if (e->flags & MISC_FMT_PRESERVE_ARGV0) {
- *dp ++ =3D 'P';
- }
- if (e->flags & MISC_FMT_OPEN_BINARY) {
- *dp ++ =3D 'O';
- }
- if (e->flags & MISC_FMT_CREDENTIALS) {
- *dp ++ =3D 'C';
- }
- *dp ++ =3D '\n';
-
+ sprintf(dp, "%s", flags);
+ dp +=3D strlen(flags);
+ if (e->flags & MISC_FMT_PRESERVE_ARGV0)
+ *dp++ =3D 'P';
+ if (e->flags & MISC_FMT_OPEN_BINARY)
+ *dp++ =3D 'O';
+ if (e->flags & MISC_FMT_CREDENTIALS)
+ *dp++ =3D 'C';
+ *dp++ =3D '\n';
=20
if (!test_bit(Magic, &e->flags)) {
sprintf(dp, "extension .%s\n", e->magic);
@@ -580,7 +570,7 @@ static void entry_status(Node *e, char *page)
=20
static struct inode *bm_get_inode(struct super_block *sb, int mode)
{
- struct inode * inode =3D new_inode(sb);
+ struct inode *inode =3D new_inode(sb);
=20
if (inode) {
inode->i_ino =3D get_next_ino();
@@ -620,13 +610,14 @@ static void kill_node(Node *e)
/* /<entry> */
=20
static ssize_t
-bm_entry_read(struct file * file, char __user * buf, size_t nbytes, lo=
ff_t *ppos)
+bm_entry_read(struct file *file, char __user *buf, size_t nbytes, loff=
_t *ppos)
{
Node *e =3D file_inode(file)->i_private;
ssize_t res;
char *page;
=20
- if (!(page =3D (char*) __get_free_page(GFP_KERNEL)))
+ page =3D (char *) __get_free_page(GFP_KERNEL);
+ if (!page)
return -ENOMEM;
=20
entry_status(e, page);
@@ -645,26 +636,28 @@ static ssize_t bm_entry_write(struct file *file, =
const char __user *buffer,
int res =3D parse_command(buffer, count);
=20
switch (res) {
- case 1:
- /* Disable this handler. */
- clear_bit(Enabled, &e->flags);
- break;
- case 2:
- /* Enable this handler. */
- set_bit(Enabled, &e->flags);
- break;
- case 3:
- /* Delete this handler. */
- root =3D dget(file->f_path.dentry->d_sb->s_root);
- mutex_lock(&root->d_inode->i_mutex);
+ case 1:
+ /* Disable this handler. */
+ clear_bit(Enabled, &e->flags);
+ break;
+ case 2:
+ /* Enable this handler. */
+ set_bit(Enabled, &e->flags);
+ break;
+ case 3:
+ /* Delete this handler. */
+ root =3D dget(file->f_path.dentry->d_sb->s_root);
+ mutex_lock(&root->d_inode->i_mutex);
=20
- kill_node(e);
+ kill_node(e);
=20
- mutex_unlock(&root->d_inode->i_mutex);
- dput(root);
- break;
- default: return res;
+ mutex_unlock(&root->d_inode->i_mutex);
+ dput(root);
+ break;
+ default:
+ return res;
}
+
return count;
}
=20
@@ -752,34 +745,36 @@ bm_status_read(struct file *file, char __user *bu=
f, size_t nbytes, loff_t *ppos)
return simple_read_from_buffer(buf, nbytes, ppos, s, strlen(s));
}
=20
-static ssize_t bm_status_write(struct file * file, const char __user *=
buffer,
+static ssize_t bm_status_write(struct file *file, const char __user *b=
uffer,
size_t count, loff_t *ppos)
{
int res =3D parse_command(buffer, count);
struct dentry *root;
=20
switch (res) {
- case 1:
- /* Disable all handlers. */
- enabled =3D 0;
- break;
- case 2:
- /* Enable all handlers. */
- enabled =3D 1;
- break;
- case 3:
- /* Delete all handlers. */
- root =3D dget(file->f_path.dentry->d_sb->s_root);
- mutex_lock(&root->d_inode->i_mutex);
+ case 1:
+ /* Disable all handlers. */
+ enabled =3D 0;
+ break;
+ case 2:
+ /* Enable all handlers. */
+ enabled =3D 1;
+ break;
+ case 3:
+ /* Delete all handlers. */
+ root =3D dget(file->f_path.dentry->d_sb->s_root);
+ mutex_lock(&root->d_inode->i_mutex);
=20
- while (!list_empty(&entries))
- kill_node(list_entry(entries.next, Node, list));
+ while (!list_empty(&entries))
+ kill_node(list_entry(entries.next, Node, list));
=20
- mutex_unlock(&root->d_inode->i_mutex);
- dput(root);
- break;
- default: return res;
+ mutex_unlock(&root->d_inode->i_mutex);
+ dput(root);
+ break;
+ default:
+ return res;
}
+
return count;
}
=20
@@ -796,14 +791,16 @@ static const struct super_operations s_ops =3D {
.evict_inode =3D bm_evict_inode,
};
=20
-static int bm_fill_super(struct super_block * sb, void * data, int sil=
ent)
+static int bm_fill_super(struct super_block *sb, void *data, int silen=
t)
{
+ int err;
static struct tree_descr bm_files[] =3D {
[2] =3D {"status", &bm_status_operations, S_IWUSR|S_IRUGO},
[3] =3D {"register", &bm_register_operations, S_IWUSR},
/* last one */ {""}
};
- int err =3D simple_fill_super(sb, BINFMTFS_MAGIC, bm_files);
+
+ err =3D simple_fill_super(sb, BINFMTFS_MAGIC, bm_files);
if (!err)
sb->s_op =3D &s_ops;
return err;
--=20
2.1.2
Joe Perches
2014-10-20 00:41:58 UTC
Permalink
let's deploy extensive pr_debug markers at
logical parse points, and add comments to the dense parsing logic.
[]
diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c
[]
@@ -259,14 +270,17 @@ static char * check_special_flags (char * sfs, Node * e)
while (cont) {
switch (*p) {
+ pr_debug("register: flag: P (preserve argv0)");
Missing '\n' newline.
Can you please add them as appropriate?
p++;
e->flags |= MISC_FMT_PRESERVE_ARGV0;
break;
+ pr_debug("register: flag: O (open binary)");
etc...
@@ -292,6 +306,8 @@ static Node *create_entry(const char __user *buffer, size_t count)
char *buf, *p;
char del;
+ pr_debug("register: received %zu bytes", count);
etc...
Mike Frysinger
2014-10-20 22:37:22 UTC
Permalink
Post by Joe Perches
let's deploy extensive pr_debug markers at
logical parse points, and add comments to the dense parsing logic.
[]
diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c
[]
@@ -259,14 +270,17 @@ static char * check_special_flags (char * sfs, Node * e)
while (cont) {
switch (*p) {
+ pr_debug("register: flag: P (preserve argv0)");
Missing '\n' newline.
Can you please add them as appropriate?
hmm, guess i was relying on pr_cont behavior to do the right thing. will do.
-mike
Mike Frysinger
2014-10-20 22:46:00 UTC
Permalink
Clean up various coding style issues that checkpatch complains about.
No functional changes here.

Signed-off-by: Mike Frysinger <***@gentoo.org>
---
v2
- rebased

fs/binfmt_misc.c | 295 +++++++++++++++++++++++++++--------------------=
--------
1 file changed, 146 insertions(+), 149 deletions(-)

diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c
index 5670b17..a953458 100644
--- a/fs/binfmt_misc.c
+++ b/fs/binfmt_misc.c
@@ -1,19 +1,10 @@
/*
- * binfmt_misc.c
+ * binfmt_misc.c
*
- * Copyright (C) 1997 Richard G=C3=BCnther
+ * Copyright (C) 1997 Richard G=C3=BCnther
*
- * binfmt_misc detects binaries via a magic or filename extension and=
invokes
- * a specified wrapper. This should obsolete binfmt_java, binfmt_em86=
and
- * binfmt_mz.
- *
- * 1997-04-25 first version
- * [...]
- * 1997-05-19 cleanup
- * 1997-06-26 hpa: pass the real filename rather than argv[0]
- * 1997-06-30 minor cleanup
- * 1997-08-09 removed extension stripping, locking cleanup
- * 2001-02-28 AV: rewritten into something that resembles C. Original=
didn't.
+ * binfmt_misc detects binaries via a magic or filename extension and =
invokes
+ * a specified wrapper. See Documentation/binfmt_misc.txt for more det=
ails.
*/
=20
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
@@ -48,9 +39,9 @@ static LIST_HEAD(entries);
static int enabled =3D 1;
=20
enum {Enabled, Magic};
-#define MISC_FMT_PRESERVE_ARGV0 (1<<31)
-#define MISC_FMT_OPEN_BINARY (1<<30)
-#define MISC_FMT_CREDENTIALS (1<<29)
+#define MISC_FMT_PRESERVE_ARGV0 (1 << 31)
+#define MISC_FMT_OPEN_BINARY (1 << 30)
+#define MISC_FMT_CREDENTIALS (1 << 29)
=20
typedef struct {
struct list_head list;
@@ -134,7 +125,7 @@ static Node *check_file(struct linux_binprm *bprm)
static int load_misc_binary(struct linux_binprm *bprm)
{
Node *fmt;
- struct file * interp_file =3D NULL;
+ struct file *interp_file =3D NULL;
char iname[BINPRM_BUF_SIZE];
const char *iname_addr =3D iname;
int retval;
@@ -142,7 +133,7 @@ static int load_misc_binary(struct linux_binprm *bp=
rm)
=20
retval =3D -ENOEXEC;
if (!enabled)
- goto _ret;
+ goto ret;
=20
/* to keep locking time low, we copy the interpreter string */
read_lock(&entries_lock);
@@ -151,25 +142,26 @@ static int load_misc_binary(struct linux_binprm *=
bprm)
strlcpy(iname, fmt->interpreter, BINPRM_BUF_SIZE);
read_unlock(&entries_lock);
if (!fmt)
- goto _ret;
+ goto ret;
=20
if (!(fmt->flags & MISC_FMT_PRESERVE_ARGV0)) {
retval =3D remove_arg_zero(bprm);
if (retval)
- goto _ret;
+ goto ret;
}
=20
if (fmt->flags & MISC_FMT_OPEN_BINARY) {
=20
/* if the binary should be opened on behalf of the
* interpreter than keep it open and assign descriptor
- * to it */
- fd_binary =3D get_unused_fd();
- if (fd_binary < 0) {
- retval =3D fd_binary;
- goto _ret;
- }
- fd_install(fd_binary, bprm->file);
+ * to it
+ */
+ fd_binary =3D get_unused_fd();
+ if (fd_binary < 0) {
+ retval =3D fd_binary;
+ goto ret;
+ }
+ fd_install(fd_binary, bprm->file);
=20
/* if the binary is not readable than enforce mm->dumpable=3D0
regardless of the interpreter's permissions */
@@ -182,32 +174,32 @@ static int load_misc_binary(struct linux_binprm *=
bprm)
bprm->interp_flags |=3D BINPRM_FLAGS_EXECFD;
bprm->interp_data =3D fd_binary;
=20
- } else {
- allow_write_access(bprm->file);
- fput(bprm->file);
- bprm->file =3D NULL;
- }
+ } else {
+ allow_write_access(bprm->file);
+ fput(bprm->file);
+ bprm->file =3D NULL;
+ }
/* make argv[1] be the path to the binary */
- retval =3D copy_strings_kernel (1, &bprm->interp, bprm);
+ retval =3D copy_strings_kernel(1, &bprm->interp, bprm);
if (retval < 0)
- goto _error;
+ goto error;
bprm->argc++;
=20
/* add the interp as argv[0] */
- retval =3D copy_strings_kernel (1, &iname_addr, bprm);
+ retval =3D copy_strings_kernel(1, &iname_addr, bprm);
if (retval < 0)
- goto _error;
- bprm->argc ++;
+ goto error;
+ bprm->argc++;
=20
/* Update interp in case binfmt_script needs it. */
retval =3D bprm_change_interp(iname, bprm);
if (retval < 0)
- goto _error;
+ goto error;
=20
- interp_file =3D open_exec (iname);
- retval =3D PTR_ERR (interp_file);
- if (IS_ERR (interp_file))
- goto _error;
+ interp_file =3D open_exec(iname);
+ retval =3D PTR_ERR(interp_file);
+ if (IS_ERR(interp_file))
+ goto error;
=20
bprm->file =3D interp_file;
if (fmt->flags & MISC_FMT_CREDENTIALS) {
@@ -218,23 +210,23 @@ static int load_misc_binary(struct linux_binprm *=
bprm)
memset(bprm->buf, 0, BINPRM_BUF_SIZE);
retval =3D kernel_read(bprm->file, 0, bprm->buf, BINPRM_BUF_SIZE);
} else
- retval =3D prepare_binprm (bprm);
+ retval =3D prepare_binprm(bprm);
=20
if (retval < 0)
- goto _error;
+ goto error;
=20
retval =3D search_binary_handler(bprm);
if (retval < 0)
- goto _error;
+ goto error;
=20
-_ret:
+ret:
return retval;
-_error:
+error:
if (fd_binary > 0)
sys_close(fd_binary);
bprm->interp_flags =3D 0;
bprm->interp_data =3D 0;
- goto _ret;
+ goto ret;
}
=20
/* Command parsers */
@@ -261,39 +253,40 @@ static char *scanarg(char *s, char del)
return s;
}
=20
-static char * check_special_flags (char * sfs, Node * e)
+static char *check_special_flags(char *sfs, Node *e)
{
- char * p =3D sfs;
+ char *p =3D sfs;
int cont =3D 1;
=20
/* special flags */
while (cont) {
switch (*p) {
- case 'P':
- pr_debug("register: flag: P (preserve argv0)\n");
- p++;
- e->flags |=3D MISC_FMT_PRESERVE_ARGV0;
- break;
- case 'O':
- pr_debug("register: flag: O (open binary)\n");
- p++;
- e->flags |=3D MISC_FMT_OPEN_BINARY;
- break;
- case 'C':
- pr_debug("register: flag: C (preserve creds)\n");
- p++;
- /* this flags also implies the
- open-binary flag */
- e->flags |=3D (MISC_FMT_CREDENTIALS |
- MISC_FMT_OPEN_BINARY);
- break;
- default:
- cont =3D 0;
+ case 'P':
+ pr_debug("register: flag: P (preserve argv0)\n");
+ p++;
+ e->flags |=3D MISC_FMT_PRESERVE_ARGV0;
+ break;
+ case 'O':
+ pr_debug("register: flag: O (open binary)\n");
+ p++;
+ e->flags |=3D MISC_FMT_OPEN_BINARY;
+ break;
+ case 'C':
+ pr_debug("register: flag: C (preserve creds)\n");
+ p++;
+ /* this flags also implies the
+ open-binary flag */
+ e->flags |=3D (MISC_FMT_CREDENTIALS |
+ MISC_FMT_OPEN_BINARY);
+ break;
+ default:
+ cont =3D 0;
}
}
=20
return p;
}
+
/*
* This registers a new binary format, it recognises the syntax
* ':name:type:offset:magic:mask:interpreter:flags'
@@ -323,26 +316,26 @@ static Node *create_entry(const char __user *buff=
er, size_t count)
=20
memset(e, 0, sizeof(Node));
if (copy_from_user(buf, buffer, count))
- goto Efault;
+ goto efault;
=20
del =3D *p++; /* delimeter */
=20
pr_debug("register: delim: %#x {%c}\n", del, del);
=20
/* Pad the buffer with the delim to simplify parsing below. */
- memset(buf+count, del, 8);
+ memset(buf + count, del, 8);
=20
/* Parse the 'name' field. */
e->name =3D p;
p =3D strchr(p, del);
if (!p)
- goto Einval;
+ goto einval;
*p++ =3D '\0';
if (!e->name[0] ||
!strcmp(e->name, ".") ||
!strcmp(e->name, "..") ||
strchr(e->name, '/'))
- goto Einval;
+ goto einval;
=20
pr_debug("register: name: {%s}\n", e->name);
=20
@@ -357,10 +350,10 @@ static Node *create_entry(const char __user *buff=
er, size_t count)
e->flags =3D (1 << Enabled) | (1 << Magic);
break;
default:
- goto Einval;
+ goto einval;
}
if (*p++ !=3D del)
- goto Einval;
+ goto einval;
=20
if (test_bit(Magic, &e->flags)) {
/* Handle the 'M' (magic) format. */
@@ -369,21 +362,21 @@ static Node *create_entry(const char __user *buff=
er, size_t count)
/* Parse the 'offset' field. */
s =3D strchr(p, del);
if (!s)
- goto Einval;
+ goto einval;
*s++ =3D '\0';
e->offset =3D simple_strtoul(p, &p, 10);
if (*p++)
- goto Einval;
+ goto einval;
pr_debug("register: offset: %#x\n", e->offset);
=20
/* Parse the 'magic' field. */
e->magic =3D p;
p =3D scanarg(p, del);
if (!p)
- goto Einval;
+ goto einval;
p[-1] =3D '\0';
if (p =3D=3D e->magic)
- goto Einval;
+ goto einval;
if (USE_DEBUG)
print_hex_dump_bytes(
KBUILD_MODNAME ": register: magic[raw]: ",
@@ -393,7 +386,7 @@ static Node *create_entry(const char __user *buffer=
, size_t count)
e->mask =3D p;
p =3D scanarg(p, del);
if (!p)
- goto Einval;
+ goto einval;
p[-1] =3D '\0';
if (p =3D=3D e->mask) {
e->mask =3D NULL;
@@ -412,9 +405,9 @@ static Node *create_entry(const char __user *buffer=
, size_t count)
e->size =3D string_unescape_inplace(e->magic, UNESCAPE_HEX);
if (e->mask &&
string_unescape_inplace(e->mask, UNESCAPE_HEX) !=3D e->size)
- goto Einval;
+ goto einval;
if (e->size + e->offset > BINPRM_BUF_SIZE)
- goto Einval;
+ goto einval;
pr_debug("register: magic/mask length: %i\n", e->size);
if (USE_DEBUG) {
print_hex_dump_bytes(
@@ -446,23 +439,23 @@ static Node *create_entry(const char __user *buff=
er, size_t count)
/* Skip the 'offset' field. */
p =3D strchr(p, del);
if (!p)
- goto Einval;
+ goto einval;
*p++ =3D '\0';
=20
/* Parse the 'magic' field. */
e->magic =3D p;
p =3D strchr(p, del);
if (!p)
- goto Einval;
+ goto einval;
*p++ =3D '\0';
if (!e->magic[0] || strchr(e->magic, '/'))
- goto Einval;
+ goto einval;
pr_debug("register: extension: {%s}\n", e->magic);
=20
/* Skip the 'mask' field. */
p =3D strchr(p, del);
if (!p)
- goto Einval;
+ goto einval;
*p++ =3D '\0';
}
=20
@@ -470,27 +463,28 @@ static Node *create_entry(const char __user *buff=
er, size_t count)
e->interpreter =3D p;
p =3D strchr(p, del);
if (!p)
- goto Einval;
+ goto einval;
*p++ =3D '\0';
if (!e->interpreter[0])
- goto Einval;
+ goto einval;
pr_debug("register: interpreter: {%s}\n", e->interpreter);
=20
/* Parse the 'flags' field. */
- p =3D check_special_flags (p, e);
+ p =3D check_special_flags(p, e);
if (*p =3D=3D '\n')
p++;
if (p !=3D buf + count)
- goto Einval;
+ goto einval;
+
return e;
=20
out:
return ERR_PTR(err);
=20
-Efault:
+efault:
kfree(e);
return ERR_PTR(-EFAULT);
-Einval:
+einval:
kfree(e);
return ERR_PTR(-EINVAL);
}
@@ -509,7 +503,7 @@ static int parse_command(const char __user *buffer,=
size_t count)
return -EFAULT;
if (!count)
return 0;
- if (s[count-1] =3D=3D '\n')
+ if (s[count - 1] =3D=3D '\n')
count--;
if (count =3D=3D 1 && s[0] =3D=3D '0')
return 1;
@@ -526,7 +520,7 @@ static void entry_status(Node *e, char *page)
{
char *dp;
char *status =3D "disabled";
- const char * flags =3D "flags: ";
+ const char *flags =3D "flags: ";
=20
if (test_bit(Enabled, &e->flags))
status =3D "enabled";
@@ -540,19 +534,15 @@ static void entry_status(Node *e, char *page)
dp =3D page + strlen(page);
=20
/* print the special flags */
- sprintf (dp, "%s", flags);
- dp +=3D strlen (flags);
- if (e->flags & MISC_FMT_PRESERVE_ARGV0) {
- *dp ++ =3D 'P';
- }
- if (e->flags & MISC_FMT_OPEN_BINARY) {
- *dp ++ =3D 'O';
- }
- if (e->flags & MISC_FMT_CREDENTIALS) {
- *dp ++ =3D 'C';
- }
- *dp ++ =3D '\n';
-
+ sprintf(dp, "%s", flags);
+ dp +=3D strlen(flags);
+ if (e->flags & MISC_FMT_PRESERVE_ARGV0)
+ *dp++ =3D 'P';
+ if (e->flags & MISC_FMT_OPEN_BINARY)
+ *dp++ =3D 'O';
+ if (e->flags & MISC_FMT_CREDENTIALS)
+ *dp++ =3D 'C';
+ *dp++ =3D '\n';
=20
if (!test_bit(Magic, &e->flags)) {
sprintf(dp, "extension .%s\n", e->magic);
@@ -580,7 +570,7 @@ static void entry_status(Node *e, char *page)
=20
static struct inode *bm_get_inode(struct super_block *sb, int mode)
{
- struct inode * inode =3D new_inode(sb);
+ struct inode *inode =3D new_inode(sb);
=20
if (inode) {
inode->i_ino =3D get_next_ino();
@@ -620,13 +610,14 @@ static void kill_node(Node *e)
/* /<entry> */
=20
static ssize_t
-bm_entry_read(struct file * file, char __user * buf, size_t nbytes, lo=
ff_t *ppos)
+bm_entry_read(struct file *file, char __user *buf, size_t nbytes, loff=
_t *ppos)
{
Node *e =3D file_inode(file)->i_private;
ssize_t res;
char *page;
=20
- if (!(page =3D (char*) __get_free_page(GFP_KERNEL)))
+ page =3D (char *) __get_free_page(GFP_KERNEL);
+ if (!page)
return -ENOMEM;
=20
entry_status(e, page);
@@ -645,26 +636,28 @@ static ssize_t bm_entry_write(struct file *file, =
const char __user *buffer,
int res =3D parse_command(buffer, count);
=20
switch (res) {
- case 1:
- /* Disable this handler. */
- clear_bit(Enabled, &e->flags);
- break;
- case 2:
- /* Enable this handler. */
- set_bit(Enabled, &e->flags);
- break;
- case 3:
- /* Delete this handler. */
- root =3D dget(file->f_path.dentry->d_sb->s_root);
- mutex_lock(&root->d_inode->i_mutex);
+ case 1:
+ /* Disable this handler. */
+ clear_bit(Enabled, &e->flags);
+ break;
+ case 2:
+ /* Enable this handler. */
+ set_bit(Enabled, &e->flags);
+ break;
+ case 3:
+ /* Delete this handler. */
+ root =3D dget(file->f_path.dentry->d_sb->s_root);
+ mutex_lock(&root->d_inode->i_mutex);
=20
- kill_node(e);
+ kill_node(e);
=20
- mutex_unlock(&root->d_inode->i_mutex);
- dput(root);
- break;
- default: return res;
+ mutex_unlock(&root->d_inode->i_mutex);
+ dput(root);
+ break;
+ default:
+ return res;
}
+
return count;
}
=20
@@ -752,34 +745,36 @@ bm_status_read(struct file *file, char __user *bu=
f, size_t nbytes, loff_t *ppos)
return simple_read_from_buffer(buf, nbytes, ppos, s, strlen(s));
}
=20
-static ssize_t bm_status_write(struct file * file, const char __user *=
buffer,
+static ssize_t bm_status_write(struct file *file, const char __user *b=
uffer,
size_t count, loff_t *ppos)
{
int res =3D parse_command(buffer, count);
struct dentry *root;
=20
switch (res) {
- case 1:
- /* Disable all handlers. */
- enabled =3D 0;
- break;
- case 2:
- /* Enable all handlers. */
- enabled =3D 1;
- break;
- case 3:
- /* Delete all handlers. */
- root =3D dget(file->f_path.dentry->d_sb->s_root);
- mutex_lock(&root->d_inode->i_mutex);
+ case 1:
+ /* Disable all handlers. */
+ enabled =3D 0;
+ break;
+ case 2:
+ /* Enable all handlers. */
+ enabled =3D 1;
+ break;
+ case 3:
+ /* Delete all handlers. */
+ root =3D dget(file->f_path.dentry->d_sb->s_root);
+ mutex_lock(&root->d_inode->i_mutex);
=20
- while (!list_empty(&entries))
- kill_node(list_entry(entries.next, Node, list));
+ while (!list_empty(&entries))
+ kill_node(list_entry(entries.next, Node, list));
=20
- mutex_unlock(&root->d_inode->i_mutex);
- dput(root);
- break;
- default: return res;
+ mutex_unlock(&root->d_inode->i_mutex);
+ dput(root);
+ break;
+ default:
+ return res;
}
+
return count;
}
=20
@@ -796,14 +791,16 @@ static const struct super_operations s_ops =3D {
.evict_inode =3D bm_evict_inode,
};
=20
-static int bm_fill_super(struct super_block * sb, void * data, int sil=
ent)
+static int bm_fill_super(struct super_block *sb, void *data, int silen=
t)
{
+ int err;
static struct tree_descr bm_files[] =3D {
[2] =3D {"status", &bm_status_operations, S_IWUSR|S_IRUGO},
[3] =3D {"register", &bm_register_operations, S_IWUSR},
/* last one */ {""}
};
- int err =3D simple_fill_super(sb, BINFMTFS_MAGIC, bm_files);
+
+ err =3D simple_fill_super(sb, BINFMTFS_MAGIC, bm_files);
if (!err)
sb->s_op =3D &s_ops;
return err;
--=20
2.1.2
Mike Frysinger
2014-10-20 22:45:59 UTC
Permalink
When trying to develop a custom format handler, the errors returned all
effectively get bucketed as EINVAL with no kernel messages. The other
errors (ENOMEM/EFAULT) are internal/obvious and basic. Thus any time a
bad handler is rejected, the developer has to walk the dense code and
try to guess where it went wrong. Needing to dive into kernel code is
itself a fairly high barrier for a lot of people.

To improve this situation, let's deploy extensive pr_debug markers at
logical parse points, and add comments to the dense parsing logic. It
let's you see exactly where the parsing aborts, the string the kernel
received (useful when dealing with shell code), how it translated the
buffers to binary data, and how it will apply the mask at runtime.

Some example output:
$ echo ':qemu-foo:M::\x7fELF\xAD\xAD\x01\x00:\xff\xff\xff\xff\xff\x00\xff\x00:/usr/bin/qemu-foo:POC' > register
$ dmesg
binfmt_misc: register: received 92 bytes
binfmt_misc: register: delim: 0x3a {:}
binfmt_misc: register: name: {qemu-foo}
binfmt_misc: register: type: M (magic)
binfmt_misc: register: offset: 0x0
binfmt_misc: register: magic[raw]: 5c 78 37 66 45 4c 46 5c 78 41 44 5c 78 41 44 5c \x7fELF\xAD\xAD\
binfmt_misc: register: magic[raw]: 78 30 31 5c 78 30 30 00 x01\x00.
binfmt_misc: register: mask[raw]: 5c 78 66 66 5c 78 66 66 5c 78 66 66 5c 78 66 66 \xff\xff\xff\xff
binfmt_misc: register: mask[raw]: 5c 78 66 66 5c 78 30 30 5c 78 66 66 5c 78 30 30 \xff\x00\xff\x00
binfmt_misc: register: mask[raw]: 00 .
binfmt_misc: register: magic/mask length: 8
binfmt_misc: register: magic[decoded]: 7f 45 4c 46 ad ad 01 00 .ELF....
binfmt_misc: register: mask[decoded]: ff ff ff ff ff 00 ff 00 ........
binfmt_misc: register: magic[masked]: 7f 45 4c 46 ad 00 01 00 .ELF....
binfmt_misc: register: interpreter: {/usr/bin/qemu-foo}
binfmt_misc: register: flag: P (preserve argv0)
binfmt_misc: register: flag: O (open binary)
binfmt_misc: register: flag: C (preserve creds)

The [raw] lines show us exactly what was received from userspace.
The lines after that show us how the kernel has decoded things.

Signed-off-by: Mike Frysinger <***@gentoo.org>
---
v2:
- add explicit trailing \n to all the pr_debug lines

fs/binfmt_misc.c | 136 +++++++++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 121 insertions(+), 15 deletions(-)

diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c
index fd8beb9..5670b17 100644
--- a/fs/binfmt_misc.c
+++ b/fs/binfmt_misc.c
@@ -16,6 +16,8 @@
* 2001-02-28 AV: rewritten into something that resembles C. Original didn't.
*/

+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
#include <linux/module.h>
#include <linux/init.h>
#include <linux/sched.h>
@@ -30,8 +32,13 @@
#include <linux/mount.h>
#include <linux/syscalls.h>
#include <linux/fs.h>
+#include <linux/uaccess.h>

-#include <asm/uaccess.h>
+#ifdef DEBUG
+# define USE_DEBUG 1
+#else
+# define USE_DEBUG 0
+#endif

enum {
VERBOSE_STATUS = 1 /* make it zero to save 400 bytes kernel memory */
@@ -87,20 +94,24 @@ static Node *check_file(struct linux_binprm *bprm)
char *p = strrchr(bprm->interp, '.');
struct list_head *l;

+ /* Walk all the registered handlers. */
list_for_each(l, &entries) {
Node *e = list_entry(l, Node, list);
char *s;
int j;

+ /* Make sure this one is currently enabled. */
if (!test_bit(Enabled, &e->flags))
continue;

+ /* Do matching based on extension if applicable. */
if (!test_bit(Magic, &e->flags)) {
if (p && !strcmp(e->magic, p + 1))
return e;
continue;
}

+ /* Do matching based on magic & mask. */
s = bprm->buf + e->offset;
if (e->mask) {
for (j = 0; j < e->size; j++)
@@ -259,14 +270,17 @@ static char * check_special_flags (char * sfs, Node * e)
while (cont) {
switch (*p) {
case 'P':
+ pr_debug("register: flag: P (preserve argv0)\n");
p++;
e->flags |= MISC_FMT_PRESERVE_ARGV0;
break;
case 'O':
+ pr_debug("register: flag: O (open binary)\n");
p++;
e->flags |= MISC_FMT_OPEN_BINARY;
break;
case 'C':
+ pr_debug("register: flag: C (preserve creds)\n");
p++;
/* this flags also implies the
open-binary flag */
@@ -292,6 +306,8 @@ static Node *create_entry(const char __user *buffer, size_t count)
char *buf, *p;
char del;

+ pr_debug("register: received %zu bytes\n", count);
+
/* some sanity checks */
err = -EINVAL;
if ((count < 11) || (count > MAX_REGISTER_LENGTH))
@@ -311,8 +327,12 @@ static Node *create_entry(const char __user *buffer, size_t count)

del = *p++; /* delimeter */

+ pr_debug("register: delim: %#x {%c}\n", del, del);
+
+ /* Pad the buffer with the delim to simplify parsing below. */
memset(buf+count, del, 8);

+ /* Parse the 'name' field. */
e->name = p;
p = strchr(p, del);
if (!p)
@@ -323,46 +343,113 @@ static Node *create_entry(const char __user *buffer, size_t count)
!strcmp(e->name, "..") ||
strchr(e->name, '/'))
goto Einval;
+
+ pr_debug("register: name: {%s}\n", e->name);
+
+ /* Parse the 'type' field. */
switch (*p++) {
- case 'E': e->flags = 1<<Enabled; break;
- case 'M': e->flags = (1<<Enabled) | (1<<Magic); break;
- default: goto Einval;
+ case 'E':
+ pr_debug("register: type: E (extension)\n");
+ e->flags = 1 << Enabled;
+ break;
+ case 'M':
+ pr_debug("register: type: M (magic)\n");
+ e->flags = (1 << Enabled) | (1 << Magic);
+ break;
+ default:
+ goto Einval;
}
if (*p++ != del)
goto Einval;
+
if (test_bit(Magic, &e->flags)) {
- char *s = strchr(p, del);
+ /* Handle the 'M' (magic) format. */
+ char *s;
+
+ /* Parse the 'offset' field. */
+ s = strchr(p, del);
if (!s)
goto Einval;
*s++ = '\0';
e->offset = simple_strtoul(p, &p, 10);
if (*p++)
goto Einval;
+ pr_debug("register: offset: %#x\n", e->offset);
+
+ /* Parse the 'magic' field. */
e->magic = p;
p = scanarg(p, del);
if (!p)
goto Einval;
p[-1] = '\0';
- if (!e->magic[0])
+ if (p == e->magic)
goto Einval;
+ if (USE_DEBUG)
+ print_hex_dump_bytes(
+ KBUILD_MODNAME ": register: magic[raw]: ",
+ DUMP_PREFIX_NONE, e->magic, p - e->magic);
+
+ /* Parse the 'mask' field. */
e->mask = p;
p = scanarg(p, del);
if (!p)
goto Einval;
p[-1] = '\0';
- if (!e->mask[0])
+ if (p == e->mask) {
e->mask = NULL;
+ pr_debug("register: mask[raw]: none\n");
+ } else if (USE_DEBUG)
+ print_hex_dump_bytes(
+ KBUILD_MODNAME ": register: mask[raw]: ",
+ DUMP_PREFIX_NONE, e->mask, p - e->mask);
+
+ /*
+ * Decode the magic & mask fields.
+ * Note: while we might have accepted embedded NUL bytes from
+ * above, the unescape helpers here will stop at the first one
+ * it encounters.
+ */
e->size = string_unescape_inplace(e->magic, UNESCAPE_HEX);
if (e->mask &&
string_unescape_inplace(e->mask, UNESCAPE_HEX) != e->size)
goto Einval;
if (e->size + e->offset > BINPRM_BUF_SIZE)
goto Einval;
+ pr_debug("register: magic/mask length: %i\n", e->size);
+ if (USE_DEBUG) {
+ print_hex_dump_bytes(
+ KBUILD_MODNAME ": register: magic[decoded]: ",
+ DUMP_PREFIX_NONE, e->magic, e->size);
+
+ if (e->mask) {
+ int i;
+ char *masked = kmalloc(e->size, GFP_USER);
+
+ print_hex_dump_bytes(
+ KBUILD_MODNAME ": register: mask[decoded]: ",
+ DUMP_PREFIX_NONE, e->mask, e->size);
+
+ if (masked) {
+ for (i = 0; i < e->size; ++i)
+ masked[i] = e->magic[i] & e->mask[i];
+ print_hex_dump_bytes(
+ KBUILD_MODNAME ": register: magic[masked]: ",
+ DUMP_PREFIX_NONE, masked, e->size);
+
+ kfree(masked);
+ }
+ }
+ }
} else {
+ /* Handle the 'E' (extension) format. */
+
+ /* Skip the 'offset' field. */
p = strchr(p, del);
if (!p)
goto Einval;
*p++ = '\0';
+
+ /* Parse the 'magic' field. */
e->magic = p;
p = strchr(p, del);
if (!p)
@@ -370,11 +457,16 @@ static Node *create_entry(const char __user *buffer, size_t count)
*p++ = '\0';
if (!e->magic[0] || strchr(e->magic, '/'))
goto Einval;
+ pr_debug("register: extension: {%s}\n", e->magic);
+
+ /* Skip the 'mask' field. */
p = strchr(p, del);
if (!p)
goto Einval;
*p++ = '\0';
}
+
+ /* Parse the 'interpreter' field. */
e->interpreter = p;
p = strchr(p, del);
if (!p)
@@ -382,10 +474,10 @@ static Node *create_entry(const char __user *buffer, size_t count)
*p++ = '\0';
if (!e->interpreter[0])
goto Einval;
+ pr_debug("register: interpreter: {%s}\n", e->interpreter);

-
+ /* Parse the 'flags' field. */
p = check_special_flags (p, e);
-
if (*p == '\n')
p++;
if (p != buf + count)
@@ -553,11 +645,17 @@ static ssize_t bm_entry_write(struct file *file, const char __user *buffer,
int res = parse_command(buffer, count);

switch (res) {
- case 1: clear_bit(Enabled, &e->flags);
+ case 1:
+ /* Disable this handler. */
+ clear_bit(Enabled, &e->flags);
break;
- case 2: set_bit(Enabled, &e->flags);
+ case 2:
+ /* Enable this handler. */
+ set_bit(Enabled, &e->flags);
break;
- case 3: root = dget(file->f_path.dentry->d_sb->s_root);
+ case 3:
+ /* Delete this handler. */
+ root = dget(file->f_path.dentry->d_sb->s_root);
mutex_lock(&root->d_inode->i_mutex);

kill_node(e);
@@ -661,9 +759,17 @@ static ssize_t bm_status_write(struct file * file, const char __user * buffer,
struct dentry *root;

switch (res) {
- case 1: enabled = 0; break;
- case 2: enabled = 1; break;
- case 3: root = dget(file->f_path.dentry->d_sb->s_root);
+ case 1:
+ /* Disable all handlers. */
+ enabled = 0;
+ break;
+ case 2:
+ /* Enable all handlers. */
+ enabled = 1;
+ break;
+ case 3:
+ /* Delete all handlers. */
+ root = dget(file->f_path.dentry->d_sb->s_root);
mutex_lock(&root->d_inode->i_mutex);

while (!list_empty(&entries))
--
2.1.2
Joe Perches
2014-10-20 22:59:56 UTC
Permalink
let's deploy extensive pr_debug markers at
logical parse points, and add comments to the dense parsing logic. It
let's you see exactly where the parsing aborts, the string the kernel
received (useful when dealing with shell code), how it translated the
buffers to binary data, and how it will apply the mask at runtime.
diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c
[]
@@ -323,46 +343,113 @@ static Node *create_entry(const char __user *buffer, size_t count)
[]
+ if (e->mask) {
+ int i;
+ char *masked = kmalloc(e->size, GFP_USER);
Why GFP_USER? Does it need it?
+ print_hex_dump_bytes(
+ KBUILD_MODNAME ": register: mask[decoded]: ",
+ DUMP_PREFIX_NONE, e->mask, e->size);
+
+ if (masked) {
+ for (i = 0; i < e->size; ++i)
+ masked[i] = e->magic[i] & e->mask[i];
+ print_hex_dump_bytes(
+ KBUILD_MODNAME ": register: magic[masked]: ",
+ DUMP_PREFIX_NONE, masked, e->size);
+
+ kfree(masked);
[]
@@ -553,11 +645,17 @@ static ssize_t bm_entry_write(struct file *file, const char __user *buffer,
int res = parse_command(buffer, count);
switch (res) {
- case 1: clear_bit(Enabled, &e->flags);
+ /* Disable this handler. */
+ clear_bit(Enabled, &e->flags);
break;
- case 2: set_bit(Enabled, &e->flags);
+ /* Enable this handler. */
+ set_bit(Enabled, &e->flags);
break;
- case 3: root = dget(file->f_path.dentry->d_sb->s_root);
+ /* Delete this handler. */
+ root = dget(file->f_path.dentry->d_sb->s_root);
mutex_lock(&root->d_inode->i_mutex);
kill_node(e);
Maybe move the case indents one tab position left

switch (res) {
case 1: /* Disable handler */
clear_bit(Enabled, ...);
break;
case 2: /* Enable handler */
set_bit(...);
break;
case 3: /* Delete handler */
etc...
}
@@ -661,9 +759,17 @@ static ssize_t bm_status_write(struct file * file, const char __user * buffer,
struct dentry *root;
switch (res) {
- case 1: enabled = 0; break;
- case 2: enabled = 1; break;
- case 3: root = dget(file->f_path.dentry->d_sb->s_root);
+ /* Disable all handlers. */
+ enabled = 0;
+ break;
+ /* Enable all handlers. */
+ enabled = 1;
+ break;
+ /* Delete all handlers. */
+ root = dget(file->f_path.dentry->d_sb->s_root);
mutex_lock(&root->d_inode->i_mutex);
while (!list_empty(&entries))
here too.
Mike Frysinger
2014-10-20 23:54:14 UTC
Permalink
Post by Joe Perches
Post by Mike Frysinger
diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c
[]
Post by Mike Frysinger
@@ -323,46 +343,113 @@ static Node *create_entry(const char __user *buffer, size_t count)
[]
Post by Mike Frysinger
+ if (e->mask) {
+ int i;
+ char *masked = kmalloc(e->size, GFP_USER);
Why GFP_USER? Does it need it?
mostly a copy & paste from earlier in this func:
e = kmalloc(memsize, GFP_USER);

the code is running process context and this buffer is only for
debugging on behalf of the user (and is shortly freed there after), so
GFP_USER seemed appropriate. that said, i'm certainly not an expert
here, so if the convention is to use GFP_KERNEL, it's easy enough to
change. the kmalloc API doesn't seem to provide guidance.
Post by Joe Perches
Post by Mike Frysinger
switch (res) {
- case 1: clear_bit(Enabled, &e->flags);
+ /* Disable this handler. */
+ clear_bit(Enabled, &e->flags);
break;
- case 2: set_bit(Enabled, &e->flags);
+ /* Enable this handler. */
+ set_bit(Enabled, &e->flags);
break;
- case 3: root = dget(file->f_path.dentry->d_sb->s_root);
+ /* Delete this handler. */
+ root = dget(file->f_path.dentry->d_sb->s_root);
mutex_lock(&root->d_inode->i_mutex);
kill_node(e);
Maybe move the case indents one tab position left
style fixes (for the whole file) are in the 2nd patch. i specifically
tried to keep those changes to a minimum in this patch (functionality
vs formatting).
-mike

Loading...