All of lore.kernel.org
 help / color / mirror / Atom feed
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
Date: Wed, 3 Apr 2024 17:12:07 +0100	[thread overview]
Message-ID: <Zg1/1xbmrY4yDfhO@shell.armlinux.org.uk> (raw)
In-Reply-To: <dcf54740-7cc3-4017-ad1b-8626a22fc15e@I-love.SAKURA.ne.jp>

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!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
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
Date: Wed, 3 Apr 2024 17:12:07 +0100	[thread overview]
Message-ID: <Zg1/1xbmrY4yDfhO@shell.armlinux.org.uk> (raw)
In-Reply-To: <dcf54740-7cc3-4017-ad1b-8626a22fc15e@I-love.SAKURA.ne.jp>

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!

  reply	other threads:[~2024-04-03 16:14 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-05 12:53 [syzbot] [hardening?] [mm?] BUG: bad usercopy in fpa_set syzbot
2023-05-07 11:03 ` syzbot
2024-03-05 10:17 ` Tetsuo Handa
2024-03-05 10:27   ` syzbot
2024-03-05 10:55   ` Tetsuo Handa
2024-03-05 11:04     ` syzbot
2024-04-05 11:42     ` Tetsuo Handa
2024-04-05 11:44       ` [syzbot] [arm] " syzbot
2024-04-05 14:02       ` [syzbot] [hardening?] [mm?] " Tetsuo Handa
2024-04-05 14:25         ` [syzbot] [arm] " syzbot
2024-03-05 11:27 ` [syzbot] [hardening?] [mm?] " Tetsuo Handa
2024-03-05 11:27   ` Tetsuo Handa
2024-04-03 16:12   ` Russell King (Oracle) [this message]
2024-04-03 16:12     ` Russell King (Oracle)
2024-04-05 14:28     ` Tetsuo Handa
2024-04-05 14:28       ` Tetsuo Handa
2024-04-15  9:02     ` Mark Rutland
2024-04-15  9:02       ` Mark Rutland
2024-04-15  9:38       ` Tetsuo Handa
2024-04-15  9:38         ` Tetsuo Handa
2024-04-15  9:44         ` Russell King (Oracle)
2024-04-15  9:44           ` Russell King (Oracle)
2024-04-15  9:58           ` Tetsuo Handa
2024-04-15  9:58             ` Tetsuo Handa
2024-04-15 10:27             ` Russell King (Oracle)
2024-04-15 10:27               ` Russell King (Oracle)
2024-04-15 11:43               ` Mark Rutland
2024-04-15 11:43                 ` Mark Rutland
2024-04-15 17:02                 ` Kees Cook
2024-04-15 17:02                   ` Kees Cook

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Zg1/1xbmrY4yDfhO@shell.armlinux.org.uk \
    --to=linux@armlinux.org.uk \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=penguin-kernel@i-love.sakura.ne.jp \
    --cc=syzbot+cb76c2983557a07cdb14@syzkaller.appspotmail.com \
    --cc=syzkaller-bugs@googlegroups.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.