linux-kernel - Re: [PATCH] proc: restrict access to /proc/$PID/{sched,schedstat}

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20111108115900.GA14587@albatros>
Date:	Tue, 8 Nov 2011 15:59:00 +0400
From:	Vasiliy Kulikov <segoon@...nwall.com>
To:	Andrew Morton <akpm@...ux-foundation.org>,
	linux-kernel@...r.kernel.org
Cc:	Al Viro <viro@...iv.linux.org.uk>,
	Stephen Wilson <wilsons@...rt.ca>,
	Alexey Dobriyan <adobriyan@...il.com>, security@...nel.org
Subject: Re: [PATCH] proc: restrict access to /proc/$PID/{sched,schedstat}

(CC'ed l-k)

On Sat, Nov 05, 2011 at 14:48 +0400, Vasiliy Kulikov wrote:
> /proc/$PID/{sched,schedstat} contain debugging scheduler counters, which
> should not be world readable.  They may be used to gather private information
> about processes' activity.  E.g. it can be used to count the number of
> characters typed in gksu dialog:
> 
> http://www.openwall.com/lists/oss-security/2011/11/05/3
> 
> This infoleak is similar to io (1d1221f375c) and stat's eip/esp (f83ce3e6b02d)
> infoleaks.  Probably other 0644/0444 procfs files are vulnerable to
> similar infoleaks.
> 
> Cc: <stable@...nel.org>
> Signed-off-by: Vasiliy Kulikov <segoon@...nwall.com>
> ---
>  fs/proc/base.c |   32 ++++++++++++++++++++++----------
>  1 files changed, 22 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/proc/base.c b/fs/proc/base.c
> index 6278ef1..8b67eec 100644
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -410,10 +410,16 @@ static int proc_pid_stack(struct seq_file *m, struct pid_namespace *ns,
>   */
>  static int proc_pid_schedstat(struct task_struct *task, char *buffer)
>  {
> -	return sprintf(buffer, "%llu %llu %lu\n",
> -			(unsigned long long)task->se.sum_exec_runtime,
> -			(unsigned long long)task->sched_info.run_delay,
> -			task->sched_info.pcount);
> +	int ret;
> +	ret = lock_trace(task);
> +	if (!ret) {
> +		ret = sprintf(buffer, "%llu %llu %lu\n",
> +				(unsigned long long)task->se.sum_exec_runtime,
> +				(unsigned long long)task->sched_info.run_delay,
> +				task->sched_info.pcount);
> +		unlock_trace(task);
> +	}
> +	return ret;
>  }
>  #endif
>  
> @@ -1390,15 +1396,21 @@ static int sched_show(struct seq_file *m, void *v)
>  {
>  	struct inode *inode = m->private;
>  	struct task_struct *p;
> +	int ret;
>  
>  	p = get_proc_task(inode);
>  	if (!p)
>  		return -ESRCH;
> -	proc_sched_show_task(p, m);
> +	ret = lock_trace(p);
> +	if (!ret) {
> +		proc_sched_show_task(p, m);
> +		ret = 0;
> +		unlock_trace(p);
> +	}
>  
>  	put_task_struct(p);
>  
> -	return 0;
> +	return ret;
>  }
>  
>  static ssize_t
> @@ -2813,7 +2825,7 @@ static const struct pid_entry tgid_base_stuff[] = {
>  	ONE("personality", S_IRUGO, proc_pid_personality),
>  	INF("limits",	  S_IRUGO, proc_pid_limits),
>  #ifdef CONFIG_SCHED_DEBUG
> -	REG("sched",      S_IRUGO|S_IWUSR, proc_pid_sched_operations),
> +	REG("sched",      S_IRUSR|S_IWUSR, proc_pid_sched_operations),
>  #endif
>  #ifdef CONFIG_SCHED_AUTOGROUP
>  	REG("autogroup",  S_IRUGO|S_IWUSR, proc_pid_sched_autogroup_operations),
> @@ -2851,7 +2863,7 @@ static const struct pid_entry tgid_base_stuff[] = {
>  	ONE("stack",      S_IRUGO, proc_pid_stack),
>  #endif
>  #ifdef CONFIG_SCHEDSTATS
> -	INF("schedstat",  S_IRUGO, proc_pid_schedstat),
> +	INF("schedstat",  S_IRUSR, proc_pid_schedstat),
>  #endif
>  #ifdef CONFIG_LATENCYTOP
>  	REG("latency",  S_IRUGO, proc_lstats_operations),
> @@ -3162,7 +3174,7 @@ static const struct pid_entry tid_base_stuff[] = {
>  	ONE("personality", S_IRUGO, proc_pid_personality),
>  	INF("limits",	 S_IRUGO, proc_pid_limits),
>  #ifdef CONFIG_SCHED_DEBUG
> -	REG("sched",     S_IRUGO|S_IWUSR, proc_pid_sched_operations),
> +	REG("sched",     S_IRUSR|S_IWUSR, proc_pid_sched_operations),
>  #endif
>  	REG("comm",      S_IRUGO|S_IWUSR, proc_pid_set_comm_operations),
>  #ifdef CONFIG_HAVE_ARCH_TRACEHOOK
> @@ -3196,7 +3208,7 @@ static const struct pid_entry tid_base_stuff[] = {
>  	ONE("stack",      S_IRUGO, proc_pid_stack),
>  #endif
>  #ifdef CONFIG_SCHEDSTATS
> -	INF("schedstat", S_IRUGO, proc_pid_schedstat),
> +	INF("schedstat", S_IRUSR, proc_pid_schedstat),
>  #endif
>  #ifdef CONFIG_LATENCYTOP
>  	REG("latency",  S_IRUGO, proc_lstats_operations),
> -- 
> 1.7.0.4
> 

-- 
Vasiliy Kulikov
http://www.openwall.com - bringing security into open computing environments
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/