From: Shradha Gupta <shradhagupta@xxxxxxxxxxxxxxxxxxx> Sent: Thursday, September 29, 2022 11:02 PM
>
> Current code allows the page_reporting_order parameter to be changed
> via sysfs to any integer value. The new value is used immediately
> in page reporting code with no validation, which could cause incorrect
> behavior. Fix this by adding validation of the new value.
> Export this parameter for use in the driver that is calling the
> page_reporting_register().
> This is needed by drivers like hv_balloon to know the order of the
> pages reported. Traditionally the values provided in the kernel boot
> line or subsequently changed via sysfs take priority therefore, if
> page_reporting_order parameter's value is set, it takes precedence
> over the value passed while registering with the driver.
>
> Signed-off-by: Shradha Gupta <shradhagupta@xxxxxxxxxxxxxxxxxxx>
> ---
> mm/page_reporting.c | 50 ++++++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 45 insertions(+), 5 deletions(-)
>
> diff --git a/mm/page_reporting.c b/mm/page_reporting.c
> index 382958eef8a9..29d67c824fd2 100644
> --- a/mm/page_reporting.c
> +++ b/mm/page_reporting.c
> @@ -11,10 +11,42 @@
> #include "page_reporting.h"
> #include "internal.h"
>
> -unsigned int page_reporting_order = MAX_ORDER;
> -module_param(page_reporting_order, uint, 0644);
> +/* Initialize to an unsupported value */
> +unsigned int page_reporting_order = -1;
> +
> +int page_order_update_notify(const char *val, const struct kernel_param *kp)
> +{
> + /*
> + * If param is set beyond this limit, order is set to default
> + * pageblock_order value
> + */
> + return param_set_uint_minmax(val, kp, 0, MAX_ORDER-1);
> +}
> +
> +const struct kernel_param_ops page_reporting_param_ops = {
> + .set = &page_order_update_notify,
> + /*
> + * For the get op, use param_get_int instead of param_get_uint.
> + * This is to make sure that when unset the initialized value of
> + * -1 is shown correctly
> + */
> + .get = ¶m_get_int,
> +};
> +
> +module_param_cb(page_reporting_order, &page_reporting_param_ops,
> + &page_reporting_order, 0644);
> MODULE_PARM_DESC(page_reporting_order, "Set page reporting order");
>
> +/*
> + * This symbol is also a kernel parameter. Export the page_reporting_order
> + * symbol so that other drivers can access it to control order values without
> + * having to introduce another configurable parameter. Only one driver can
> + * register with the page_reporting driver for the service, so we have just
> + * one control parameter for the use case(which can be accessed in both
> + * drivers)
> + */
> +EXPORT_SYMBOL_GPL(page_reporting_order);
> +
> #define PAGE_REPORTING_DELAY (2 * HZ)
> static struct page_reporting_dev_info __rcu *pr_dev_info __read_mostly;
>
> @@ -330,10 +362,18 @@ int page_reporting_register(struct page_reporting_dev_info
> *prdev)
> }
>
> /*
> - * Update the page reporting order if it's specified by driver.
> - * Otherwise, it falls back to @pageblock_order.
> + * If the page_reporting_order value is not set, we check if
> + * an order is provided from the driver that is performing the
> + * registration. If that is not provided either, we default to
> + * pageblock_order.
> */
> - page_reporting_order = prdev->order ? : pageblock_order;
> +
> + if (page_reporting_order == -1) {
> + if (prdev->order > 0 && prdev->order <= MAX_ORDER)
> + page_reporting_order = prdev->order;
> + else
> + page_reporting_order = pageblock_order;
> + }
>
> /* initialize state and work structures */
> atomic_set(&prdev->state, PAGE_REPORTING_IDLE);
> --
> 2.37.2
Reviewed-by: Michael Kelley <mikelley@xxxxxxxxxxxxx>
