Bug 287896 - udf_readlink should check fentry->inf_len
Summary: udf_readlink should check fentry->inf_len
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Some People
Assignee: Dag-Erling Smørgrav
URL: https://reviews.freebsd.org/D51339
Keywords:
Depends on:
Blocks:
 
Reported: 2025-06-29 10:53 UTC by Robert Morris
Modified: 2025-07-24 13:03 UTC (History)
1 user (show)

See Also:
des: mfc-stable14+
des: mfc-stable13+


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Robert Morris 2025-06-29 10:53:21 UTC
A malformed UDF disk with a symbolic link's File Entry with a huge
(negative) inf_len can cause trouble starting with this code in
udf_readlink():

        len = le64toh(node->fentry->inf_len);
        buf = malloc(len, M_DEVBUF, M_WAITOK);
        iov[0].iov_len = len;
        iov[0].iov_base = buf;
        uio.uio_resid = iov[0].iov_len;
        error = VOP_READ(vp, &uio, 0, ap->a_cred);

I've put a demo UDF image at http://www.rtmrtm.org/rtm/udf52d.iso

(gdb) print node->fentry->inf_len
$1 = 9223369288075702256
(gdb) print/x node->fentry->inf_len
$2 = 0xffffffffffffeff0
(gdb) print len
$3 = -4112

If INVARIANTS, uiomove() will panic due to resid underflow. And I
worry that malloc() is not very robust to large negative arguments
that wrap when rounded up.

# uname -a
FreeBSD xxx 15.0-CURRENT FreeBSD 15.0-CURRENT #30 main-n275522-551d428b5bdc: Fri Jun 27 16:05:41 AST 2025     root@xxx:/usr/obj/usr/src/amd64.amd64/sys/GENERIC amd64
# fetch http://www.rtmrtm.org/rtm/udf52d.iso
# mdconfig -f udf52d.iso
# mount_udf /dev/md0 /mnt
# ls -l /mnt
panic: uiomove_faultflag: uio 0xfffffe00d6de3ba8 resid underflow
KDB: stack backtrace:
db_trace_self_wrapper() at db_trace_self_wrapper+0x2b/frame 0xfffffe00d6de39a0
vpanic() at vpanic+0x136/frame 0xfffffe00d6de3ad0
panic() at panic+0x43/frame 0xfffffe00d6de3b30
uiomove_faultflag() at uiomove_faultflag+0x1d2/frame 0xfffffe00d6de3b70
VOP_READ_APV() at VOP_READ_APV+0x2a/frame 0xfffffe00d6de3b90
udf_readlink() at udf_readlink+0xb8/frame 0xfffffe00d6de3c50
VOP_READLINK_APV() at VOP_READLINK_APV+0x2a/frame 0xfffffe00d6de3c70
kern_readlinkat() at kern_readlinkat+0x1bf/frame 0xfffffe00d6de3de0
sys_readlink() at sys_readlink+0x26/frame 0xfffffe00d6de3e00
amd64_syscall() at amd64_syscall+0x169/frame 0xfffffe00d6de3f30
fast_syscall_common() at fast_syscall_common+0xf8/frame 0xfffffe00d6de3f30
--- syscall (58, FreeBSD ELF64, readlink), rip = 0xd6c8f963dda, rsp = 0xd6c8ba07088, rbp = 0xd6c8ba07920 ---
Comment 1 commit-hook freebsd_committer freebsd_triage 2025-07-16 19:34:43 UTC
A commit in branch main references this bug:

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

commit 55f80afa17e8926f69660f19631194bcf7fa66f4
Author:     Dag-Erling Smørgrav <des@FreeBSD.org>
AuthorDate: 2025-07-16 19:33:24 +0000
Commit:     Dag-Erling Smørgrav <des@FreeBSD.org>
CommitDate: 2025-07-16 19:33:41 +0000

    udf: Improve input validation.

    The existing code frequently assigns unsigned 64-bit values to variables
    that are signed and / or shorter without checking for overflow.  Try to
    deal with these cases.

    While here, fix two structs that used single-element arrays in place of
    flexible array members.

    PR:             287896
    MFC after:      1 week
    Reviewed by:    imp
    Differential Revision:  https://reviews.freebsd.org/D51339

 sys/fs/udf/ecma167-udf.h |  4 ++--
 sys/fs/udf/udf_vfsops.c  |  7 ++++++-
 sys/fs/udf/udf_vnops.c   | 48 ++++++++++++++++++++++++++++++++++++------------
 3 files changed, 44 insertions(+), 15 deletions(-)
Comment 2 commit-hook freebsd_committer freebsd_triage 2025-07-24 13:03:08 UTC
A commit in branch stable/13 references this bug:

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

commit 4cafd1021a5b6002812f4446794541eab8c08534
Author:     Dag-Erling Smørgrav <des@FreeBSD.org>
AuthorDate: 2025-07-16 19:33:24 +0000
Commit:     Dag-Erling Smørgrav <des@FreeBSD.org>
CommitDate: 2025-07-24 13:02:12 +0000

    udf: Improve input validation.

    The existing code frequently assigns unsigned 64-bit values to variables
    that are signed and / or shorter without checking for overflow.  Try to
    deal with these cases.

    While here, fix two structs that used single-element arrays in place of
    flexible array members.

    PR:             287896
    MFC after:      1 week
    Reviewed by:    imp
    Differential Revision:  https://reviews.freebsd.org/D51339

    (cherry picked from commit 55f80afa17e8926f69660f19631194bcf7fa66f4)

 sys/fs/udf/ecma167-udf.h |  4 ++--
 sys/fs/udf/udf_vfsops.c  |  7 ++++++-
 sys/fs/udf/udf_vnops.c   | 48 ++++++++++++++++++++++++++++++++++++------------
 3 files changed, 44 insertions(+), 15 deletions(-)
Comment 3 commit-hook freebsd_committer freebsd_triage 2025-07-24 13:03:09 UTC
A commit in branch stable/14 references this bug:

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

commit 99dfaac790a3f252d38d1fbe4b3ec2e19f23127f
Author:     Dag-Erling Smørgrav <des@FreeBSD.org>
AuthorDate: 2025-07-16 19:33:24 +0000
Commit:     Dag-Erling Smørgrav <des@FreeBSD.org>
CommitDate: 2025-07-24 13:02:45 +0000

    udf: Improve input validation.

    The existing code frequently assigns unsigned 64-bit values to variables
    that are signed and / or shorter without checking for overflow.  Try to
    deal with these cases.

    While here, fix two structs that used single-element arrays in place of
    flexible array members.

    PR:             287896
    MFC after:      1 week
    Reviewed by:    imp
    Differential Revision:  https://reviews.freebsd.org/D51339

    (cherry picked from commit 55f80afa17e8926f69660f19631194bcf7fa66f4)

 sys/fs/udf/ecma167-udf.h |  4 ++--
 sys/fs/udf/udf_vfsops.c  |  7 ++++++-
 sys/fs/udf/udf_vnops.c   | 48 ++++++++++++++++++++++++++++++++++++------------
 3 files changed, 44 insertions(+), 15 deletions(-)