public inbox for linux-m68k@lists.linux-m68k.org
 help / color / mirror / Atom feed
From: Michael Schmitz <schmitzmic@gmail.com>
To: Finn Thain <fthain@linux-m68k.org>
Cc: linux-m68k@vger.kernel.org, geert@linux-m68k.org,
	gerg@linux-m68k.org, linux-m68k@lists.linux-m68k.org
Subject: Re: [PATCH v4 1/2] m68k: Handle __generic_copy_to_user faults more carefully
Date: Thu, 8 Aug 2024 07:32:34 +1200	[thread overview]
Message-ID: <42dfdef0-88d1-4c15-b04b-174f12bd8f3f@gmail.com> (raw)
In-Reply-To: <e0d53477-1646-9504-2382-f8793d078704@linux-m68k.org>

Hi Finn,

thanks for (inadvertently) proving this bug in __clear_user() does exist!

I'm afraid I've lost track of where we're at with this patch series. 
Does it need more work, or more bug reports such as the one below? The 
previous bug reports might be considered somewhat contrived but this 
one's from 'real' user space code, and none too complex at that?

Cheers,

     Michael

On 7/08/24 20:14, Finn Thain wrote:
> On Mon, 29 Apr 2024, Michael Schmitz wrote:
>
>> As mentioned by Finn Thain in his patch to improve put_user
>> exception handling on 040, a similar problem exists on 030
>> processors.
>>
>> A moves instruction that crosses a page boundary from a
>> mapped page into an unmapped one will cause a mid-instruction
>> bus error exception (frame format b), with the PC pointing
>> (usually) two instructions past the faulting movesl instruction.
>>
>> Our exception handling in __generic_copy_to_user only covers
>> the instruction immediately following the faulting one. As
>> a result, fixup_exception in send_fault_sig does not detect
>> this case, and cause send_fault_sig to oops.
>>
>> Extend the exception table to cover one additional instruction
>> beyond the moves[lwb] instructions.
>>
>> Tested on 68030 (Atari Falcon 030) with transfers beginning
>> at one to six bytes offset from the end of a mapped page,
>> followed by further bytes on an unmapped page (testcase
>> derived from stress-ng sysbadaddr stressor by Finn Thain).
>>
>> Tested on 68040 (Mac Quadra) and 68030 (Mac IIci) by Finn Thain.
>>
>> A similar problem is present in __clear_user(); modify the
>> exception table for that function in the same way (tested by
>> Finn Thain).
>>
> Well, that __clear_user() bug is no longer theoretical. I accidentally
> bumped into it when I sent a ^C to a shell script I wrote to test some
> mac_scsi driver patches...
>
>
> [    0.000000] Linux version 6.10.0-mac-00011-gd1d490afeb2a (fthain@nippy) (m68k-unknown-linux-musl-gcc (Gentoo 13.2.1_p20240210 p14) 13.2.1 20240210, GNU ld (Gentoo 2.40 p7) 2.40.0) #39 Sat Aug  3 19:57:43 AEST 2024
>
> ...
>
> root@(none):/# bash scsi-test.sh /dev/sda5
> bs=512k count=1
> [  130.090000] sd 0:0:0:0: [sda] tag#9 PDMA fixup: DRQ timeout
> [  130.090000] sd 0:0:0:0: [sda] tag#9 switching to slow handshake
> [  130.180000] sd 0:0:0:0: Power-on or device reset occurred
> 1+0 records in
> 1+0 records out
> 524288 bytes (524 kB, 512 KiB) copied, 5.13732 s, 102 kB/s
> [  136.030000] bash (57): drop_caches: 3
> ^C
> root@(none):/# bash scsi-test.sh /dev/sdb5
> bs=512k count=1
> 1+0 records in
> 1+0 records out
> 524288 bytes (524 kB, 512 KiB) copied, 1.1273 s, 465 kB/s
> [  160.440000] bash (61): drop_caches: 3
> 1+0 records in
> 1+0 records out
> 524288 bytes (524 kB, 512 KiB) copied, 1.12763 s, 465 kB/s
> [  171.920000] bash (61): drop_caches: 3
>
> bs=64k count=8
> 8+0 records in
> 8+0 records out
> 524288 bytes (524 kB, 512 KiB) copied, 1.03469 s, 507 kB/s
> [  184.480000] bash (61): drop_caches: 3
> 8+0 records in
> 8+0 records out
> 524288 bytes (524 kB, 512 KiB) copied, 1.03221 s, 508 kB/s
> [  196.340000] bash (61): drop_caches: 3
>
> bs=4k count=128
> 128+0 records in
> 128+0 records out
> 524288 bytes (524 kB, 512 KiB) copied, 1.13089 s, 464 kB/s
> [  208.860000] bash (61): drop_caches: 3
> 128+0 records in
> 128+0 records out
> 524288 bytes (524 kB, 512 KiB) copied, 1.13089 s, 464 kB/s
> [  220.850000] bash (61): drop_caches: 3
>
> bs=512 count=1k
> 1024+0 records in
> 1024+0 records out
> 524288 bytes (524 kB, 512 KiB) copied, 3.69678 s, 142 kB/s
> [  236.020000] bash (61): drop_caches: 3
> 1024+0 records in
> 1024+0 records out
> 524288 bytes (524 kB, 512 KiB) copied, 3.68874 s, 142 kB/s
> [  250.570000] bash (61): drop_caches: 3
>
> bs=512k count=1
> 1+0 records in
> 1+0 records out
> 524288 bytes (524 kB, 512 KiB) copied, 1.12338 s, 467 kB/s
> [  263.120000] bash (61): drop_caches: 3
> 1+0 records in
> 1+0 records out
> 524288 bytes (524 kB, 512 KiB) copied, 1.12665 s, 465 kB/s
> [  275.110000] bash (61): drop_caches: 3
>
> bs=64k count=8
> 8+0 records in
> 8+0 records out
> 524288 bytes (524 kB, 512 KiB) copied, 1.08861 s, 482 kB/s
> [  287.610000] bash (61): drop_caches: 3
> 8+0 records in
> 8+0 records out
> 524288 bytes (524 kB, 512 KiB) copied, 1.03319 s, 507 kB/s
> [  299.510000] bash (61): drop_caches: 3
> ^C[  303.190000] Unable to handle kernel access at virtual address d7d17fd7
> [  303.200000] Oops: 00000000
> [  303.210000] Modules linked in:
> [  303.220000] PC: [<004301fe>] __clear_user+0x22/0x40
> [  303.240000] SR: 2000  SP: 4841d159  a2: 00b78590
> [  303.250000] d0: 000003ab    d1: 00000000    d2: 00000000    d3: 8000c150
> [  303.260000] d4: 00002000    d5: 00000000    a0: 8000c154    a1: 009bdb3c
> [  303.280000] Process cmp (pid: 92, task=7d95deea)
> [  303.290000] Frame format=B ssw=0709 isc=0801 isb=0001 daddr=8000c150 dobuf=00000000
> [  303.310000] baddr=00430202 dibuf=8000c150 ver=f
> [  303.320000] Stack from 0149de8c:
> [  303.320000]         8001c2c4 0149deb8 00139904 8000c150 00000eb0 00983c00 00000000 00000003
> [  303.320000]         00000003 8000bf00 00983a00 0149df40 0013a1fa 00912480 8000bf00 00983c60
> [  303.320000]         00000003 00000012 00000000 fffffff8 00000006 d012f728 00000000 00983a5a
> [  303.320000]         00000004 00983a00 000579d4 0052b7e8 009125a0 00912480 00983a5a 80000034
> [  303.320000]         000d1704 014994c0 80008864 80000000 00000000 80008864 80008864 00000000
> [  303.320000]         01497800 00000000 00000034 fffffff8 fffffff8 0149df7c 000e9296 00983a00
> [  303.380000] Call Trace: [<00139904>] elf_load+0x192/0x1da
> [  303.390000]  [<0013a1fa>] load_elf_binary+0x8ae/0xe46
> [  303.400000]  [<000579d4>] try_module_get+0x0/0x48
> [  303.410000]  [<000d1704>] kfree+0x0/0x160
> [  303.420000]  [<000e9296>] bprm_execve+0x134/0x416
> [  303.430000]  [<000e9daa>] copy_string_kernel+0x0/0x184
> [  303.440000]  [<000e9fa4>] copy_strings+0x0/0x310
> [  303.460000]  [<000ea3f6>] do_execveat_common+0x142/0x1d2
> [  303.480000]  [<000eb128>] sys_execve+0x2a/0x36
> [  303.490000]  [<0000275e>] syscall+0x8/0xc
> [  303.510000]  [<0008c010>] do_read_cache_folio+0xde/0x2c8
> [  303.520000]
> [  303.520000] Code: 206e 0008 4282 4a80 6708 0e98 2800 5380 <66f8> 0801 0001 6704 0e58 2800 0801 0000 6704 0e10 2800 241f 4e5e 4e75 4e75 4e56
> [  303.560000] Disabling lock debugging due to kernel taint
>
> root@(none):/#
>
>
> Also, it appears that the patch Michael posted in this thread would have
> prevented that oops because the "jne 1b" at __clear_user+0x22 would have
> been found in the exception table.

  reply	other threads:[~2024-08-07 19:32 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-29  3:09 [PATCH v4 0/2] m68k uaccess fault handling fixes Michael Schmitz
2024-04-29  3:09 ` [PATCH v4 1/2] m68k: Handle __generic_copy_to_user faults more carefully Michael Schmitz
2024-08-07  8:14   ` Finn Thain
2024-08-07 19:32     ` Michael Schmitz [this message]
2024-08-08  1:57       ` Finn Thain
2024-08-08  6:05         ` Greg Ungerer
2024-08-08  6:56           ` Finn Thain
2024-08-08 14:52             ` Greg Ungerer
2024-08-08 19:27               ` Michael Schmitz
2024-08-09  3:34                 ` Finn Thain
2024-08-09  8:03                   ` Michael Schmitz
2024-08-09 12:58                     ` Greg Ungerer
2024-08-09  3:22               ` Finn Thain
2024-08-08  6:58           ` Michael Schmitz
2024-04-29  3:09 ` [PATCH v4 2/2] m68k: improve __constant_copy_to_user_asm() fault handling Michael Schmitz
2024-04-29  7:58 ` [PATCH v4 0/2] m68k uaccess fault handling fixes Greg Ungerer
2024-04-29  8:08   ` Geert Uytterhoeven

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=42dfdef0-88d1-4c15-b04b-174f12bd8f3f@gmail.com \
    --to=schmitzmic@gmail.com \
    --cc=fthain@linux-m68k.org \
    --cc=geert@linux-m68k.org \
    --cc=gerg@linux-m68k.org \
    --cc=linux-m68k@lists.linux-m68k.org \
    --cc=linux-m68k@vger.kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox