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(-)
diff --git a/fs/proc/array.c b/fs/proc/array.c
index 71c9be5..1235fab 100644
--- a/fs/proc/array.c
+++ b/fs/proc/array.c
@@ -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)
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 3d9120c..f2fc441 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -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;
--
1.5.6.3
--
Kees Cook
Ubuntu Security Team
On Thu, 2 Oct 2008, Kees Cook 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:
>
> $ ./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(-)
Hey you,
The patch seems to be missing an update to Documentation/filesystems/proc.txt !
Please add that. TIA.
> diff --git a/fs/proc/array.c b/fs/proc/array.c
> index 71c9be5..1235fab 100644
> --- a/fs/proc/array.c
> +++ b/fs/proc/array.c
> @@ -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)
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index 3d9120c..f2fc441 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -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;
>
--
~Randy
Heya Randy,
On Thu, Oct 02, 2008 at 03:46:31PM -0700, Randy.Dunlap wrote:
> On Thu, 2 Oct 2008, Kees Cook 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.
>
> 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
--
Kees Cook
Ubuntu Security Team
On Thu, Oct 02, 2008 at 02:14:24PM -0700, Kees Cook 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:
>
> $ ./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 :-)
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.
--
Arjan van de Ven Intel Open Source Technology Centre
For development, discussion and tips for power savings,
visit http://www.lesswatts.org
On Sat, Oct 04, 2008 at 02:51:49PM -0700, Arjan van de Ven wrote:
> 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
--
Kees Cook
Ubuntu Security Team
On Sat, 4 Oct 2008 15:02:20 -0700
Kees Cook <[email protected]> wrote:
> On Sat, Oct 04, 2008 at 02:51:49PM -0700, Arjan van de Ven wrote:
> > 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" ?
how about a file called "personality" that has each set bit as an ascii
version ?
(one per line maybe)
--
Arjan van de Ven Intel Open Source Technology Centre
For development, discussion and tips for power savings,
visit http://www.lesswatts.org
On Sat, Oct 04, 2008 at 04:42:39PM -0700, Arjan van de Ven wrote:
> 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
--
Kees Cook
Ubuntu Security Team
On Sat, 4 Oct 2008 17:42:33 -0700
Kees Cook <[email protected]> wrote:
> On Sat, Oct 04, 2008 at 04:42:39PM -0700, Arjan van de Ven wrote:
> > 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.
well.. if "you" as kernel can't really make out what it is, how is poor
userspace supposed to do it ?
--
Arjan van de Ven Intel Open Source Technology Centre
For development, discussion and tips for power savings,
visit http://www.lesswatts.org
On Sat, Oct 04, 2008 at 05:48:43PM -0700, Arjan van de Ven wrote:
> On Sat, 4 Oct 2008 17:42:33 -0700
> Kees Cook <[email protected]> wrote:
>
> > On Sat, Oct 04, 2008 at 04:42:39PM -0700, Arjan van de Ven wrote:
> > > 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.
>
> 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.
On Sun, Oct 05, 2008 at 01:11:20PM +0400, Alexey Dobriyan wrote:
> On Sat, Oct 04, 2008 at 05:48:43PM -0700, Arjan van de Ven wrote:
> > On Sat, 4 Oct 2008 17:42:33 -0700
> > Kees Cook <[email protected]> wrote:
> >
> > > On Sat, Oct 04, 2008 at 04:42:39PM -0700, Arjan van de Ven wrote:
> > > > 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.
> >
> > 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.)
> And name should be /proc/*/personality (obviously :-)
>
> Assuming anybody cares about personalities at all.
I do. :)
--
Kees Cook
Ubuntu Security Team
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(-)
diff --git a/fs/proc/array.c b/fs/proc/array.c
index 71c9be5..6b6b492 100644
--- a/fs/proc/array.c
+++ b/fs/proc/array.c
@@ -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)
{
diff --git a/fs/proc/base.c b/fs/proc/base.c
index a28840b..c675c62 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -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),
diff --git a/fs/proc/internal.h b/fs/proc/internal.h
index 4422023..747e3de 100644
--- a/fs/proc/internal.h
+++ b/fs/proc/internal.h
@@ -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
On Sun, Oct 05, 2008 at 03:14:38AM -0700, Kees Cook wrote:
> 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.
> Please revert the prior patch against the "status" file -- this is the
> alternative.
Sure.
Kees,
On Sun, Oct 5, 2008 at 12:14 PM, Kees Cook <[email protected]> wrote:
> 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
> 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(-)
>
> diff --git a/fs/proc/array.c b/fs/proc/array.c
> index 71c9be5..6b6b492 100644
> --- a/fs/proc/array.c
> +++ b/fs/proc/array.c
> @@ -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)
> {
> diff --git a/fs/proc/base.c b/fs/proc/base.c
> index a28840b..c675c62 100644
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -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),
> diff --git a/fs/proc/internal.h b/fs/proc/internal.h
> index 4422023..747e3de 100644
> --- a/fs/proc/internal.h
> +++ b/fs/proc/internal.h
> @@ -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/
>
--
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
Hi Michael,
On Tue, Oct 07, 2008 at 03:39:26PM +0200, Michael Kerrisk wrote:
> On Sun, Oct 5, 2008 at 12:14 PM, Kees Cook <[email protected]> wrote:
> > 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
--
Kees Cook
Ubuntu Security Team
Hi Kees,
On Tue, Oct 7, 2008 at 6:14 PM, Kees Cook <[email protected]> wrote:
> Hi Michael,
>
> On Tue, Oct 07, 2008 at 03:39:26PM +0200, Michael Kerrisk wrote:
>> On Sun, Oct 5, 2008 at 12:14 PM, Kees Cook <[email protected]> wrote:
>> > 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. :)
It's there, but only as of a few days ago.
--
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