proc: add "personality" to process status file

PATCH Applied Kees Cook 6 participants 16 messages Oct 2, 2008 – Oct 8, 2008

Subsystems (3)

Scheduler Proc filesystem ABI/API

lore

Share & tools

lore

Kees Cook

author

2008-10-02 21:51 UTC #0 lore raw

PATCH 1/1 proc: add "personality" to process status file

There is no sane way to query the personality flags of arbitrary process from userspace. This adds the flags to the /proc/$pid/status file. For example, to detect READ_IMPLIES_EXEC:

$ ./32bit-sleeper &
[1] 6732
$ grep ^Pers /proc/6732/status
Personality:    00000000
$ execstack -s 32bit-sleeper
$ ./32bit-slepper &
[2] 6735
$ grep ^Pers /proc/6735/status
Personality:    00040000

Signed-off-by Kees Cook <[email protected]>

---
 fs/proc/array.c       |    6 ++++--
 include/linux/sched.h |    3 +--
 2 files changed, 5 insertions(+), 4 deletions(-)

fs/proc/array.c +4 -2

index 71c9be5..1235fab 100644
@@ -182,13 +182,15 @@ static inline void task_state(struct seq_file *m, struct pid_namespace *ns,
 		"PPid:\t%d\n"
 		"TracerPid:\t%d\n"
 		"Uid:\t%d\t%d\t%d\t%d\n"
-		"Gid:\t%d\t%d\t%d\t%d\n",
+		"Gid:\t%d\t%d\t%d\t%d\n"
+		"Personality:\t%08x\n",
 		get_task_state(p),
 		task_tgid_nr_ns(p, ns),
 		pid_nr_ns(pid, ns),
 		ppid, tpid,
 		p->uid, p->euid, p->suid, p->fsuid,
-		p->gid, p->egid, p->sgid, p->fsgid);
+		p->gid, p->egid, p->sgid, p->fsgid,
+		p->personality);
 
 	task_lock(p);
 	if (p->files)

include/linux/sched.h +1 -2

index 3d9120c..f2fc441 100644
@@ -1085,8 +1085,7 @@ struct task_struct {
 	int exit_state;
 	int exit_code, exit_signal;
 	int pdeath_signal;  /*  The signal sent when the parent dies  */
-	/* ??? */
-	unsigned int personality;
+	unsigned int personality; /* Special-case bits (linux/personality.h) */
 	unsigned did_exec:1;
 	pid_t pid;
 	pid_t tgid;

Signature · 6 lines

1.5.6.3


-- 
Kees Cook
Ubuntu Security Team

Randy.Dunlap

2008-10-02 22:46 UTC #1 lore raw

Quoted 14 lines · Kees Cook

There is no sane way to query the personality flags of arbitrary process from userspace. This adds the flags to the /proc/$pid/status file. For example, to detect READ_IMPLIES_EXEC:
$ ./32bit-sleeper &
[1] 6732
$ grep ^Pers /proc/6732/status
Personality:    00000000
$ execstack -s 32bit-sleeper
$ ./32bit-slepper &
[2] 6735
$ grep ^Pers /proc/6735/status
Personality:    00040000

Quoted 5 lines

Signed-off-by Kees Cook <[email protected]>

---
 fs/proc/array.c       |    6 ++++--
 include/linux/sched.h |    3 +--
 2 files changed, 5 insertions(+), 4 deletions(-)

Hey you,

The patch seems to be missing an update to Documentation/filesystems/proc.txt ! Please add that. TIA.

Quoted 37 lines

fs/proc/array.c +4 -2

index 71c9be5..1235fab 100644
@@ -182,13 +182,15 @@ static inline void task_state(struct seq_file *m, struct pid_namespace *ns,
 		"PPid:\t%d\n"
 		"TracerPid:\t%d\n"
 		"Uid:\t%d\t%d\t%d\t%d\n"
-		"Gid:\t%d\t%d\t%d\t%d\n",
+		"Gid:\t%d\t%d\t%d\t%d\n"
+		"Personality:\t%08x\n",
 		get_task_state(p),
 		task_tgid_nr_ns(p, ns),
 		pid_nr_ns(pid, ns),
 		ppid, tpid,
 		p->uid, p->euid, p->suid, p->fsuid,
-		p->gid, p->egid, p->sgid, p->fsgid);
+		p->gid, p->egid, p->sgid, p->fsgid,
+		p->personality);
 
 	task_lock(p);
 	if (p->files)

