Received-SPF: pass (google.com: domain of linux-kernel+bounces-130174-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Date: Wed, 3 Apr 2024 17:12:07 +0100
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
Cc: Linux ARM <linux-arm-kernel@lists.infradead.org>,
	syzbot <syzbot+cb76c2983557a07cdb14@syzkaller.appspotmail.com>,
	linux-kernel@vger.kernel.org, syzkaller-bugs@googlegroups.com
Subject: Re: [syzbot] [hardening?] [mm?] BUG: bad usercopy in fpa_set
Message-ID: <Zg1/1xbmrY4yDfhO@shell.armlinux.org.uk>
References: <0000000000004cf5c205faf1c7f3@google.com>
 <dcf54740-7cc3-4017-ad1b-8626a22fc15e@I-love.SAKURA.ne.jp>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <dcf54740-7cc3-4017-ad1b-8626a22fc15e@I-love.SAKURA.ne.jp>
Sender: Russell King (Oracle) <linux@armlinux.org.uk>

On Tue, Mar 05, 2024 at 08:27:07PM +0900, Tetsuo Handa wrote:
> Hello.
> 
> syzbot is reporting kernel memory overwrite attempt at fpa_set().
> I guessed that the amount to copy from/to should be sizeof(union fp_state)
> than sizeof(struct user_fp), for arch_ptrace(PTRACE_[SG]ETFPREGS) for arm
> is using offset == 0 and size == sizeof(union fp_state). But my guess did not
> solve the issue ( https://syzkaller.appspot.com/x/patch.diff?x=11e46dbc180000 ).

This is silly.

sizeof(struct user_fp) is:

	8 * (
		1 bit for sign1, 15 bits unused => 2 bytes
		1 bit for sign2, 14 bits unused, 1 bit for j => 2 bytes
		31 bits for mantissa1 => 4 bytes
		32 bits for mantissa0 => 4 bytes
	) +
	4 bytes for fpsr
	4 bytes for fpcr
	8 bytes for ftype
	4 bytes for init_flag

This totals 8 * 12 + 4 + 4 + 8 + 4 = 116 bytes or 29 32-bit quantities,
or 29 "unsigned int"s.

This is copied into union fp_state. This union is made up of one of
several different formats depending on the FP being used. user_fp
doesn't reflect this. However, one of these, struct fp_soft_struct,
is specifically sized to ensure that user_fp is _smaller_.

struct fp_soft_struct is 35 unsigned int's. This is 140 bytes. This
is larger than sizeof(user_fp).

Therefore, there is _no way_ for fpa_set() to overwrite anything
outside of thread_info->fpstate, because sizeof(struct user_fp)
is smaller than sizeof(thread->fpstate).

Syzbot appears to be wrong in this instance.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!