diff mbox

proc: change permission of /proc/self/mountstats to 0644

Message ID 1385553135-23131-1-git-send-email-tigran.mkrtchyan@desy.de (mailing list archive)
State New, archived
Headers show

Commit Message

Mkrtchyan, Tigran Nov. 27, 2013, 11:52 a.m. UTC
While working on a ganglia plugin for NFS I run into problem that
/proc/self/mountstats is readable by process owner only. As ganglia
changes collector threads effective uid, it can't access to mountstats
file any more.

The content of mountstats is equal for all processes and can be safely
shared.

Signed-off-by: Tigran Mkrtchyan <tigran.mkrtchyan@desy.de>
---
 fs/proc/base.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Chuck Lever Nov. 27, 2013, 3:21 p.m. UTC | #1
On Nov 27, 2013, at 6:52 AM, Tigran Mkrtchyan <tigran.mkrtchyan@desy.de> wrote:

> While working on a ganglia plugin for NFS I run into problem that
> /proc/self/mountstats is readable by process owner only. As ganglia
> changes collector threads effective uid, it can't access to mountstats
> file any more.
> 
> The content of mountstats is equal for all processes and can be safely
> shared.

That is true only if there is a single mount namespace on your system.  The content of mountstats depends on each process's mount namespace.

However, I see that mounts and mountinfo, which also depend on a process's mount namespace, are all S_IRUGO.  I don't see a good reason mountstats should be different.

> 
> Signed-off-by: Tigran Mkrtchyan <tigran.mkrtchyan@desy.de>
> ---
> fs/proc/base.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/proc/base.c b/fs/proc/base.c
> index 03c8d74..d41e284 100644
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -2607,7 +2607,7 @@ static const struct pid_entry tgid_base_stuff[] = {
> 	LNK("exe",        proc_exe_link),
> 	REG("mounts",     S_IRUGO, proc_mounts_operations),
> 	REG("mountinfo",  S_IRUGO, proc_mountinfo_operations),
> -	REG("mountstats", S_IRUSR, proc_mountstats_operations),
> +	REG("mountstats", S_IRUGO, proc_mountstats_operations),
> #ifdef CONFIG_PROC_PAGE_MONITOR
> 	REG("clear_refs", S_IWUSR, proc_clear_refs_operations),
> 	REG("smaps",      S_IRUGO, proc_pid_smaps_operations),
> -- 
> 1.8.4.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Christoph Hellwig Nov. 27, 2013, 3:47 p.m. UTC | #2
On Wed, Nov 27, 2013 at 10:21:29AM -0500, Chuck Lever wrote:
> That is true only if there is a single mount namespace on your system.  The content of mountstats depends on each process's mount namespace.
> 
> However, I see that mounts and mountinfo, which also depend on a process's mount namespace, are all S_IRUGO.  I don't see a good reason mountstats should be different.

Could any of the counters be used as a covert channel?  Any leakage of
information related to RPCSEC_GSS that could helper attackers?

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Chuck Lever Nov. 27, 2013, 4:22 p.m. UTC | #3
On Nov 27, 2013, at 10:47 AM, Christoph Hellwig <hch@infradead.org> wrote:

> On Wed, Nov 27, 2013 at 10:21:29AM -0500, Chuck Lever wrote:
>> That is true only if there is a single mount namespace on your system.  The content of mountstats depends on each process's mount namespace.
>> 
>> However, I see that mounts and mountinfo, which also depend on a process's mount namespace, are all S_IRUGO.  I don't see a good reason mountstats should be different.
> 
> Could any of the counters be used as a covert channel?  Any leakage of
> information related to RPCSEC_GSS that could helper attackers?

Certainly not for the authentication-only pseudoflavor.

The numbers in this file are averages and approximate byte counts.  I'm hard-pressed to think of a way anything useful could be extracted.  But I'm not a security expert.
diff mbox

Patch

diff --git a/fs/proc/base.c b/fs/proc/base.c
index 03c8d74..d41e284 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -2607,7 +2607,7 @@  static const struct pid_entry tgid_base_stuff[] = {
 	LNK("exe",        proc_exe_link),
 	REG("mounts",     S_IRUGO, proc_mounts_operations),
 	REG("mountinfo",  S_IRUGO, proc_mountinfo_operations),
-	REG("mountstats", S_IRUSR, proc_mountstats_operations),
+	REG("mountstats", S_IRUGO, proc_mountstats_operations),
 #ifdef CONFIG_PROC_PAGE_MONITOR
 	REG("clear_refs", S_IWUSR, proc_clear_refs_operations),
 	REG("smaps",      S_IRUGO, proc_pid_smaps_operations),