include/linux/sched.h +1 -2

index 3d9120c..f2fc441 100644
@@ -1085,8 +1085,7 @@ struct task_struct {
 	int exit_state;
 	int exit_code, exit_signal;
 	int pdeath_signal;  /*  The signal sent when the parent dies  */
-	/* ??? */
-	unsigned int personality;
+	unsigned int personality; /* Special-case bits (linux/personality.h) */
 	unsigned did_exec:1;
 	pid_t pid;
 	pid_t tgid;

Signature · 1 line

~Randy

Kees Cook

author

2008-10-02 22:56 UTC #2 lore raw

Heya Randy,

Quoted 6 lines · Randy.Dunlap

Quoted 2 lines · Kees Cook
There is no sane way to query the personality flags of arbitrary process from userspace. This adds the flags to the /proc/$pid/status file.
The patch seems to be missing an update to Documentation/filesystems/proc.txt ! Please add that. TIA.

The /proc/pid/status file is "self documenting" (in theory), and doesn't have separate documentation in proc.txt yet. If I had touched the "stat" file, I would have included and update, since I was the last to document that file. ;)

-Kees

Signature · 2 lines

Kees Cook
Ubuntu Security Team

Alexey Dobriyan

Applied

2008-10-04 21:38 UTC #3 lore raw

In reply to Kees Cook

Quoted 4 lines · Kees Cook

There is no sane way to query the personality flags of arbitrary process from userspace. This adds the flags to the /proc/$pid/status file. For example, to detect READ_IMPLIES_EXEC:

Quoted 9 lines

$ ./32bit-sleeper &
[1] 6732
$ grep ^Pers /proc/6732/status
Personality:    00000000
$ execstack -s 32bit-sleeper
$ ./32bit-slepper &
[2] 6735
$ grep ^Pers /proc/6735/status
Personality:    00040000

Applied to proc.git without unrelated sched.h commentary :-)

Arjan van de Ven

2008-10-04 21:52 UTC #4 lore raw

In reply to Kees Cook

Quoted 4 lines · On Thu, 2 Oct 2008 14:14:24 -0700 Kees Cook <[email protected]> wrote:

There is no sane way to query the personality flags of arbitrary process from userspace. This adds the flags to the /proc/$pid/status file. For example, to detect READ_IMPLIES_EXEC:

.. I'm sure local exploit writers will love this to find out which processes to attack.
Realistically, this probably shouldn't be in a world-readable file.

Signature · 3 lines

Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Kees Cook

author

2008-10-04 22:38 UTC #5 lore raw

Quoted 11 lines · Arjan van de Ven

Quoted 4 lines · On Thu, 2 Oct 2008 14:14:24 -0700 Kees Cook <[email protected]> wrote:
There is no sane way to query the personality flags of arbitrary process from userspace. This adds the flags to the /proc/$pid/status file. For example, to detect READ_IMPLIES_EXEC:
.. I'm sure local exploit writers will love this to find out which processes to attack.
Realistically, this probably shouldn't be in a world-readable file.

Nothing else seemed appropriate, but I could make a brand new proc file, if that's sensible. "status_private" ?

-Kees

Signature · 2 lines

Kees Cook
Ubuntu Security Team

Arjan van de Ven

2008-10-04 23:43 UTC #6 lore raw

Quoted 10 lines · On Sat, 4 Oct 2008 15:02:20 -0700 Kees Cook <[email protected]> wrote:

Quoted 9 lines · Arjan van de Ven
Quoted 5 lines · On Thu, 2 Oct 2008 14:14:24 -0700 Kees Cook <[email protected]> wrote:
There is no sane way to query the personality flags of arbitrary process from userspace. This adds the flags to
the /proc/$pid/status file. For example, to detect READ_IMPLIES_EXEC:

