[PATCH 1/1] procfs: expose umask in stat and status

[PATCH 1/1] procfs: expose umask in stat and status

[Pierre Carrier]

A new stat field uses decimal, contains umask or -ENOENT if !task->fs.
A new status line is displayed if task->fs, and uses octal.

Signed-off-by: Pierre Carrier <pierre@spotify.com>
---
Tested shortly.

$ awk '{print $NF}' /proc/1/stat
18
$ grep Umask /proc/1/status
Umask:  022
$ umask 077
$ grep Umask /proc/$$/status
Umask:  077
$ umask 0
$ grep Umask /proc/$$/status
Umask:  00

 Documentation/filesystems/proc.txt |    2 ++
 fs/proc/array.c                    |   24 +++++++++++++++++++++++-
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/Documentation/filesystems/proc.txt b/Documentation/filesystems/proc.txt
index b7413cb..aa33543 100644
--- a/Documentation/filesystems/proc.txt
+++ b/Documentation/filesystems/proc.txt
@@ -212,6 +212,7 @@ Table 1-2: Contents of the status files (as of 2.6.30-rc7)
  TracerPid                   PID of process tracing this process (0 if not)
  Uid                         Real, effective, saved set, and  file system UIDs
  Gid                         Real, effective, saved set, and  file system GIDs
+ Umask                       file mode creation mask
  FDSize                      number of file descriptor slots currently allocated
  Groups                      supplementary group list
  VmPeak                      peak virtual memory size
@@ -310,6 +311,7 @@ Table 1-4: Contents of the stat files (as of 2.6.30-rc7)
   start_data    address above which program data+bss is placed
   end_data      address below which program data+bss is placed
   start_brk     address above which program heap can be expanded with brk()
+  umask         file creation mode mask
 ..............................................................................
 
 The /proc/PID/maps file containing the currently mapped memory regions and
diff --git a/fs/proc/array.c b/fs/proc/array.c
index f9bd395..5e7d5df 100644
--- a/fs/proc/array.c
+++ b/fs/proc/array.c
@@ -81,6 +81,7 @@
 #include <linux/pid_namespace.h>
 #include <linux/ptrace.h>
 #include <linux/tracehook.h>
+#include <linux/fs_struct.h>
 
 #include <asm/pgtable.h>
 #include <asm/processor.h>
@@ -158,11 +159,24 @@ static inline const char *get_task_state(struct task_struct *tsk)
 	return *p;
 }
 
