linux-kernel - Re: [tip:sched/core] sched/core: Fix a potential double-fetch bug in sched_copy

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-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.DEB.2.21.1901271201170.1622@nanos.tec.linutronix.de>
Date:   Sun, 27 Jan 2019 12:04:44 +0100 (CET)
From:   Thomas Gleixner <tglx@...utronix.de>
To:     mingo@...nel.org, kjlu@....edu, hpa@...or.com,
        stable@...r.kernel.org, peterz@...radead.org,
        torvalds@...ux-foundation.org, linux-kernel@...r.kernel.org
cc:     linux-tip-commits@...r.kernel.org
Subject: Re: [tip:sched/core] sched/core: Fix a potential double-fetch bug
 in sched_copy_attr()

On Mon, 21 Jan 2019, tip-bot for Kangjie Lu wrote:
> Commit-ID:  120e4e76857ddbc9268e1aa3f9de61a498e84618
> Gitweb:     https://git.kernel.org/tip/120e4e76857ddbc9268e1aa3f9de61a498e84618
> Author:     Kangjie Lu <kjlu@....edu>
> AuthorDate: Wed, 9 Jan 2019 01:45:24 -0600
> Committer:  Ingo Molnar <mingo@...nel.org>
> CommitDate: Mon, 21 Jan 2019 11:26:17 +0100
> 
> sched/core: Fix a potential double-fetch bug in sched_copy_attr()
> 
> "uattr->size" is copied in from user space and checked. However, it is
> copied in again after the security check. A malicious user may race to
> change it. The fix sets uattr->size to be the checked size.
> 
> Signed-off-by: Kangjie Lu <kjlu@....edu>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@...radead.org>
> Cc: Linus Torvalds <torvalds@...ux-foundation.org>
> Cc: Peter Zijlstra <peterz@...radead.org>
> Cc: Thomas Gleixner <tglx@...utronix.de>
> Cc: pakki001@....edu
> Cc: <stable@...r.kernel.org>
> Link: https://lkml.kernel.org/r/20190109074524.10176-1-kjlu@umn.edu
> Signed-off-by: Ingo Molnar <mingo@...nel.org>
> ---
>  kernel/sched/core.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index a674c7db2f29..d4d3514c4fe9 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -4499,6 +4499,9 @@ static int sched_copy_attr(struct sched_attr __user *uattr, struct sched_attr *a
>  	if (ret)
>  		return -EFAULT;
>  
> +	/* In case attr->size was changed by user-space: */
> +	attr->size = size;
> +

Just when pondering to send that to Linus, I tried to write up a concise
summary for this which made me look at the patch.

If the size changed, then its clear that user space fiddled with the date
between the size fetch and the full copy from user. So why restoring the
size instead of doing the obvious:

   	 if (attr->size != size)
	 	return -ECRAP;

Hmm?

Thanks,

	tglx