Quoted 6 lines

Quoted 3 lines
.. I'm sure local exploit writers will love this to find out which processes to attack.
Realistically, this probably shouldn't be in a world-readable file.
Nothing else seemed appropriate, but I could make a brand new proc file, if that's sensible. "status_private" ?

how about a file called "personality" that has each set bit as an ascii version ?
(one per line maybe)

Signature · 3 lines

Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Kees Cook

author

2008-10-05 00:42 UTC #7 lore raw

Quoted 8 lines · Arjan van de Ven

Quoted 2 lines · On Sat, 4 Oct 2008 15:02:20 -0700 Kees Cook <[email protected]> wrote:
Nothing else seemed appropriate, but I could make a brand new proc file, if that's sensible. "status_private" ?
how about a file called "personality" that has each set bit as an ascii version ?
(one per line maybe)

Well, it's a one-to-many due to how the personality types are defined, so doing a bitfield-to-ASCII-names conversion wouldn't really work out:

        PER_SCOSVR3 =           0x0003 | STICKY_TIMEOUTS |
                                         WHOLE_SECONDS | SHORT_INODE,
        PER_OSR5 =              0x0003 | STICKY_TIMEOUTS | WHOLE_SECONDS,
...
        PER_BSD =               0x0006,
        PER_SUNOS =             0x0006 | STICKY_TIMEOUTS,

But I can certainly just make it a stand-alone file with just the bitfield. Or, I can still do the conversion and ignore PER_OSR5 and PER_SUNOS.

-Kees

Signature · 2 lines

Kees Cook
Ubuntu Security Team

Arjan van de Ven

2008-10-05 00:48 UTC #8 lore raw

Quoted 21 lines · On Sat, 4 Oct 2008 17:42:33 -0700 Kees Cook <[email protected]> wrote:

Quoted 8 lines · Arjan van de Ven
Quoted 2 lines · On Sat, 4 Oct 2008 15:02:20 -0700 Kees Cook <[email protected]> wrote:
Nothing else seemed appropriate, but I could make a brand new proc file, if that's sensible. "status_private" ?
how about a file called "personality" that has each set bit as an ascii version ?
(one per line maybe)
Well, it's a one-to-many due to how the personality types are defined, so doing a bitfield-to-ASCII-names conversion wouldn't really work out:
        PER_SCOSVR3 =           0x0003 | STICKY_TIMEOUTS |
                                         WHOLE_SECONDS | SHORT_INODE,
        PER_OSR5 =              0x0003 | STICKY_TIMEOUTS |
WHOLE_SECONDS, ...
        PER_BSD =               0x0006,
        PER_SUNOS =             0x0006 | STICKY_TIMEOUTS,

Quoted 3 lines

But I can certainly just make it a stand-alone file with just the bitfield. Or, I can still do the conversion and ignore PER_OSR5 and PER_SUNOS.

well.. if "you" as kernel can't really make out what it is, how is poor userspace supposed to do it ?

Signature · 3 lines

Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Alexey Dobriyan

2008-10-05 09:08 UTC #9 lore raw

Quoted 24 lines · Arjan van de Ven

Quoted 21 lines · On Sat, 4 Oct 2008 17:42:33 -0700 Kees Cook <[email protected]> wrote:
Quoted 8 lines · Arjan van de Ven
Quoted 2 lines · On Sat, 4 Oct 2008 15:02:20 -0700 Kees Cook <[email protected]> wrote:
Nothing else seemed appropriate, but I could make a brand new proc file, if that's sensible. "status_private" ?
how about a file called "personality" that has each set bit as an ascii version ?
(one per line maybe)
Well, it's a one-to-many due to how the personality types are defined, so doing a bitfield-to-ASCII-names conversion wouldn't really work out:
        PER_SCOSVR3 =           0x0003 | STICKY_TIMEOUTS |
                                         WHOLE_SECONDS | SHORT_INODE,
        PER_OSR5 =              0x0003 | STICKY_TIMEOUTS |
