Bug 258833

Summary: corrupt disk image can cause g_label_ntfs_taste to hang the kernel
Product: Base System Reporter: Robert Morris <rtm>
Component: kernAssignee: Mark Johnston <markj>
Status: Closed FIXED    
Severity: Affects Only Me CC: emaste, markj
Priority: ---    
Version: 13.0-RELEASE   
Hardware: Any   
OS: Any   
Attachments:
Description Flags
disk image that causes g_label_ntfs_taste to hang the kernel none

Description Robert Morris 2021-10-01 09:49:31 UTC
Created attachment 228310 [details]
disk image that causes g_label_ntfs_taste to hang the kernel

The attached disk image causes g_label_ntfs_taste to freeze
my kernel. To see this:

% gunzip ntx1.img.gz
% sudo mdconfig -f ntx1.img

Or I can write ntx1.img to a USB memory stick with

% dd bs=512 if=ntx1.img of=/dev/da0 conv=sync

and it will hang my system as soon as I insert it. Here's what
I'm running:

FreeBSD xxx 13.0-RELEASE-p4 FreeBSD 13.0-RELEASE-p4 #0: Tue Aug 24 07:33:27 UTC 2021     root@amd64-builder.daemonology.net:/usr/obj/usr/src/amd64.amd64/sys/GENERIC  amd64

The problem is that these lines in g_label_ntfs_taste() can
generate negative values if the disk content is bad, which
g_read_data() &c don't handle well:

    recsize = (mftrecsz > 0) ? (mftrecsz * bf->bf_bps * bf->bf_spc) : (1 << -mft

    voloff = bf->bf_mftcn * bf->bf_spc * bf->bf_bps +
        recsize * NTFS_VOLUMEINO;
Comment 1 commit-hook freebsd_committer freebsd_triage 2021-10-04 22:18:35 UTC
A commit in branch main references this bug:

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

commit f0a08fa9f532a58f5d7a4814d6eb7ddd49f368da
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2021-10-04 21:48:44 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2021-10-04 22:15:06 +0000

    geom_label: Add more validation for NTFS volume tasting

    - Ensure that the computed MFT record size isn't negative or larger than
      maxphys before trying to read $Volume.
    - Guard against truncated records in volume metadata.
    - Ensure that the record length is large enough to contain the volume
      name.
    - Verify that the (UTF-16-encoded) volume name's length is a multiple of
      two.

    PR:             258833, 258914
    MFC after:      2 weeks
    Sponsored by:   The FreeBSD Foundation

 sys/geom/label/g_label_ntfs.c | 37 ++++++++++++++++++++++++-------------
 1 file changed, 24 insertions(+), 13 deletions(-)
Comment 2 commit-hook freebsd_committer freebsd_triage 2021-10-18 13:09:28 UTC
A commit in branch stable/12 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=116a988f99cd9fcac1a34c17ec0aa81be70d5e4f

commit 116a988f99cd9fcac1a34c17ec0aa81be70d5e4f
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2021-10-04 21:48:44 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2021-10-18 13:08:32 +0000

    geom_label: Add more validation for NTFS volume tasting

    - Ensure that the computed MFT record size isn't negative or larger than
      maxphys before trying to read $Volume.
    - Guard against truncated records in volume metadata.
    - Ensure that the record length is large enough to contain the volume
      name.
    - Verify that the (UTF-16-encoded) volume name's length is a multiple of
      two.

    PR:             258833, 258914
    Sponsored by:   The FreeBSD Foundation

    (cherry picked from commit f0a08fa9f532a58f5d7a4814d6eb7ddd49f368da)

 sys/geom/label/g_label_ntfs.c | 37 ++++++++++++++++++++++++-------------
 1 file changed, 24 insertions(+), 13 deletions(-)
Comment 3 commit-hook freebsd_committer freebsd_triage 2021-10-18 13:09:30 UTC
A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=41dbff735184337b910b36a4ce963b36233916e4

commit 41dbff735184337b910b36a4ce963b36233916e4
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2021-10-04 21:48:44 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2021-10-18 13:07:58 +0000

    geom_label: Add more validation for NTFS volume tasting

    - Ensure that the computed MFT record size isn't negative or larger than
      maxphys before trying to read $Volume.
    - Guard against truncated records in volume metadata.
    - Ensure that the record length is large enough to contain the volume
      name.
    - Verify that the (UTF-16-encoded) volume name's length is a multiple of
      two.

    PR:             258833, 258914
    Sponsored by:   The FreeBSD Foundation

    (cherry picked from commit f0a08fa9f532a58f5d7a4814d6eb7ddd49f368da)

 sys/geom/label/g_label_ntfs.c | 37 ++++++++++++++++++++++++-------------
 1 file changed, 24 insertions(+), 13 deletions(-)
Comment 4 Mark Johnston freebsd_committer freebsd_triage 2021-10-18 13:10:15 UTC
Thanks for the report.