Re: [PATCH v3 1/5] revocable: Revocable resource management

[Jonathan Corbet <corbet@lwn.net>]

Tzung-Bi Shih <tzungbi@kernel.org> writes:

> Some resources can be removed asynchronously, for example, resources
> provided by a hot-pluggable device like USB.  When holding a reference
> to such a resource, it's possible for the resource to be removed and
> its memory freed, leading to use-after-free errors on subsequent access.

Far be it from me to complain about a new feature that comes with nice
documentation!  I will make one small observation, though, for
consideration.

We have the document itself:

> diff --git a/Documentation/driver-api/driver-model/revocable.rst b/Documentation/driver-api/driver-model/revocable.rst
> new file mode 100644
> index 000000000000..b9e2968ba9c1
> --- /dev/null
> +++ b/Documentation/driver-api/driver-model/revocable.rst
> @@ -0,0 +1,151 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +==============================
> +Revocable Resource Management
> +==============================
> +
> +Overview
> +========
> +
> +In a system with hot-pluggable devices, such as USB, resources provided by
> +these devices can be removed asynchronously.  If a consumer holds a reference
> +to such a resource, the resource might be deallocated while the reference is
> +still held, leading to use-after-free errors upon subsequent access.
> +
> +The "revocable" mechanism addresses this by establishing a weak reference to a
> +resource that might be freed at any time.  It allows a resource consumer to
> +safely attempt to access the resource, guaranteeing that the access is valid
> +for the duration of its use, or it fails safely if the resource has already
> +been revoked.

[...]

Then there is the in-code documentation:

> diff --git a/drivers/base/revocable.c b/drivers/base/revocable.c
> new file mode 100644
> index 000000000000..80a48896b241
> --- /dev/null
> +++ b/drivers/base/revocable.c
> @@ -0,0 +1,229 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright 2025 Google LLC
> + *
> + * Revocable resource management
> + */
> +
> +#include <linux/device.h>
> +#include <linux/kref.h>
> +#include <linux/revocable.h>
> +#include <linux/slab.h>
> +#include <linux/srcu.h>
> +
> +/**
> + * DOC: Overview
> + *
> + * Some resources can be removed asynchronously, for example, resources
> + * provided by a hot-pluggable device like USB.  When holding a reference
> + * to such a resource, it's possible for the resource to be removed and
> + * its memory freed, leading to use-after-free errors on subsequent access.
> + *
> + * Introduce the revocable to establish weak references to such resources.
> + * It allows a resource consumer to safely attempt to access a resource
> + * that might be freed at any time by the resource provider.
> + *
> + * The implementation uses a provider/consumer model built on Sleepable
> + * RCU (SRCU) to guarantee safe memory access:
> + *
> + * - A resource provider allocates a struct revocable_provider and
> + *   initializes it with a pointer to the resource.

There is a certain amount of duplication here, stuff that might go out
of sync at some point.  I would consider pushing the bulk of the
information into the kerneldoc comments, then actually *using* those
comments in the .rst file (with kernel-doc directives) to create the
rendered version.

Thanks,

jon