WHOLE_SECONDS, ...
        PER_BSD =               0x0006,
        PER_SUNOS =             0x0006 | STICKY_TIMEOUTS,

Quoted 6 lines

Quoted 3 lines
But I can certainly just make it a stand-alone file with just the bitfield. Or, I can still do the conversion and ignore PER_OSR5 and PER_SUNOS.
well.. if "you" as kernel can't really make out what it is, how is poor userspace supposed to do it ?

Kernel should just print with %lx and leave pretty-printing to luserspace.

And name should be /proc/*/personality (obviously :-)

Assuming anybody cares about personalities at all.

Kees Cook

author

2008-10-05 10:07 UTC #10 lore raw

Quoted 29 lines · Alexey Dobriyan

Quoted 28 lines · Arjan van de Ven
Quoted 24 lines · On Sat, 4 Oct 2008 17:42:33 -0700 Kees Cook <[email protected]> wrote:
Quoted 8 lines · Arjan van de Ven
Quoted 2 lines · On Sat, 4 Oct 2008 15:02:20 -0700 Kees Cook <[email protected]> wrote:
Nothing else seemed appropriate, but I could make a brand new proc file, if that's sensible. "status_private" ?
how about a file called "personality" that has each set bit as an ascii version ?
(one per line maybe)
Well, it's a one-to-many due to how the personality types are defined, so doing a bitfield-to-ASCII-names conversion wouldn't really work out:
        PER_SCOSVR3 =           0x0003 | STICKY_TIMEOUTS |
                                         WHOLE_SECONDS | SHORT_INODE,
        PER_OSR5 =              0x0003 | STICKY_TIMEOUTS |
WHOLE_SECONDS, ...
        PER_BSD =               0x0006,
        PER_SUNOS =             0x0006 | STICKY_TIMEOUTS,
But I can certainly just make it a stand-alone file with just the bitfield. Or, I can still do the conversion and ignore PER_OSR5 and PER_SUNOS.

Quoted 5 lines

Quoted 2 lines
well.. if "you" as kernel can't really make out what it is, how is poor userspace supposed to do it ?
Kernel should just print with %lx and leave pretty-printing to luserspace.

I would tend to agree with this -- I don't want to fill the kernel with string-dumping case statements. (Though I have a different patch that does this, if anyone actually wants it.)

Quoted 3 lines

And name should be /proc/*/personality (obviously :-)
Assuming anybody cares about personalities at all.

I do. :)

Signature · 2 lines

Kees Cook
Ubuntu Security Team

Kees Cook

author

2008-10-05 10:15 UTC #11 lore raw

PATCH v2 1/1 proc: show personality via /proc/pid/personality

In reply to Alexey Dobriyan

Make process personality flags visible in /proc. Since a process's personality is potentially sensitive (e.g. READ_IMPLIES_EXEC), make this file only readable by the process owner.

Signed-off-by Kees Cook <[email protected]>

---
Please revert the prior patch against the "status" file -- this is the alternative.

---
 fs/proc/array.c    |    8 ++++++++
 fs/proc/base.c     |    2 ++
 fs/proc/internal.h |    2 ++
 3 files changed, 12 insertions(+), 0 deletions(-)

fs/proc/array.c +8 -0

index 71c9be5..6b6b492 100644
@@ -337,6 +337,14 @@ int proc_pid_status(struct seq_file *m, struct pid_namespace *ns,
 	return 0;
 }
 