+static inline int get_task_umask(struct task_struct *tsk)
+{
+	struct fs_struct *fs;
+	int umask = -ENOENT;
+
+	task_lock(tsk);
+	fs = tsk->fs;
+	if (fs)
+		umask = fs->umask;
+	task_unlock(tsk);
+	return umask;
+}
+
 static inline void task_state(struct seq_file *m, struct pid_namespace *ns,
 				struct pid *pid, struct task_struct *p)
 {
 	struct group_info *group_info;
-	int g;
+	int g, umask;
 	struct fdtable *fdt = NULL;
 	const struct cred *cred;
 	pid_t ppid, tpid;
@@ -192,6 +206,10 @@ static inline void task_state(struct seq_file *m, struct pid_namespace *ns,
 		cred->uid, cred->euid, cred->suid, cred->fsuid,
 		cred->gid, cred->egid, cred->sgid, cred->fsgid);
 
+	umask = get_task_umask(p);
+	if (umask >= 0)
+		seq_printf(m, "Umask:\t0%o\n", umask);
+
 	task_lock(p);
 	if (p->files)
 		fdt = files_fdtable(p->files);
@@ -368,6 +386,7 @@ static int do_task_stat(struct seq_file *m, struct pid_namespace *ns,
 	pid_t ppid = 0, pgid = -1, sid = -1;
 	int num_threads = 0;
 	int permitted;
+	int umask;
 	struct mm_struct *mm;
 	unsigned long long start_time;
 	unsigned long cmin_flt = 0, cmaj_flt = 0;
@@ -462,6 +481,8 @@ static int do_task_stat(struct seq_file *m, struct pid_namespace *ns,
 	/* convert nsec -> ticks */
 	start_time = nsec_to_clock_t(start_time);
 
+	umask = get_task_umask(task);
+
 	seq_printf(m, "%d (%s) %c", pid_nr_ns(pid, ns), tcomm, state);
 	seq_put_decimal_ll(m, ' ', ppid);
 	seq_put_decimal_ll(m, ' ', pgid);
@@ -511,6 +532,7 @@ static int do_task_stat(struct seq_file *m, struct pid_namespace *ns,
 	seq_put_decimal_ull(m, ' ', (mm && permitted) ? mm->start_data : 0);
 	seq_put_decimal_ull(m, ' ', (mm && permitted) ? mm->end_data : 0);
 	seq_put_decimal_ull(m, ' ', (mm && permitted) ? mm->start_brk : 0);
+	seq_put_decimal_ll(m, ' ', umask);
 	seq_putc(m, '\n');
 	if (mm)
 		mmput(mm);
-- 
1.7.10.1

Re: [PATCH 1/1] procfs: expose umask in stat and status

[Cong Wang]

On 05/05/2012 10:50 AM, Pierre Carrier wrote:
> A new stat field uses decimal, contains umask or -ENOENT if !task->fs.

How is it useful to display numeric -ENOENT in a /proc file?

> A new status line is displayed if task->fs, and uses octal.

So sometimes "Umask:" is displayed, sometimes not...

Re: [PATCH 1/1] procfs: expose umask in stat and status

[Pierre Carrier]

On Sat, May 5, 2012 at 5:15 AM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> How is it useful to display numeric -ENOENT in a /proc file?

The fields in stat are looked up by index, so they should all always appear.
Sticking to a numerical value makes both this code and parsing
reasonably straightforward.

Given expected values are masks, a negative value makes a good marker.
Overall I don't see a compelling reason to avoid an errno.
They seem common in kernel outputs and benefit from some tooling
(self-promotion: [1]).

Would you have a better idea in mind?

> So sometimes "Umask:" is displayed, sometimes not...

Well, it is displayed whenever available (which sounds better to my ear).
This seemed consistent with the current practices: status already has
guards for FDSize, Groups, VmPeak, VmSize, VmLck, VmPin, VmHWM, VmRSS,
VmData, VmStk, VmExe, VmLib, VmPTE, VmSwap.

In all honesty I can't think of a realistic situation where one would
look for a umask in a task that doesn't have one.

-- 
Pierre

[1] https://github.com/pcarrier/stuff/blob/master/sys/errnos.c

Re: [PATCH 1/1] procfs: expose umask in stat and status

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 392 bytes --]

Hi Pierre,

On Sat,  5 May 2012 04:50:27 +0200 Pierre Carrier <pierre@spotify.com> wrote:
>
> +	umask = get_task_umask(p);
> +	if (umask >= 0)
> +		seq_printf(m, "Umask:\t0%o\n", umask);

Why not use "Umask:\t%#o\n" ? that way you don't get two zeros if the
umask is zero.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/1] procfs: expose umask in stat and status

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 388 bytes --]

On Sat,  5 May 2012 04:50:27 +0200 Pierre Carrier <pierre@spotify.com> wrote:
>
> A new stat field uses decimal, contains umask or -ENOENT if !task->fs.
> A new status line is displayed if task->fs, and uses octal.

It would be good to tell us why we need this, of course.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/1] procfs: expose umask in stat and status

[Pierre Carrier]

On Sat, May 5, 2012 at 6:54 AM, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> Why not use "Umask:\t%#o\n" ? that way you don't get two zeros if the
> umask is zero.

Because of ignorance and laziness.
Just tried "%#o" with v3.4-rc5-182-g71eb557 and got equivalent results
to "0%o", including 0->"00".

So it's agreeably better, even we just don't see it yet.


