[v1] driver/base/memory.c: Validate memory block size early
diff mbox series

Message ID 20190806090142.22709-1-david@redhat.com
State Accepted
Commit 902ce63b337381092ff865f542e854ff3d0ebe2b
Headers show
Series
  • [v1] driver/base/memory.c: Validate memory block size early
Related show

Commit Message

David Hildenbrand Aug. 6, 2019, 9:01 a.m. UTC
Let's validate the memory block size early, when initializing the
memory device infrastructure. Fail hard in case the value is not
suitable.

As nobody checks the return value of memory_dev_init(), turn it into a
void function and fail with a panic in all scenarios instead. Otherwise,
we'll crash later during boot when core/drivers expect that the memory
device infrastructure (including memory_block_size_bytes()) works as
expected.

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Pavel Tatashin <pasha.tatashin@soleen.com>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 drivers/base/memory.c  | 31 +++++++++----------------------
 include/linux/memory.h |  6 +++---
 2 files changed, 12 insertions(+), 25 deletions(-)

Comments

David Hildenbrand Aug. 6, 2019, 9:26 a.m. UTC | #1
On 06.08.19 11:01, David Hildenbrand wrote:

"s/driver/drivers/" in subject.

I think long term, we should move the whole memory block size
configuration (set_memory_block_size_order() and
memory_block_size_bytes()) into drivers/base/memory.c.

