Skip to content

fix(sg,cam): do not dereference a NULL supported-device table - #651

Open
abdelhaleemahmed wants to merge 1 commit into
LinearTapeFileSystem:release/v2.4.9.1from
abdelhaleemahmed:fix/issue-650-null-supported-devs
Open

abdelhaleemahmed wants to merge 1 commit into
LinearTapeFileSystem:release/v2.4.9.1from
abdelhaleemahmed:fix/issue-650-null-supported-devs

Conversation

@abdelhaleemahmed

Copy link
Copy Markdown

Fixes the SIGSEGV reported in #650.

get_supported_devs() returns NULL for any vendor that is not IBM, HP, HPE
or QUANTUM — the initialiser is NULL and the switch has no default — and
the sg and cam backends use the result without checking it. Opening a drive
whose INQUIRY vendor id is not one of those four therefore segfaults instead of
returning -EDEV_DEVICE_UNSUPPORTABLE three lines further down.

The iokit backend already guards this with while(cur && *cur)
(iokit_tape.c:861); this makes the other two agree with it. No behaviour
changes for any supported drive: the added condition is only ever false when
the table is NULL, which is exactly the unsupported case.

iokit_tape.c:1000 has a similar loop but walks ibm_supported_drives
directly, which is never NULL, so it is left alone.

Verified

Rocky Linux 9.8, LTFS built from v2.4.9.0-10523, drive presented by mhvtl as
vendor STK, product ULT3580-TD8, no cartridge loaded:

ltfs -o devname=/dev/sg9 /tmp/probe
before exit 139 (SIGSEGV) after LTFS30209I Opening a device through sg-ibmtape driver
after LTFS30213I Unsupported Drive 'STK ' / 'ULT3580-TD8 '., exit 1

Base is release/v2.4.9.1, following the process in CONTRIBUTING — the recent
fixes I could see all went to the active release branch rather than main. Say
the word if you would rather have it elsewhere.

Not verified

The cam (FreeBSD) change is the identical expression in the identical
position and is not compiled or run in my environment. Happy to drop it
from this PR if you would rather it went separately.

get_supported_devs() returns NULL for any vendor that is not IBM, HP,
HPE or QUANTUM - the switch has no default case and the initialiser is
NULL. Two backends use the result without checking it:

    struct supported_device **cur = get_supported_devs(priv->vendor);
    while(*cur) {

so opening a drive whose INQUIRY vendor id is not one of those four
segfaults instead of returning -EDEV_DEVICE_UNSUPPORTABLE three lines
further down.

The iokit backend already guards it with `while(cur && *cur)`; this
makes the sg and cam backends agree with it.

Reproduced on Linux with mhvtl, which lets a virtual drive present any
vendor id: a drive reporting vendor 'STK' with product 'ULT3580-TD8'
crashed `ltfs -o devname=... <mountpoint>` with SIGSEGV. With this
change the same command logs

    LTFS30213I Unsupported Drive 'STK     ' / 'ULT3580-TD8     '.

and exits 1.

The device list is unaffected: sg_get_device_list() names drives through
_generate_product_name(), which matches on product id alone, so such a
drive is listed and only refused when it is opened.

Signed-off-by: Ahmed Abdelhaleem Ahmed <ahmedhal@gmail.com>
@vandelvan
vandelvan requested a review from XV02 October 1, 2026 17:10
@vandelvan vandelvan assigned Piloalucard and unassigned Piloalucard Oct 1, 2026

@XV02 XV02 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is necessary, but the problem occurs in more places when calling get_supported_devs that are not addressed here. Also it may be a better fix to directly patch the function to return an empty struct with just { NULL },, that would mean the double check would not be necessary anymore and *cur would safely return NULL and stop the while without the need for the double check.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants