Re: [PATCH v2 11/19] driver core: Introduce a generic device trust level

[Leon Romanovsky <leon@kernel.org>]

On Tue, Nov 10, 2026 at 07:35:20AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential
> issues to consider:
> - [High] driver core: device_trust_prepare_full() breaks default boots
>   with -EPERM
> - [Medium] driver core: device_trust_prepare_full() calls
>   trust_resolve() unlocked
> --
> 
> commit cc9d85465ea19f97fc7a0889f6993f37d1a58048
> Author: Leon Romanovsky <leon@kernel.org>
> 
> driver core: Introduce a generic device trust level
> 
> This commit adds generic device trust admission and a device_trust.default
> boot parameter to resolve a device's trust policy into an active trust
> level before driver probing.
> 
> > diff --git a/drivers/base/trust.c b/drivers/base/trust.c
> > new file mode 100644
> > index 0000000000000..6a71abf10b3bb
> > --- /dev/null
> > +++ b/drivers/base/trust.c
> > @@ -0,0 +1,132 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +#include <linux/device.h>
> > +#include <linux/errno.h>
> > +#include <linux/export.h>
> > +#include <linux/init.h>
> > +#include <linux/string.h>
> > +#include <kunit/visibility.h>
> > +
> > +#include "base.h"
> > +
> > +static enum device_trust_policy device_trust_default_policy __ro_after_init;
> 
> [ ... ]
> 
> > +static int __device_trust_prepare(struct device *dev,
> > +				  const struct device_driver *drv, bool full_only)
> > +{
> > +	enum device_trust_policy policy;
> > +	enum device_trust_level level;
> > +	int ret;
> > +
> > +	policy = dev->trust_policy;
> > +
> > +	if (policy == DEVICE_TRUST_POLICY_DISABLED)
> > +		return -EPERM;
> > +	if (dev->bus && dev->bus->trust_resolve) {
> > +		ret = dev->bus->trust_resolve(dev, drv, policy, &level);
> > +		if (ret)
> > +			return ret;
> > +	} else {
> > +		level = policy == DEVICE_TRUST_POLICY_ADVERSARY ?
> > +			DEVICE_TRUST_ADVERSARY : DEVICE_TRUST_FULL;
> > +	}
> > +	if (level == DEVICE_TRUST_DISABLED)
> > +		return -EPERM;
> > +	if (full_only && level != DEVICE_TRUST_FULL)
> > +		return -EPERM;
> > +	if (dev->p->trust_state == DEVICE_TRUST_COMMITTED &&
> > +	    dev->trust_committed_level != level)
> > +		return -EINVAL;
> > +
> > +	WRITE_ONCE(dev->trust_committed_level, level);
> > +	dev->p->trust_state = DEVICE_TRUST_COMMITTED;
> > +	WRITE_ONCE(dev->trust_level, level);
> > +	return 0;
> > +}
> > +
> > +int device_trust_prepare(struct device *dev, const struct device_driver *drv)
> > +{
> > +	device_lock_assert(dev);
> > +	return __device_trust_prepare(dev, drv, false);
> > +}
> > +
> > +int device_trust_prepare_full(struct device *dev, const struct device_driver *drv)
> > +{
> > +	if (device_trust_default_policy != DEVICE_TRUST_POLICY_FULL)
> > +		return -EPERM;
> 
> [Severity: High]
> Does requiring device_trust_default_policy == DEVICE_TRUST_POLICY_FULL
> here cause device_driver_attach_full() to fail with -EPERM on every
> default boot where device_trust.default= is not passed on the command
> line?
> 
> When device_trust.default= is omitted, device_trust_default_policy remains
> DEVICE_TRUST_POLICY_DEFAULT, even though that policy resolves to
> DEVICE_TRUST_FULL on buses without trust_resolve.  Callers converted to
> device_driver_attach_full(), such as phy_attach_direct() when attaching
> the generic PHY fallback driver and serio_bind_driver(), will then fail
> with -EPERM unless the kernel is booted with device_trust.default=full.

While I did this intentionally to make sure that missing option won't
leave system unprotected, I had second thought now as it will break
backward compatibility.

> 
> > +
> > +	return __device_trust_prepare(dev, drv, true);
> > +}
> 
> [Severity: Medium]
> Can device_trust_prepare_full() end up calling dev->bus->trust_resolve()
> without holding the device lock?

Not for the upstream code, both serio and PHY are connected to the
buses without ->trust_resolve().

Thanks

> 
> The kernel-doc for struct bus_type::trust_resolve requires the device lock
> to be held, and the kernel-doc for device_driver_attach_full() states that
> buses with a trust resolver must use device_driver_attach().  Unlike
> device_bind_driver(), which rejects dev->bus->trust_resolve with
> -EOPNOTSUPP, device_trust_prepare_full() calls __device_trust_prepare()
> unlocked without checking dev->bus->trust_resolve.
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20261109-device-trust-core-v2-0-3b12d9a9f861@nvidia.com?part=11
>