Re: [PATCH 01/27] regset: Add support for dynamically sized regsets

[Alex Bennée]

Dave Martin  writes:

> Currently the regset API doesn't allow for the possibility that
> regsets (or at least, the amount of meaningful data in a regset)
> may change in size.
>
> In particular, this results in useless padding being added to
> coredumps in a regset's current size is smaller than its
> theoretical maximum size.
>
> This patch adds a get_size() function to struct user_regset.
> Individual regset implementations can implement this function to
> return the current size of the regset data.  A regset_size()
> function is added to provide callers with an abstract interface for
> determining the size of a regset without needing to know whether
> the regset is dynamically sized or not.
>
> The only affected user of this interface is the ELF coredump code:
> This patch ports ELF coredump to dump regsets with their actual
> size in the coredump.  This has no effect except for new regsets
> that are dynamically sized and provide a get_size() implementation.
>
> Signed-off-by: Dave Martin 

Reviewed-by: Alex Bennée 

> ---
>  fs/binfmt_elf.c|  6 ++---
>  include/linux/regset.h | 67 
> --
>  2 files changed, 63 insertions(+), 10 deletions(-)
>
> diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
> index 879ff9c..16d2403 100644
> --- a/fs/binfmt_elf.c
> +++ b/fs/binfmt_elf.c
> @@ -1697,7 +1697,7 @@ static int fill_thread_core_info(struct 
> elf_thread_core_info *t,
>long signr, size_t *total)
>  {
>   unsigned int i;
> - unsigned int regset_size = view->regsets[0].n * view->regsets[0].size;
> + unsigned int size = regset_size(t->task, >regsets[0]);
>
>   /*
>* NT_PRSTATUS is the one special case, because the regset data
> @@ -1706,7 +1706,7 @@ static int fill_thread_core_info(struct 
> elf_thread_core_info *t,
>* We assume that regset 0 is NT_PRSTATUS.
>*/
>   fill_prstatus(>prstatus, t->task, signr);
> - (void) view->regsets[0].get(t->task, >regsets[0], 0, regset_size,
> + (void) view->regsets[0].get(t->task, >regsets[0], 0, size,
>   >prstatus.pr_reg, NULL);
>
>   fill_note(>notes[0], "CORE", NT_PRSTATUS,
> @@ -1726,7 +1726,7 @@ static int fill_thread_core_info(struct 
> elf_thread_core_info *t,
>   if (regset->core_note_type && regset->get &&
>   (!regset->active || regset->active(t->task, regset))) {
>   int ret;
> - size_t size = regset->n * regset->size;
> + size_t size = regset_size(t->task, regset);
>   void *data = kmalloc(size, GFP_KERNEL);
>   if (unlikely(!data))
>   return 0;
> diff --git a/include/linux/regset.h b/include/linux/regset.h
> index 8e0c9fe..494ceda 100644
> --- a/include/linux/regset.h
> +++ b/include/linux/regset.h
> @@ -107,6 +107,28 @@ typedef int user_regset_writeback_fn(struct task_struct 
> *target,
>int immediate);
>
>  /**
> + * user_regset_get_size_fn - type of @get_size function in  
> user_regset
> + * @target:  thread being examined
> + * @regset:  regset being examined
> + *
> + * This call is optional; usually the pointer is %NULL.
> + *
> + * When provided, this function must return the current size of regset
> + * data, as observed by the @get function in  user_regset.  The
> + * value returned must be a multiple of @size.  The returned size is
> + * required to be valid only until the next time (if any) @regset is
> + * modified for @target.
> + *
> + * This function is intended for dynamically sized regsets.  A regset
> + * that is statically sized does not need to implement it.
> + *
> + * This function should not be called directly: instead, callers should
> + * call regset_size() to determine the current size of a regset.
> + */
> +typedef unsigned int user_regset_get_size_fn(struct task_struct *target,
> +  const struct user_regset *regset);
> +
> +/**
>   * struct user_regset - accessible thread CPU state
>   * @n:   Number of slots (registers).
>   * @size:Size in bytes of a slot (register).
> @@ -117,19 +139,33 @@ typedef int user_regset_writeback_fn(struct task_struct 
> *target,
>   * @set: Function to store values.
>   * @active:  Function to report if regset is active, or %NULL.
>   * @writeback:   Function to write data back to user memory, or 
> %NULL.
> + * @get_size:Function to return the regset's size, or %NULL.
>   *
>   * This data structure describes a machine resource we call a register set.
>   * This is part of the state of an individual thread, not necessarily
>   * actual CPU registers per se.  A register set consists of a number of
>   * similar slots, given by @n.  Each slot is @size bytes, 

[PATCH 01/27] regset: Add support for dynamically sized regsets

[Dave Martin]

Currently the regset API doesn't allow for the possibility that
regsets (or at least, the amount of meaningful data in a regset)
may change in size.

In particular, this results in useless padding being added to
coredumps in a regset's current size is smaller than its
theoretical maximum size.

This patch adds a get_size() function to struct user_regset.
Individual regset implementations can implement this function to
return the current size of the regset data.  A regset_size()
function is added to provide callers with an abstract interface for
determining the size of a regset without needing to know whether
the regset is dynamically sized or not.

The only affected user of this interface is the ELF coredump code:
This patch ports ELF coredump to dump regsets with their actual
size in the coredump.  This has no effect except for new regsets
that are dynamically sized and provide a get_size() implementation.

Signed-off-by: Dave Martin 
---
 fs/binfmt_elf.c|  6 ++---
 include/linux/regset.h | 67 --
 2 files changed, 63 insertions(+), 10 deletions(-)

diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 879ff9c..16d2403 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -1697,7 +1697,7 @@ static int fill_thread_core_info(struct 
elf_thread_core_info *t,
 long signr, size_t *total)
 {
unsigned int i;
-   unsigned int regset_size = view->regsets[0].n * view->regsets[0].size;
+   unsigned int size = regset_size(t->task, >regsets[0]);
 
/*
 * NT_PRSTATUS is the one special case, because the regset data
@@ -1706,7 +1706,7 @@ static int fill_thread_core_info(struct 
elf_thread_core_info *t,
 * We assume that regset 0 is NT_PRSTATUS.
 */
fill_prstatus(>prstatus, t->task, signr);
-   (void) view->regsets[0].get(t->task, >regsets[0], 0, regset_size,
+   (void) view->regsets[0].get(t->task, >regsets[0], 0, size,
>prstatus.pr_reg, NULL);
 
fill_note(>notes[0], "CORE", NT_PRSTATUS,
@@ -1726,7 +1726,7 @@ static int fill_thread_core_info(struct 
elf_thread_core_info *t,
if (regset->core_note_type && regset->get &&
(!regset->active || regset->active(t->task, regset))) {
int ret;
-   size_t size = regset->n * regset->size;
+   size_t size = regset_size(t->task, regset);
void *data = kmalloc(size, GFP_KERNEL);
if (unlikely(!data))
return 0;
diff --git a/include/linux/regset.h b/include/linux/regset.h
index 8e0c9fe..494ceda 100644
--- a/include/linux/regset.h
+++ b/include/linux/regset.h
@@ -107,6 +107,28 @@ typedef int user_regset_writeback_fn(struct task_struct 
*target,
 int immediate);
 
 /**
+ * user_regset_get_size_fn - type of @get_size function in  user_regset
+ * @target:thread being examined
+ * @regset:regset being examined
+ *
+ * This call is optional; usually the pointer is %NULL.
+ *
+ * When provided, this function must return the current size of regset
+ * data, as observed by the @get function in  user_regset.  The
+ * value returned must be a multiple of @size.  The returned size is
+ * required to be valid only until the next time (if any) @regset is
+ * modified for @target.
+ *
+ * This function is intended for dynamically sized regsets.  A regset
+ * that is statically sized does not need to implement it.
+ *
+ * This function should not be called directly: instead, callers should
+ * call regset_size() to determine the current size of a regset.
+ */
+typedef unsigned int user_regset_get_size_fn(struct task_struct *target,
+const struct user_regset *regset);
+
+/**
  * struct user_regset - accessible thread CPU state
  * @n: Number of slots (registers).
  * @size:  Size in bytes of a slot (register).
@@ -117,19 +139,33 @@ typedef int user_regset_writeback_fn(struct task_struct 
*target,
  * @set:   Function to store values.
  * @active:Function to report if regset is active, or %NULL.
  * @writeback: Function to write data back to user memory, or %NULL.
+ * @get_size:  Function to return the regset's size, or %NULL.
  *
  * This data structure describes a machine resource we call a register set.
  * This is part of the state of an individual thread, not necessarily
  * actual CPU registers per se.  A register set consists of a number of
  * similar slots, given by @n.  Each slot is @size bytes, and aligned to
- * @align bytes (which is at least @size).
+ * @align bytes (which is at least @size).  For dynamically-sized
+ * regsets, @n must contain the maximum possible number of slots for the
+ * regset, and @get_size must point to a function that returns the