> Let's validate the memory block size early, when initializing the
> memory device infrastructure. Fail hard in case the value is not
> suitable.
> 
> As nobody checks the return value of memory_dev_init(), turn it into a
> void function and fail with a panic in all scenarios instead. Otherwise,
> we'll crash later during boot when core/drivers expect that the memory
> device infrastructure (including memory_block_size_bytes()) works as
> expected.
> 
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Pavel Tatashin <pasha.tatashin@soleen.com>
> Cc: Michal Hocko <mhocko@suse.com>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  drivers/base/memory.c  | 31 +++++++++----------------------
>  include/linux/memory.h |  6 +++---
>  2 files changed, 12 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/base/memory.c b/drivers/base/memory.c
> index 790b3bcd63a6..6bea4f3f8040 100644
> --- a/drivers/base/memory.c
> +++ b/drivers/base/memory.c
> @@ -100,21 +100,6 @@ unsigned long __weak memory_block_size_bytes(void)
>  }
>  EXPORT_SYMBOL_GPL(memory_block_size_bytes);
>  
> -static unsigned long get_memory_block_size(void)
> -{
> -	unsigned long block_sz;
> -
> -	block_sz = memory_block_size_bytes();
> -
> -	/* Validate blk_sz is a power of 2 and not less than section size */
> -	if ((block_sz & (block_sz - 1)) || (block_sz < MIN_MEMORY_BLOCK_SIZE)) {
> -		WARN_ON(1);
> -		block_sz = MIN_MEMORY_BLOCK_SIZE;
> -	}
> -
> -	return block_sz;
> -}
> -
>  /*
>   * Show the first physical section index (number) of this memory block.
>   */
> @@ -461,7 +446,7 @@ static DEVICE_ATTR_RO(removable);
>  static ssize_t block_size_bytes_show(struct device *dev,
>  				     struct device_attribute *attr, char *buf)
>  {
> -	return sprintf(buf, "%lx\n", get_memory_block_size());
> +	return sprintf(buf, "%lx\n", memory_block_size_bytes());
>  }
>  
>  static DEVICE_ATTR_RO(block_size_bytes);
> @@ -811,19 +796,22 @@ static const struct attribute_group *memory_root_attr_groups[] = {
>  /*
>   * Initialize the sysfs support for memory devices...
>   */
> -int __init memory_dev_init(void)
> +void __init memory_dev_init(void)
>  {
>  	int ret;
>  	int err;
>  	unsigned long block_sz, nr;
>  
> +	/* Validate the configured memory block size */
> +	block_sz = memory_block_size_bytes();
> +	if (!is_power_of_2(block_sz) || block_sz < MIN_MEMORY_BLOCK_SIZE)
> +		panic("Memory block size not suitable: 0x%lx\n", block_sz);
> +	sections_per_block = block_sz / MIN_MEMORY_BLOCK_SIZE;
> +
>  	ret = subsys_system_register(&memory_subsys, memory_root_attr_groups);
>  	if (ret)
>  		goto out;
>  
> -	block_sz = get_memory_block_size();
> -	sections_per_block = block_sz / MIN_MEMORY_BLOCK_SIZE;
> -
>  	/*
>  	 * Create entries for memory sections that were found
>  	 * during boot and have been initialized
> @@ -839,8 +827,7 @@ int __init memory_dev_init(void)
>  
>  out:
>  	if (ret)
> -		printk(KERN_ERR "%s() failed: %d\n", __func__, ret);
> -	return ret;
> +		panic("%s() failed: %d\n", __func__, ret);
>  }
>  
>  /**
> diff --git a/include/linux/memory.h b/include/linux/memory.h
> index 704215d7258a..0ebb105eb261 100644
> --- a/include/linux/memory.h
> +++ b/include/linux/memory.h
> @@ -79,9 +79,9 @@ struct mem_section;
>  #define IPC_CALLBACK_PRI        10
>  
>  #ifndef CONFIG_MEMORY_HOTPLUG_SPARSE
> -static inline int memory_dev_init(void)
> +static inline void memory_dev_init(void)
>  {
> -	return 0;
> +	return;
>  }
>  static inline int register_memory_notifier(struct notifier_block *nb)
>  {
> @@ -112,7 +112,7 @@ extern int register_memory_isolate_notifier(struct notifier_block *nb);
>  extern void unregister_memory_isolate_notifier(struct notifier_block *nb);
>  int create_memory_block_devices(unsigned long start, unsigned long size);
>  void remove_memory_block_devices(unsigned long start, unsigned long size);
> -extern int memory_dev_init(void);
> +extern void memory_dev_init(void);
>  extern int memory_notify(unsigned long val, void *v);
>  extern int memory_isolate_notify(unsigned long val, void *v);
>  extern struct memory_block *find_memory_block(struct mem_section *);
>

Patch
diff mbox series

diff --git a/drivers/base/memory.c b/drivers/base/memory.c
index 790b3bcd63a6..6bea4f3f8040 100644
--- a/drivers/base/memory.c
+++ b/drivers/base/memory.c
@@ -100,21 +100,6 @@  unsigned long __weak memory_block_size_bytes(void)
 }
 EXPORT_SYMBOL_GPL(memory_block_size_bytes);
 
-static unsigned long get_memory_block_size(void)
-{
-	unsigned long block_sz;
-
-	block_sz = memory_block_size_bytes();
-
-	/* Validate blk_sz is a power of 2 and not less than section size */
-	if ((block_sz & (block_sz - 1)) || (block_sz < MIN_MEMORY_BLOCK_SIZE)) {
-		WARN_ON(1);
-		block_sz = MIN_MEMORY_BLOCK_SIZE;
-	}
-
-	return block_sz;
-}
-
 /*
  * Show the first physical section index (number) of this memory block.
  */
@@ -461,7 +446,7 @@  static DEVICE_ATTR_RO(removable);
 static ssize_t block_size_bytes_show(struct device *dev,
 				     struct device_attribute *attr, char *buf)
 {
-	return sprintf(buf, "%lx\n", get_memory_block_size());
+	return sprintf(buf, "%lx\n", memory_block_size_bytes());
 }
 
 static DEVICE_ATTR_RO(block_size_bytes);
@@ -811,19 +796,22 @@  static const struct attribute_group *memory_root_attr_groups[] = {
 /*
  * Initialize the sysfs support for memory devices...
  */
-int __init memory_dev_init(void)
+void __init memory_dev_init(void)
 {
 	int ret;
 	int err;
 	unsigned long block_sz, nr;
 
+	/* Validate the configured memory block size */
+	block_sz = memory_block_size_bytes();
+	if (!is_power_of_2(block_sz) || block_sz < MIN_MEMORY_BLOCK_SIZE)
+		panic("Memory block size not suitable: 0x%lx\n", block_sz);
+	sections_per_block = block_sz / MIN_MEMORY_BLOCK_SIZE;
+
 	ret = subsys_system_register(&memory_subsys, memory_root_attr_groups);
 	if (ret)
 		goto out;
 
-	block_sz = get_memory_block_size();
-	sections_per_block = block_sz / MIN_MEMORY_BLOCK_SIZE;
-
 	/*
 	 * Create entries for memory sections that were found
 	 * during boot and have been initialized
@@ -839,8 +827,7 @@  int __init memory_dev_init(void)
 
 out:
 	if (ret)
-		printk(KERN_ERR "%s() failed: %d\n", __func__, ret);
-	return ret;
+		panic("%s() failed: %d\n", __func__, ret);
 }
 
 /**
diff --git a/include/linux/memory.h b/include/linux/memory.h
index 704215d7258a..0ebb105eb261 100644
--- a/include/linux/memory.h
+++ b/include/linux/memory.h
@@ -79,9 +79,9 @@  struct mem_section;
 #define IPC_CALLBACK_PRI        10
 
 #ifndef CONFIG_MEMORY_HOTPLUG_SPARSE
-static inline int memory_dev_init(void)
+static inline void memory_dev_init(void)
 {
-	return 0;
+	return;
 }
 static inline int register_memory_notifier(struct notifier_block *nb)
 {
@@ -112,7 +112,7 @@  extern int register_memory_isolate_notifier(struct notifier_block *nb);
 extern void unregister_memory_isolate_notifier(struct notifier_block *nb);
 int create_memory_block_devices(unsigned long start, unsigned long size);
 void remove_memory_block_devices(unsigned long start, unsigned long size);
-extern int memory_dev_init(void);
+extern void memory_dev_init(void);
 extern int memory_notify(unsigned long val, void *v);
 extern int memory_isolate_notify(unsigned long val, void *v);
 extern struct memory_block *find_memory_block(struct mem_section *);