Bug 258063

Summary: Kernel panic from a corrupt cd9660 image.
Product: Base System Reporter: Robert Morris <rtm>
Component: kernAssignee: John Baldwin <jhb>
Status: Closed FIXED    
Severity: Affects Only Me CC: emaste, grahamperrin, ish, jhb
Priority: ---    
Version: CURRENT   
Hardware: Any   
OS: Any   
Attachments:
Description Flags
mount_cd9660 of this image on a CD causes CURRENT to panic. none

Description Robert Morris 2021-08-26 15:29:11 UTC
Created attachment 227450 [details]
mount_cd9660 of this image on a CD causes CURRENT to panic.

If I write the attached file to a CD with cdrecord,
and then mount it on my amd64 machine, it panics. You can
also get the panic with

  # mdconfig -S 2048 -f bad2.iso
  # mount_cd9660 /dev/md0 /mnt

FreeBSD 14.0-CURRENT (GENERIC) #0 main-n248636-d20e9e02db3: Thu Aug 12 05:47:18 UTC 2021

panic: wrong offset -1099511604224 for sectorsize 2048
cpuid = 1
time = 1629984411
KDB: stack backtrace:
db_trace_self_wrapper() at db_trace_self_wrapper+0x2b/frame 0xfffffe0063ad1410
vpanic() at vpanic+0x187/frame 0xfffffe0063ad1470
panic() at panic+0x43/frame 0xfffffe0063ad14d0
g_io_request() at g_io_request+0x2ae/frame 0xfffffe0063ad1500
breadn_flags() at breadn_flags+0x1a3/frame 0xfffffe0063ad1570
cd9660_mount() at cd9660_mount+0x941/frame 0xfffffe0063ad1770
vfs_domount() at vfs_domount+0x8d8/frame 0xfffffe0063ad19e0
vfs_donmount() at vfs_donmount+0x880/frame 0xfffffe0063ad1a80
sys_nmount() at sys_nmount+0x69/frame 0xfffffe0063ad1ac0
amd64_syscall() at amd64_syscall+0x12e/frame 0xfffffe0063ad1bf0
fast_syscall_common() at fast_syscall_common+0xf8/frame 0xfffffe0063ad1bf0
--- syscall (378, FreeBSD ELF64, sys_nmount), rip = 0x8011abafa, rsp = 0x7fffffffe278, rbp = 0x7fffffffea20 ---
Comment 1 Graham Perrin freebsd_committer freebsd_triage 2021-09-05 16:30:33 UTC
Also: bug 130941
Comment 2 John Baldwin freebsd_committer freebsd_triage 2023-07-28 22:02:18 UTC
The root issue I believe is that the cd9660 vfs doesn't support volumes where the logical block size in the primary volume descriptor is smaller than the sector size of the underlying medium.  Fixing this would require auditing all the places that call bread*/bwrite* in cd9660 and fixing them to align requests on the underlying sector boundary, both by rounding up the size but also by possibly using an offset into the buf's b_data.