On Sat, May 5, 2012 at 6:54 AM, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> It would be good to tell us why we need this, of course.

Oops. I don't have a killer argument.

We happened to look for the information for a running service and
couldn't think of a simple, non-invasive solution.
It feels like it'd be useful to expose it.

I assumed status is a good fit (already has euid, egid and ngroups for example).
AFAICT there wouldn't be any significant security or performance implications.

But I could very well be missing something.


Thanks,

-- 
Pierre Carrier
Service Reliability Engineer
Spotify AB

Re: [PATCH 1/1] procfs: expose umask in stat and status

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 1274 bytes --]

Hi Pierre,

On Sat, 5 May 2012 13:57:47 +0200 Pierre Carrier <pierre@spotify.com> wrote:
>
> On Sat, May 5, 2012 at 6:54 AM, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> > Why not use "Umask:\t%#o\n" ? that way you don't get two zeros if the
> > umask is zero.
> 
> Because of ignorance and laziness.

:-)

> Just tried "%#o" with v3.4-rc5-182-g71eb557 and got equivalent results
> to "0%o", including 0->"00".

That looks like a misimplementation (i.e. a bug) :-)

> So it's agreeably better, even we just don't see it yet.

Yep, then if someone fixes the bug it will look nicer.

> On Sat, May 5, 2012 at 6:54 AM, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> > It would be good to tell us why we need this, of course.
> 
> Oops. I don't have a killer argument.
> 
> We happened to look for the information for a running service and
> couldn't think of a simple, non-invasive solution.
> It feels like it'd be useful to expose it.

Who is "we"?  i.e. what is the application that would be using this?

i.e. assume I know nothing (which is not so far from the truth :-)) and
tell me why I would want this in my kernel.  Then put that in the commit
message.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

[PATCH 1/1] procfs: expose umask in stat and status

[Pierre Carrier]

- A new stat field prints umask (or -errno if unavailable) in decimal.
- A new status line prints umask in octal whenever available.

Background
----------

Sometimes processes conditionally set their umask, making it hard to know
how files will be created, specially when the open(3) depends on external
factors (eg. remote file server).

Another source of difficulties is daemons inheriting their umask.
Without prior knowledge about the spawning process, the umask can be
hard to know without process inspection.

Knowing the umask is often required to predict the mode used in
future file creation. Knowing the umasks can then be needed to detect or
catch certain security issues on a running system.

Current approaches
------------------

As the kernel doesn't expose it to userspace across process boundaries,
the known available generic solutions to inspect it involve side effects
that may not be acceptable and introduce new userspace tooling
dependencies.
- Attaching gdb to a process will cause delays and degrade performance.
  'call' is needed, which could introduce side effects.
- systemtap also requires utrace, currently missing from mainline, and
  debugging information which can be hard to make available.
  Performance would also degrade.

Process-specific solutions can be cumbersome (eg getting credentials for
a network service, accessing a client implementation to trigger a file
creation, then observing the filesystem), or downright too intrusive
(eg triggering a database creation for a db server, or logging an event
that would later be involved in analytics processes, etc.).

Offered solution
----------------

Offering this information through procfs's stat (resp. status) makes it
straightforward for userspace tools (resp. humans) to access it.
Running systems can be troubleshot or audited without any significant
risk of affecting the system.

Signed-off-by: Pierre Carrier <pierre@spotify.com>
---
Changes from first LKML submission to cover Stephen Rothwell's
feedback:
- Use %#o to print octal.
- Provide use cases.

 Documentation/filesystems/proc.txt |    2 ++
 fs/proc/array.c                    |   24 +++++++++++++++++++++++-
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/Documentation/filesystems/proc.txt b/Documentation/filesystems/proc.txt
index b7413cb..aa33543 100644
--- a/Documentation/filesystems/proc.txt
+++ b/Documentation/filesystems/proc.txt
@@ -212,6 +212,7 @@ Table 1-2: Contents of the status files (as of 2.6.30-rc7)
  TracerPid                   PID of process tracing this process (0 if not)
  Uid                         Real, effective, saved set, and  file system UIDs
  Gid                         Real, effective, saved set, and  file system GIDs
