[BUG] drivers/misc/ibmvmc.c: unlocked global state (vmc_drc_index race + HMC session aliasing)

[Kanishka De Silva]

Hi Brad, Ritu,

While auditing drivers/misc/ibmvmc.c in Linux 7.1.3 I found two
related concurrency issues, both stemming from the acknowledged
missing-locking TODO at line 1304:

    /* TODO: (adreznec) Add locking to control multiple process access */

Issue 1 — vmc_drc_index data race:

    ibmvmc_ioctl_requestvmc() writes, with no lock:
        ibmvmc.vmc_drc_index = vmc_drc_index;   // line 1336

    ibmvmc_ioctl_query() reads, with no lock:
        query_struct.vmc_drc_index = ibmvmc.vmc_drc_index;   // line 1281

    Both ioctls are registered as .unlocked_ioctl (no BKL), so two
processes issuing VMC_IOCTL_REQUESTVMC and VMC_IOCTL_QUERY
concurrently can race on this global field.

Issue 2 — HMC session aliasing:

    ibmvmc_setup_hmc() (line 1149) does a state check, HMC
acquisition, and session setup as three separate steps with no
file-level lock across them. Two concurrent VMC_IOCTL_SETHMCID calls
can both pass the state check before either finishes setup, both
acquire the same HMC slot, and end up aliasing hmcs[i].adapter /
hmcs[i].file_session — which looks like a path to a use-after-free if
one session is subsequently torn down while the other still references
the same HMC struct.

I verified both races by extracting the relevant global state, struct
layout, and ioctl control flow into a standalone userspace program and
running it under ThreadSanitizer with pthreads simulating concurrent
ioctl calls. TSan flags data races on hmcs[i].state, hmcs[i].adapter,
and hmcs[i].file_session, consistent with the above. I can send the
verifier program if it's useful, though to be clear it's a userspace
reconstruction of the lock discipline, not the compiled driver.

Exploitability is limited by the fact that /dev/ibmvmc has no explicit
permission override in the miscdevice struct, so it isn't broadly
world-accessible — this needs whatever access level actually reaches
/dev/ibmvmc (root or an HMC-admin group in your deployment model),
which I understand is the intended trust boundary already. Given that,
I'd rate this Medium: real bug, but requires privileged/HMC-admin
access to trigger, and the impact (state corruption / possible UAF on
session teardown) is worth closing given this runs on IBM Power
management infrastructure.

Suggested direction, per your TODO: a mutex around the ioctl handler
(or at minimum around requestvmc/query/sethmcid) would close both:

    static long ibmvmc_ioctl(struct file *file, unsigned int cmd,
unsigned long arg)
    {
    +   mutex_lock(&ibmvmc_mutex);
            ... existing switch ...
    +   mutex_unlock(&ibmvmc_mutex);
    }

Happy to help work up a proper patch if you'd like a hand, or if you
have a preferred locking granularity in mind (I noticed the TODO
predates a full design for this, so wanted to flag it rather than
assume the fix shape).

Thanks,
Kanishka
HEXER365 / github.com/hexer365