Rather than solve that bigger problem, I've just added a check during mount to reject such volumes in the proposed patch at https://reviews.freebsd.org/D41228
Comment 3 John Baldwin freebsd_committer freebsd_triage 2023-07-28 22:05:15 UTC
Also, I looked at the other bug (130941), but I think it is probably not the same issue.  Namely, in the stack traces in that bug the size passed to breadn is 2048 which should be a sane size.
Comment 4 Robert Morris 2023-07-29 14:22:46 UTC
(In reply to John Baldwin from comment #2)
D41228 does indeed eliminate the problem for me by forbidding the mount.
Comment 5 commit-hook freebsd_committer freebsd_triage 2023-08-04 23:43:54 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=4af849d71f69306432941d91fa46c3c303059d63

commit 4af849d71f69306432941d91fa46c3c303059d63
Author:     John Baldwin <jhb@FreeBSD.org>
AuthorDate: 2023-08-04 23:41:50 +0000
Commit:     John Baldwin <jhb@FreeBSD.org>
CommitDate: 2023-08-04 23:41:50 +0000

    cd9660: Reject volumes with small logical block sizes

    ISO9660 permits specifying a logical block size that is any power of 2
    greater than or equal to 512.  The geom disk layer requires requests
    to be aligned on sector boundaries of the provider.  With a volume
    that uses a logical block size smaller than the underlying disk sector
    size (e.g. a logical block size of 512 or 1024 on a CD which uses 2048
    byte sectors), the current cd9660 vfs can issue requests for partial
    sectors, or on non-sector boundaries.

    Fixing this properly would require wrapping all of the calls to
    bread*/bwrite* in cd9660 vfs to roundup requests to be on sector
    boundaries which can include both the length, but also the starting
    sector number (and thus requiring use of an offset relative to b_data
    in the resulting buf).

    These images do not seem to be common however given that no one has
    fixed this in cd9660's vfs in the past few decades, so just reject
    them during mount with an error.  If such images are found to be used
    in the wild in practice, then the larger fix can be applied.

    PR:             258063
    Reported by:    Robert Morris <rtm@lcs.mit.edu>
    Reviewed by:    emaste
    MFC after:      1 week
    Sponsored by:   The FreeBSD Foundation
    Differential Revision:  https://reviews.freebsd.org/D41228

 sys/fs/cd9660/cd9660_vfsops.c | 7 +++++++
 1 file changed, 7 insertions(+)
Comment 6 commit-hook freebsd_committer freebsd_triage 2023-09-06 21:57:15 UTC
A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=983819d217999303770876885250e70a89ad2687

commit 983819d217999303770876885250e70a89ad2687
Author:     John Baldwin <jhb@FreeBSD.org>
AuthorDate: 2023-08-04 23:41:50 +0000
Commit:     John Baldwin <jhb@FreeBSD.org>
CommitDate: 2023-09-06 21:56:10 +0000

    cd9660: Reject volumes with small logical block sizes

    ISO9660 permits specifying a logical block size that is any power of 2
    greater than or equal to 512.  The geom disk layer requires requests
    to be aligned on sector boundaries of the provider.  With a volume
    that uses a logical block size smaller than the underlying disk sector
    size (e.g. a logical block size of 512 or 1024 on a CD which uses 2048
    byte sectors), the current cd9660 vfs can issue requests for partial
    sectors, or on non-sector boundaries.

    Fixing this properly would require wrapping all of the calls to
    bread*/bwrite* in cd9660 vfs to roundup requests to be on sector
    boundaries which can include both the length, but also the starting
    sector number (and thus requiring use of an offset relative to b_data
    in the resulting buf).

    These images do not seem to be common however given that no one has
    fixed this in cd9660's vfs in the past few decades, so just reject
    them during mount with an error.  If such images are found to be used
    in the wild in practice, then the larger fix can be applied.

    PR:             258063
    Reported by:    Robert Morris <rtm@lcs.mit.edu>
    Reviewed by:    emaste
    MFC after:      1 week
    Sponsored by:   The FreeBSD Foundation
    Differential Revision:  https://reviews.freebsd.org/D41228

    (cherry picked from commit 4af849d71f69306432941d91fa46c3c303059d63)

 sys/fs/cd9660/cd9660_vfsops.c | 7 +++++++
 1 file changed, 7 insertions(+)
Comment 7 commit-hook freebsd_committer freebsd_triage 2023-09-06 21:57:20 UTC
A commit in branch stable/12 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=16f142865cb8190c733271969aa243a2fc934093

commit 16f142865cb8190c733271969aa243a2fc934093
Author:     John Baldwin <jhb@FreeBSD.org>
AuthorDate: 2023-08-04 23:41:50 +0000
Commit:     John Baldwin <jhb@FreeBSD.org>
CommitDate: 2023-09-06 20:03:05 +0000

    cd9660: Reject volumes with small logical block sizes

    ISO9660 permits specifying a logical block size that is any power of 2
    greater than or equal to 512.  The geom disk layer requires requests
    to be aligned on sector boundaries of the provider.  With a volume
    that uses a logical block size smaller than the underlying disk sector
    size (e.g. a logical block size of 512 or 1024 on a CD which uses 2048
    byte sectors), the current cd9660 vfs can issue requests for partial
    sectors, or on non-sector boundaries.

    Fixing this properly would require wrapping all of the calls to
    bread*/bwrite* in cd9660 vfs to roundup requests to be on sector
    boundaries which can include both the length, but also the starting
    sector number (and thus requiring use of an offset relative to b_data
    in the resulting buf).

    These images do not seem to be common however given that no one has
    fixed this in cd9660's vfs in the past few decades, so just reject
    them during mount with an error.  If such images are found to be used
    in the wild in practice, then the larger fix can be applied.

    PR:             258063
    Reported by:    Robert Morris <rtm@lcs.mit.edu>
    Reviewed by:    emaste
    MFC after:      1 week
    Sponsored by:   The FreeBSD Foundation
    Differential Revision:  https://reviews.freebsd.org/D41228

    (cherry picked from commit 4af849d71f69306432941d91fa46c3c303059d63)

 sys/fs/cd9660/cd9660_vfsops.c | 7 +++++++
 1 file changed, 7 insertions(+)