+ Umask                       file mode creation mask
  FDSize                      number of file descriptor slots currently allocated
  Groups                      supplementary group list
  VmPeak                      peak virtual memory size
@@ -310,6 +311,7 @@ Table 1-4: Contents of the stat files (as of 2.6.30-rc7)
   start_data    address above which program data+bss is placed
   end_data      address below which program data+bss is placed
   start_brk     address above which program heap can be expanded with brk()
+  umask         file creation mode mask
 ..............................................................................
 
 The /proc/PID/maps file containing the currently mapped memory regions and
diff --git a/fs/proc/array.c b/fs/proc/array.c
index f9bd395..5ee5731 100644
--- a/fs/proc/array.c
+++ b/fs/proc/array.c
@@ -81,6 +81,7 @@
 #include <linux/pid_namespace.h>
 #include <linux/ptrace.h>
 #include <linux/tracehook.h>
+#include <linux/fs_struct.h>
 
 #include <asm/pgtable.h>
 #include <asm/processor.h>
@@ -158,11 +159,24 @@ static inline const char *get_task_state(struct task_struct *tsk)
 	return *p;
 }
 
+static inline int get_task_umask(struct task_struct *tsk)
+{
+	struct fs_struct *fs;
+	int umask = -ENOENT;
+
+	task_lock(tsk);
+	fs = tsk->fs;
+	if (fs)
+		umask = fs->umask;
+	task_unlock(tsk);
+	return umask;
+}
+
 static inline void task_state(struct seq_file *m, struct pid_namespace *ns,
 				struct pid *pid, struct task_struct *p)
 {
 	struct group_info *group_info;
-	int g;
+	int g, umask;
 	struct fdtable *fdt = NULL;
 	const struct cred *cred;
 	pid_t ppid, tpid;
@@ -192,6 +206,10 @@ static inline void task_state(struct seq_file *m, struct pid_namespace *ns,
 		cred->uid, cred->euid, cred->suid, cred->fsuid,
 		cred->gid, cred->egid, cred->sgid, cred->fsgid);
 
+	umask = get_task_umask(p);
+	if (umask >= 0)
+		seq_printf(m, "Umask:\t%#o\n", umask);
+
 	task_lock(p);
 	if (p->files)
 		fdt = files_fdtable(p->files);
@@ -368,6 +386,7 @@ static int do_task_stat(struct seq_file *m, struct pid_namespace *ns,
 	pid_t ppid = 0, pgid = -1, sid = -1;
 	int num_threads = 0;
 	int permitted;
+	int umask;
 	struct mm_struct *mm;
 	unsigned long long start_time;
 	unsigned long cmin_flt = 0, cmaj_flt = 0;
@@ -462,6 +481,8 @@ static int do_task_stat(struct seq_file *m, struct pid_namespace *ns,
 	/* convert nsec -> ticks */
 	start_time = nsec_to_clock_t(start_time);
 
+	umask = get_task_umask(task);
+
 	seq_printf(m, "%d (%s) %c", pid_nr_ns(pid, ns), tcomm, state);
 	seq_put_decimal_ll(m, ' ', ppid);
 	seq_put_decimal_ll(m, ' ', pgid);
@@ -511,6 +532,7 @@ static int do_task_stat(struct seq_file *m, struct pid_namespace *ns,
 	seq_put_decimal_ull(m, ' ', (mm && permitted) ? mm->start_data : 0);
 	seq_put_decimal_ull(m, ' ', (mm && permitted) ? mm->end_data : 0);
 	seq_put_decimal_ull(m, ' ', (mm && permitted) ? mm->start_brk : 0);
+	seq_put_decimal_ll(m, ' ', umask);
 	seq_putc(m, '\n');
 	if (mm)
 		mmput(mm);
-- 
1.7.10.1