[Bug 297187] `bcm2835_audio_release()` Uses a Freed VCHI Service After Close

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=3D297187

            Bug ID: 297187
           Summary: `bcm2835_audio_release()` Uses a Freed VCHI Service
                    After Close
           Product: Base System
           Version: CURRENT
          Hardware: Any
                OS: Any
            Status: New
          Severity: Affects Some People
          Priority: ---
         Component: kern
          Assignee: [email protected]
          Reporter: [email protected]

## Summary

`bcm2835_audio_release()` calls `vchi_service_close()` and then uncondition=
ally
calls `vchi_service_release()` with the same service handle.

In the VCHI shim implementation, a successful `vchi_service_close()` calls
`service_free(service)`. The subsequent `vchi_service_release()` therefore
dereferences a freed `SHIM_SERVICE_T` object when it reads `service->handle=
`.

This is a real kernel heap-use-after-free during `bcm2835_audio` device det=
ach.
It can result in a kernel panic or other undefined kernel behavior.

I have not identified a remote network trigger. I am reporting this private=
ly
because it is a kernel use-after-free and I would appreciate the Security
Team's classification.

## Bug Details

### Vulnerable caller

At `sys/arm/broadcom/bcm2835/bcm2835_audio.c:358`, `bcm2835_audio_release()`
checks the service handle. At lines 363-367 it closes the service and then
releases the same handle:

```c
if (sc->vchi_handle !=3D VCHIQ_SERVICE_HANDLE_INVALID) {
        success =3D vchi_service_close(sc->vchi_handle);
        if (success !=3D 0)
                BCM2835_LOG_ERROR(sc, "vchi_service_close failed: %d\\n",
                    success);
        vchi_service_release(sc->vchi_handle);
        sc->vchi_handle =3D VCHIQ_SERVICE_HANDLE_INVALID;
}
```

### Free in `vchi_service_close()`

At `sys/contrib/vchiq/interface/vchiq_arm/vchiq_shim.c:694-708`, a successf=
ul
close frees the service object:

```c
int32_t vchi_service_close(const VCHI_SERVICE_HANDLE_T handle)
{
        int32_t ret =3D -1;
        SHIM_SERVICE_T *service =3D (SHIM_SERVICE_T *)handle;
        if (service) {
                VCHIQ_STATUS_T status =3D vchiq_close_service(service->hand=
le);
                if (status =3D=3D VCHIQ_SUCCESS) {
                        service_free(service);
                        service =3D NULL;
                }

                ret =3D vchiq_status_to_vchi(status);
        }
        return ret;
}
```

`service_free()` at approximately
`sys/contrib/vchiq/interface/vchiq_arm/vchiq_shim.c:625` deletes the queue =
and
frees the `SHIM_SERVICE_T` object.

### Use after free in `vchi_service_release()`

At `sys/contrib/vchiq/interface/vchiq_arm/vchiq_shim.c:851-860`,
`vchi_service_release()` casts the handle and reads `service->handle`:

```c
int32_t vchi_service_release(const VCHI_SERVICE_HANDLE_T handle)
{
        int32_t ret =3D -1;
        SHIM_SERVICE_T *service =3D (SHIM_SERVICE_T *)handle;
        if (service)
                ret =3D vchiq_status_to_vchi(
                    vchiq_release_service(service->handle));
        return ret;
}
```

When `vchi_service_close()` returns success, `service` points to freed memo=
ry
before `vchi_service_release()` is called. The read of `service->handle` is
consequently a use-after-free.

The vulnerable call chain is:

```text
bcm2835_audio_detach()
  -> bcm2835_audio_release()                     [bcm2835_audio.c:358]
    -> vchi_service_close(sc->vchi_handle)       [bcm2835_audio.c:363]
      -> vchiq_close_service(service->handle)    [vchiq_shim.c:699]
      -> service_free(service)                   [vchiq_shim.c:701]
    -> vchi_service_release(sc->vchi_handle)    [bcm2835_audio.c:367]
      -> vchiq_release_service(service->handle)  [vchiq_shim.c:856]
```

## Impact and Severity

The direct impact is a kernel heap-use-after-free during audio device detac=
h.
Depending on allocator state and the contents of the freed object, this may
cause a kernel panic, failed driver unload, or other undefined kernel behav=
ior.

Preliminary assessment:

- **Confidentiality:** No impact observed.
- **Integrity:** No impact observed.
- **Availability:** Possible local kernel panic or denial of service.
- **Privilege requirement:** Device detach or driver unload likely requires
root or equivalent administrative privilege.
- **Remote exploitability:** Not demonstrated and unlikely through network
traffic.
- **Security classification:** Likely a local kernel reliability issue rath=
er
than a remotely exploitable DoS under the stated FreeBSD policy. It may be
suitable for a normal bug report or an Errata Notice rather than a Security
Advisory.

The use-after-free is nevertheless in kernel code and could warrant security
handling during initial triage.

## Proposed Fix

The minimal fix is to avoid calling `vchi_service_release()` after a succes=
sful
`vchi_service_close()`, because the close operation has already freed the s=
him
service object:

```diff
--- a/sys/arm/broadcom/bcm2835/bcm2835_audio.c
+++ b/sys/arm/broadcom/bcm2835/bcm2835_audio.c
@@ -360,12 +360,12 @@ bcm2835_audio_release(struct bcm2835_audio_info *sc)
        if (sc->vchi_handle !=3D VCHIQ_SERVICE_HANDLE_INVALID) {
                success =3D vchi_service_close(sc->vchi_handle);
                if (success !=3D 0)
                        BCM2835_LOG_ERROR(sc, "vchi_service_close failed:
%d\\n",
                            success);
-               vchi_service_release(sc->vchi_handle);
                sc->vchi_handle =3D VCHIQ_SERVICE_HANDLE_INVALID;
        }
```

The final fix should confirm the intended VCHI lifecycle semantics for a fa=
iled
`vchi_service_close()`. If `vchi_service_release()` is required only after a
failed close, the implementation should call it conditionally and only while
the handle remains valid. It should never be called after a successful close
has freed the shim service.

Reporter: Xuqing Yang

--=20
You are receiving this mail because:
You are the assignee for the bug.=