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

[Kanishka De Silva]

Hi Brad, Ritu,

Following up on my own report — I want to correct part of Issue 2
before you spend time reviewing it, since I traced the actual
mechanism more carefully against the current source and my original
description overstated it.

I wrote that "two concurrent VMC_IOCTL_SETHMCID calls can both pass
the state check before either finishes setup, both acquire the same
HMC slot." That's not quite right. ibmvmc_get_free_hmc() checks and
transitions hmcs[i].state atomically under hmcs[i].lock per slot:

spin_lock_irqsave(&hmcs[i].lock, flags);
if (hmcs[i].state == ibmhmc_state_free) {
    hmcs[i].state = ibmhmc_state_initial;
    ...
}
spin_unlock_irqrestore(&hmcs[i].lock, flags);

That rules out two concurrent callers walking away with the same hmc
slot — the second caller will simply see the first slot already
transitioned and move to the next free one.

The actual race is narrower, and lives one level up in
ibmvmc_setup_hmc(), in the unlocked writes after a (distinct) hmc is
obtained:

session->hmc = hmc;
hmc->adapter = &ibmvmc_adapter;
hmc->file_session = session;
session->valid = 1;

If two threads share the same session — e.g. two threads of one
process both issuing VMC_IOCTL_SETHMCID on the same fd before either
has set session->hmc — both pass the if (!session->hmc) check in
ibmvmc_ioctl_sethmcid(), both call ibmvmc_setup_hmc(), and each is
handed a different free hmc. But both then write session->hmc = hmc to
the same shared session struct. Whichever write lands last wins; the
other hmc slot is left orphaned in ibmhmc_state_initial, unreachable
via session->hmc, but still holding hmc->file_session pointing at that
session.

When that session is later freed in ibmvmc_close()
(kfree_sensitive(session)), the orphaned hmc's file_session becomes a
dangling pointer into freed memory — a real use-after-free path, but
via session aliasing between two ioctl calls on a shared fd, not via
two calls acquiring the same hmc slot as I originally described.

The suggested fix direction is unchanged — the TODO at line 1304 still
applies, and a lock covering the state-check-through-session-linkage
sequence in ibmvmc_setup_hmc() (or a per-session mutex serializing
ioctl entry) would close this. I just wanted the mechanism on record
correctly before either of you dug into it.

Apologies for the imprecision in the original report.

Thanks,
Kanishka


On Mon, 13 Jul 2026 at 10:19, Kanishka De Silva <kpskanna1915@xxxxxxxxx> wrote:
>
> 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