Re: [PATCH v5 6/9] dax: plumb hotplug online_type through dax

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Assigning DAX_ONLINE_DEFAULT (-1) to enum mmop triggers UBSan out-of-bounds violation.
--

commit e0320ba16dade8553731440e4c066c6a7c836047
Author: Gregory Price <gourry@gourry.net>

dax: plumb hotplug online_type through dax
    
There is no way for drivers leveraging dax_kmem to plumb through a
preferred auto-online policy - the system default policy is forced.
Add 'enum mmop' field to DAX device creation path to allow drivers
to specify an auto-online policy when using the kmem driver.

> diff --git a/drivers/dax/bus.h b/drivers/dax/bus.h
> index 5909171a4428b..f3c9dae5de6ba 100644
> --- a/drivers/dax/bus.h
> +++ b/drivers/dax/bus.h
> @@ -16,6 +17,13 @@ struct dax_region;
>  #define IORESOURCE_DAX_STATIC BIT(0)
>  #define IORESOURCE_DAX_KMEM BIT(1)
>  
> +/*
> + * online_type sentinel: the device was created without an explicit online
> + * policy, so the system default is resolved when the kmem driver binds,
> + * (not at device-creation time, which would freeze a stale policy).
> + */
> +#define DAX_ONLINE_DEFAULT	(-1)
> +
>  struct dax_region *alloc_dax_region(struct device *parent, int region_id,
>  		struct range *range, int target_node, unsigned int align,
>  		unsigned long flags);
> @@ -26,6 +34,7 @@ struct dev_dax_data {
>  	resource_size_t size;
>  	int id;
>  	bool memmap_on_memory;
> +	enum mmop online_type;

[Severity: Medium]
Can this declaration cause an out-of-bounds UBSan violation when assigned
DAX_ONLINE_DEFAULT? DAX_ONLINE_DEFAULT is defined as -1, but enum mmop only
has a valid range from 0 to 3.

>  };

[ ... ]

> diff --git a/drivers/dax/dax-private.h b/drivers/dax/dax-private.h
> index 81e4af49e39c1..ccd77965fe3ed 100644
> --- a/drivers/dax/dax-private.h
> +++ b/drivers/dax/dax-private.h
> @@ -95,6 +98,7 @@ struct dev_dax {
>  	struct device dev;
>  	struct dev_pagemap *pgmap;
>  	bool memmap_on_memory;
> +	enum mmop online_type;

[Severity: Medium]
Does this struct member also need to be an int instead of enum mmop to safely
store the -1 sentinel value?

>  	int nr_range;
>  	struct dev_dax_range *ranges;
>  };

[ ... ]

> diff --git a/drivers/dax/kmem.c b/drivers/dax/kmem.c
> index 592171ec10f49..0a184c0878ddf 100644
> --- a/drivers/dax/kmem.c
> +++ b/drivers/dax/kmem.c
> @@ -132,6 +133,11 @@ static int dev_dax_kmem_probe(struct dev_dax *dev_dax)
>  		goto err_reg_mgid;
>  	data->mgid = rc;
>  
> +	/* Resolve system default at bind time in case it changed */
> +	online_type = dev_dax->online_type;

[Severity: Medium]
Could loading dev_dax->online_type here when it is set to -1 trigger a UBSan
out-of-bounds violation during probe? 

> +	if (online_type == DAX_ONLINE_DEFAULT)
> +		online_type = mhp_get_default_online_type();
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260624145744.3532049-1-gourry@gourry.net?part=6