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 <[email protected]>

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/[email protected]?part=6

Reply via email to