+int proc_pid_personality(struct seq_file *m, struct pid_namespace *ns,
+			struct pid *pid, struct task_struct *task)
+{
+	seq_printf(m, "%08x\n", task->personality);
+
+	return 0;
+}
+
 static int do_task_stat(struct seq_file *m, struct pid_namespace *ns,
 			struct pid *pid, struct task_struct *task, int whole)
 {

fs/proc/base.c +2 -0

index a28840b..c675c62 100644
@@ -2459,6 +2459,7 @@ static const struct pid_entry tgid_base_stuff[] = {
 	REG("environ",    S_IRUSR, environ),
 	INF("auxv",       S_IRUSR, pid_auxv),
 	ONE("status",     S_IRUGO, pid_status),
+	ONE("personality", S_IRUSR, pid_personality),
 	INF("limits",	  S_IRUSR, pid_limits),
 #ifdef CONFIG_SCHED_DEBUG
 	REG("sched",      S_IRUGO|S_IWUSR, pid_sched),

@@ -2794,6 +2795,7 @@ static const struct pid_entry tid_base_stuff[] = {
 	REG("environ",   S_IRUSR, environ),
 	INF("auxv",      S_IRUSR, pid_auxv),
 	ONE("status",    S_IRUGO, pid_status),
+	ONE("personality", S_IRUSR, pid_personality),
 	INF("limits",	 S_IRUSR, pid_limits),
 #ifdef CONFIG_SCHED_DEBUG
 	REG("sched",     S_IRUGO|S_IWUSR, pid_sched),

fs/proc/internal.h +2 -0

index 4422023..747e3de 100644
@@ -53,6 +53,8 @@ extern int proc_tgid_stat(struct seq_file *m, struct pid_namespace *ns,
 				struct pid *pid, struct task_struct *task);
 extern int proc_pid_status(struct seq_file *m, struct pid_namespace *ns,
 				struct pid *pid, struct task_struct *task);
+extern int proc_pid_personality(struct seq_file *m, struct pid_namespace *ns,
+				struct pid *pid, struct task_struct *task);
 extern int proc_pid_statm(struct seq_file *m, struct pid_namespace *ns,
 				struct pid *pid, struct task_struct *task);
 extern loff_t mem_lseek(struct file *file, loff_t offset, int orig);

Signature · 6 lines

1.5.6.3


-- 
Kees Cook
Ubuntu Security Team

Alexey Dobriyan

Applied

2008-10-05 23:18 UTC #12 lore raw

Re: [PATCH v2] proc: show personality via /proc/pid/personality

Quoted 3 lines · Kees Cook

Make process personality flags visible in /proc. Since a process's personality is potentially sensitive (e.g. READ_IMPLIES_EXEC), make this file only readable by the process owner.

Applied to proc.git, and I moved show hook to fs/proc/base.c where most of this irregular stuff lives.

Quoted 2 lines

Please revert the prior patch against the "status" file -- this is the alternative.

Sure.

Michael Kerrisk

2008-10-07 13:39 UTC #13 lore raw

Re: [PATCH v2] proc: show personality via /proc/pid/personality

In reply to Kees Cook

Kees,

Quoted 3 lines · Kees Cook

Make process personality flags visible in /proc. Since a process's personality is potentially sensitive (e.g. READ_IMPLIES_EXEC), make this file only readable by the process owner.

Please CC userland interface changes to [email protected]

Ceers,

Michael

Quoted 75 lines

Signed-off-by Kees Cook <[email protected]>

---
Please revert the prior patch against the "status" file -- this is the alternative.

---
 fs/proc/array.c    |    8 ++++++++
 fs/proc/base.c     |    2 ++
 fs/proc/internal.h |    2 ++
 3 files changed, 12 insertions(+), 0 deletions(-)

fs/proc/array.c +8 -0

index 71c9be5..6b6b492 100644
@@ -337,6 +337,14 @@ int proc_pid_status(struct seq_file *m, struct pid_namespace *ns,
       return 0;
 }
 
+int proc_pid_personality(struct seq_file *m, struct pid_namespace *ns,
+                       struct pid *pid, struct task_struct *task)
+{
+       seq_printf(m, "%08x\n", task->personality);
+
+       return 0;
+}
+
 static int do_task_stat(struct seq_file *m, struct pid_namespace *ns,
                       struct pid *pid, struct task_struct *task, int whole)
 {

fs/proc/base.c +2 -0

index a28840b..c675c62 100644
@@ -2459,6 +2459,7 @@ static const struct pid_entry tgid_base_stuff[] = {
       REG("environ",    S_IRUSR, environ),
       INF("auxv",       S_IRUSR, pid_auxv),
       ONE("status",     S_IRUGO, pid_status),
+       ONE("personality", S_IRUSR, pid_personality),
       INF("limits",     S_IRUSR, pid_limits),
 #ifdef CONFIG_SCHED_DEBUG
       REG("sched",      S_IRUGO|S_IWUSR, pid_sched),

@@ -2794,6 +2795,7 @@ static const struct pid_entry tid_base_stuff[] = {
       REG("environ",   S_IRUSR, environ),
       INF("auxv",      S_IRUSR, pid_auxv),
       ONE("status",    S_IRUGO, pid_status),
+       ONE("personality", S_IRUSR, pid_personality),
       INF("limits",    S_IRUSR, pid_limits),
 #ifdef CONFIG_SCHED_DEBUG
       REG("sched",     S_IRUGO|S_IWUSR, pid_sched),

fs/proc/internal.h +2 -1

index 4422023..747e3de 100644
@@ -53,6 +53,8 @@ extern int proc_tgid_stat(struct seq_file *m, struct pid_namespace *ns,
                               struct pid *pid, struct task_struct *task);
 extern int proc_pid_status(struct seq_file *m, struct pid_namespace *ns,
                               struct pid *pid, struct task_struct *task);
+extern int proc_pid_personality(struct seq_file *m, struct pid_namespace *ns,
+                               struct pid *pid, struct task_struct *task);
 extern int proc_pid_statm(struct seq_file *m, struct pid_namespace *ns,
                               struct pid *pid, struct task_struct *task);
 extern loff_t mem_lseek(struct file *file, loff_t offset, int orig);
--

1.5.6.3

--
Kees Cook
Ubuntu Security Team
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Signature · 7 lines

Michael Kerrisk Linux man-pages maintainer;
http://www.kernel.org/doc/man-pages/ Found a documentation bug?
http://www.kernel.org/doc/man-pages/reporting_bugs.html
--
To unsubscribe from this list: send the line "unsubscribe linux-api" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Kees Cook

author

2008-10-07 16:14 UTC #14 lore raw

Re: [PATCH v2] proc: show personality via /proc/pid/personality

Hi Michael,

Quoted 6 lines · Michael Kerrisk

Quoted 3 lines · Kees Cook
Make process personality flags visible in /proc. Since a process's personality is potentially sensitive (e.g. READ_IMPLIES_EXEC), make this file only readable by the process owner.
Please CC userland interface changes to [email protected]

Sure, I'd be happy to do that for future stuff. I don't see this email address mentioned anywhere in the kernel tree. It seems like this is useful information that should go in either MAINTAINERS or better yet Documentation/SubmittingPatches for it to be discoverable by future patch-senders -- and somewhere that I can look it up easily later. :)

-Kees

Signature · 2 lines

Kees Cook
Ubuntu Security Team

Michael Kerrisk

2008-10-08 02:45 UTC #15 lore raw

Re: [PATCH v2] proc: show personality via /proc/pid/personality

Hi Kees,

Quoted 8 lines · Kees Cook

Hi Michael,
Quoted 5 lines · Michael Kerrisk
Quoted 3 lines · Kees Cook
Make process personality flags visible in /proc. Since a process's personality is potentially sensitive (e.g. READ_IMPLIES_EXEC), make this file only readable by the process owner.

Quoted 7 lines

Quoted 1 line
Please CC userland interface changes to [email protected]
Sure, I'd be happy to do that for future stuff. I don't see this email address mentioned anywhere in the kernel tree. It seems like this is useful information that should go in either MAINTAINERS or better yet Documentation/SubmittingPatches for it to be discoverable by future patch-senders -- and somewhere that I can look it up easily later. :)

It's there, but only as of a few days ago.

Signature · 9 lines

Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
git://git.kernel.org/pub/scm/docs/man-pages/man-pages.git
man-pages online: http://www.kernel.org/doc/man-pages/online_pages.html
Found a bug? http://www.kernel.org/doc/man-pages/reporting_bugs.html
--
To unsubscribe from this list: send the line "unsubscribe linux-api" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html