diff mbox series

[v3,10/12] livepatch: Handle arbitrary size names with the list operation

Message ID 20190916105945.93632-11-wipawel@amazon.de (mailing list archive)
State Superseded
Headers show
Series livepatch: new features and fixes | expand

Commit Message

Wieczorkiewicz, Pawel Sept. 16, 2019, 10:59 a.m. UTC
The payloads' name strings can be of arbitrary size (typically small
with an upper bound of XEN_LIVEPATCH_NAME_SIZE).
Current implementation of the list operation interface allows to copy
names in the XEN_LIVEPATCH_NAME_SIZE chunks regardless of its actual
size and enforces space allocation requirements on userland tools.

To unify and simplify the interface, handle the name strings of
arbitrary size by copying them in adhering chunks to the userland.
In order to let the userland allocate enough space for the incoming
data add an auxiliary interface xc_livepatch_list_get_sizes() that
provides the current number of payload entries and the total size of
all name strings. This is achieved by extending the sysctl list
interface with an extra fields: name_total_size.

The xc_livepatch_list_get_sizes() issues the livepatch sysctl list
operation with the nr field set to 0. In this mode the operation
returns the number of payload entries and calculates the total sizes
for all payloads' names.
When the sysctl operation is issued with a non-zero nr field (for
instance with a value obtained earlier with the prior call to the
xc_livepatch_list_get_sizes()) the new field name_total_size provides
the total size of actually copied data.

Extend the libxc to handle the name back-to-back data transfers.

The xen-livepatch tool is modified to start the list operation with a
call to the xc_livepatch_list_get_sizes() to obtain the actual number
of payloads as well as the necessary space for names.
The tool now always requests the actual number of entries and leaves
the preemption handling to the libxc routine. The libxc still returns
'done' and 'left' parameters with the same semantic allowing the tool
to detect anomalies and react to them. At the moment it is expected
that the tool receives the exact number of entires as requested.
The xen-livepatch tool has been also modified to handle the name
back-to-back transfers correctly.

Signed-off-by: Pawel Wieczorkiewicz <wipawel@amazon.de>
Reviewed-by: Andra-Irina Paraschiv <andraprs@amazon.com>
Reviewed-by: Bjoern Doebel <doebel@amazon.de>
Reviewed-by: Martin Pohlack <mpohlack@amazon.de>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
Changed since v1:
  * added corresponding documentation

 docs/misc/livepatch.pandoc    |  24 +++++----
 tools/libxc/include/xenctrl.h |  49 ++++++++++++------
 tools/libxc/xc_misc.c         | 100 ++++++++++++++++++++++++++++---------
 tools/misc/xen-livepatch.c    | 112 ++++++++++++++++++++++--------------------
 xen/common/livepatch.c        |  31 +++++++++---
 xen/include/public/sysctl.h   |  15 +++---
 6 files changed, 219 insertions(+), 112 deletions(-)

Comments

Jan Beulich Sept. 17, 2019, 8:27 a.m. UTC | #1
On 16.09.2019 12:59, Pawel Wieczorkiewicz wrote:
> @@ -951,11 +952,13 @@ struct xen_sysctl_livepatch_list {
>                                                 amount of payloads and version.
>                                                 OUT: How many payloads left. */
>      uint32_t pad;                           /* IN: Must be zero. */
> +    uint64_t name_total_size;               /* OUT: Total size of all transfer names */

Why uint64_t and not uint32_t? You don't expect this to grow
beyond 4GiB, do you?

And why OUT rather than IN/OUT? Once you make this "arbitrary
size", I don't see a need for limiting this to ...

>      XEN_GUEST_HANDLE_64(xen_livepatch_status_t) status;  /* OUT. Must have enough
>                                                 space allocate for nr of them. */
>      XEN_GUEST_HANDLE_64(char) name;         /* OUT: Array of names. Each member
> -                                               MUST XEN_LIVEPATCH_NAME_SIZE in size.
> -                                               Must have nr of them. */
> +                                               may have an arbitrary length up to
> +                                               XEN_LIVEPATCH_NAME_SIZE bytes. Must have
> +                                               nr of them. */

... XEN_LIVEPATCH_NAME_SIZE bytes per entry.

And finally - please send to the list just once, i.e. please
don't have two xen-devel@ in the recipients list.

Jan
Wieczorkiewicz, Pawel Sept. 17, 2019, 8:40 a.m. UTC | #2
On 17. Sep 2019, at 10:27, Jan Beulich <jbeulich@suse.com<mailto:jbeulich@suse.com>> wrote:

On 16.09.2019 12:59, Pawel Wieczorkiewicz wrote:
@@ -951,11 +952,13 @@ struct xen_sysctl_livepatch_list {
                                               amount of payloads and version.
                                               OUT: How many payloads left. */
    uint32_t pad;                           /* IN: Must be zero. */
+    uint64_t name_total_size;               /* OUT: Total size of all transfer names */

Why uint64_t and not uint32_t? You don't expect this to grow
beyond 4GiB, do you?

I don’t, but uint32_t is not really compatible with size_t.
And I was thought to always use size_t compatible types for sizes.

Anyway, I do not mind changing this to whatever type you prefer.


And why OUT rather than IN/OUT? Once you make this "arbitrary
size", I don't see a need for limiting this to ...

    XEN_GUEST_HANDLE_64(xen_livepatch_status_t) status;  /* OUT. Must have enough
                                               space allocate for nr of them. */
    XEN_GUEST_HANDLE_64(char) name;         /* OUT: Array of names. Each member
-                                               MUST XEN_LIVEPATCH_NAME_SIZE in size.
-                                               Must have nr of them. */
+                                               may have an arbitrary length up to
+                                               XEN_LIVEPATCH_NAME_SIZE bytes. Must have
+                                               nr of them. */

... XEN_LIVEPATCH_NAME_SIZE bytes per entry.

Changing the upper bound limitation will break certain assumptions and I did not want to pile all these on top of the current change.
But, yes, the upper bound limit could be dropped. I would prefer to do it as an independent patch.


And finally - please send to the list just once, i.e. please
don't have two xen-devel@ in the recipients list.


Sorry, I did not notice the add_maintainers.pl script adds an explicit To: for the xen-devel@.

Jan

Best Regards,
Pawel Wieczorkiewicz






Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Ralf Herbrich
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879
Jan Beulich Sept. 17, 2019, 8:48 a.m. UTC | #3
On 17.09.2019 10:40,  Wieczorkiewicz, Pawel  wrote:
> 
> 
> On 17. Sep 2019, at 10:27, Jan Beulich <jbeulich@suse.com<mailto:jbeulich@suse.com>> wrote:
> 
> On 16.09.2019 12:59, Pawel Wieczorkiewicz wrote:
> @@ -951,11 +952,13 @@ struct xen_sysctl_livepatch_list {
>                                                amount of payloads and version.
>                                                OUT: How many payloads left. */
>     uint32_t pad;                           /* IN: Must be zero. */
> +    uint64_t name_total_size;               /* OUT: Total size of all transfer names */
> 
> Why uint64_t and not uint32_t? You don't expect this to grow
> beyond 4GiB, do you?
> 
> I don’t, but uint32_t is not really compatible with size_t.
> And I was thought to always use size_t compatible types for sizes.

That's a fair point, but I think we use 32-bit sizes elsewhere
as well, when crossing the 4GiB boundary would seem entirely
unexpected.

But what's worse here - you shouldn't use plain uint64_t in
sysctl.h (and domctl.h) anyway. If anything, you ought to use
uint64_aligned_t. (Going through the file I notice a few other
bad instances have crept in.)

> Anyway, I do not mind changing this to whatever type you prefer.

Well, preference - if anyone's - would be the livepatch maintainers'
one here.

Also - can you please see about adjusting your reply style? In plain
text mode it's impossible to tell context from your responses.

Jan
Wieczorkiewicz, Pawel Sept. 17, 2019, 8:55 a.m. UTC | #4
> On 17. Sep 2019, at 10:48, Jan Beulich <jbeulich@suse.com> wrote:
> 
> On 17.09.2019 10:40,  Wieczorkiewicz, Pawel  wrote:
>> 
>> 
>> On 17. Sep 2019, at 10:27, Jan Beulich <jbeulich@suse.com<mailto:jbeulich@suse.com>> wrote:
>> 
>> On 16.09.2019 12:59, Pawel Wieczorkiewicz wrote:
>> @@ -951,11 +952,13 @@ struct xen_sysctl_livepatch_list {
>>                                               amount of payloads and version.
>>                                               OUT: How many payloads left. */
>>    uint32_t pad;                           /* IN: Must be zero. */
>> +    uint64_t name_total_size;               /* OUT: Total size of all transfer names */
>> 
>> Why uint64_t and not uint32_t? You don't expect this to grow
>> beyond 4GiB, do you?
>> 
>> I don’t, but uint32_t is not really compatible with size_t.
>> And I was thought to always use size_t compatible types for sizes.
> 
> That's a fair point, but I think we use 32-bit sizes elsewhere
> as well, when crossing the 4GiB boundary would seem entirely
> unexpected.
> 
> But what's worse here - you shouldn't use plain uint64_t in
> sysctl.h (and domctl.h) anyway. If anything, you ought to use
> uint64_aligned_t. (Going through the file I notice a few other
> bad instances have crept in.)
> 

I see. Noted, thanks for letting me know.

>> Anyway, I do not mind changing this to whatever type you prefer.
> 
> Well, preference - if anyone's - would be the livepatch maintainers'
> one here.
> 

Waiting for the maintainers' wise judgment then :-).

> Also - can you please see about adjusting your reply style? In plain
> text mode it's impossible to tell context from your responses.

Oh, sorry about that. I tweaked my settings.
Please let me know if it does not get better.

> 
> Jan

Best Regards,
Pawel Wieczorkiewicz






Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Ralf Herbrich
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879
Konrad Rzeszutek Wilk Sept. 17, 2019, 12:55 p.m. UTC | #5
On Tue, Sep 17, 2019 at 08:55:22AM +0000, Wieczorkiewicz, Pawel wrote:
> 
> 
> > On 17. Sep 2019, at 10:48, Jan Beulich <jbeulich@suse.com> wrote:
> > 
> > On 17.09.2019 10:40,  Wieczorkiewicz, Pawel  wrote:
> >> 
> >> 
> >> On 17. Sep 2019, at 10:27, Jan Beulich <jbeulich@suse.com<mailto:jbeulich@suse.com>> wrote:
> >> 
> >> On 16.09.2019 12:59, Pawel Wieczorkiewicz wrote:
> >> @@ -951,11 +952,13 @@ struct xen_sysctl_livepatch_list {
> >>                                               amount of payloads and version.
> >>                                               OUT: How many payloads left. */
> >>    uint32_t pad;                           /* IN: Must be zero. */
> >> +    uint64_t name_total_size;               /* OUT: Total size of all transfer names */
> >> 
> >> Why uint64_t and not uint32_t? You don't expect this to grow
> >> beyond 4GiB, do you?
> >> 
> >> I don’t, but uint32_t is not really compatible with size_t.
> >> And I was thought to always use size_t compatible types for sizes.
> > 
> > That's a fair point, but I think we use 32-bit sizes elsewhere
> > as well, when crossing the 4GiB boundary would seem entirely
> > unexpected.
> > 
> > But what's worse here - you shouldn't use plain uint64_t in
> > sysctl.h (and domctl.h) anyway. If anything, you ought to use
> > uint64_aligned_t. (Going through the file I notice a few other
> > bad instances have crept in.)
> > 
> 
> I see. Noted, thanks for letting me know.
> 
> >> Anyway, I do not mind changing this to whatever type you prefer.
> > 
> > Well, preference - if anyone's - would be the livepatch maintainers'
> > one here.
> > 
> 
> Waiting for the maintainers' wise judgment then :-).

32-bit please. Thx
> 
> > Also - can you please see about adjusting your reply style? In plain
> > text mode it's impossible to tell context from your responses.
> 
> Oh, sorry about that. I tweaked my settings.
> Please let me know if it does not get better.
> 
> > 
> > Jan
> 
> Best Regards,
> Pawel Wieczorkiewicz
> 
> 
> 
> 
> 
> 
> Amazon Development Center Germany GmbH
> Krausenstr. 38
> 10117 Berlin
> Geschaeftsfuehrung: Christian Schlaeger, Ralf Herbrich
> Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
> Sitz: Berlin
> Ust-ID: DE 289 237 879
> 
>
Ross Lagerwall Sept. 25, 2019, 11 a.m. UTC | #6
On 9/16/19 11:59 AM, Pawel Wieczorkiewicz wrote:
> The payloads' name strings can be of arbitrary size (typically small
> with an upper bound of XEN_LIVEPATCH_NAME_SIZE).
> Current implementation of the list operation interface allows to copy
> names in the XEN_LIVEPATCH_NAME_SIZE chunks regardless of its actual
> size and enforces space allocation requirements on userland tools.
> 
> To unify and simplify the interface, handle the name strings of
> arbitrary size by copying them in adhering chunks to the userland.
> In order to let the userland allocate enough space for the incoming
> data add an auxiliary interface xc_livepatch_list_get_sizes() that
> provides the current number of payload entries and the total size of
> all name strings. This is achieved by extending the sysctl list
> interface with an extra fields: name_total_size.
> 
> The xc_livepatch_list_get_sizes() issues the livepatch sysctl list
> operation with the nr field set to 0. In this mode the operation
> returns the number of payload entries and calculates the total sizes
> for all payloads' names.
> When the sysctl operation is issued with a non-zero nr field (for
> instance with a value obtained earlier with the prior call to the
> xc_livepatch_list_get_sizes()) the new field name_total_size provides
> the total size of actually copied data.
> 
> Extend the libxc to handle the name back-to-back data transfers.
> 
> The xen-livepatch tool is modified to start the list operation with a
> call to the xc_livepatch_list_get_sizes() to obtain the actual number
> of payloads as well as the necessary space for names.
> The tool now always requests the actual number of entries and leaves
> the preemption handling to the libxc routine. The libxc still returns
> 'done' and 'left' parameters with the same semantic allowing the tool
> to detect anomalies and react to them. At the moment it is expected
> that the tool receives the exact number of entires as requested.

entries

> The xen-livepatch tool has been also modified to handle the name
> back-to-back transfers correctly.
> 
> Signed-off-by: Pawel Wieczorkiewicz <wipawel@amazon.de>
> Reviewed-by: Andra-Irina Paraschiv <andraprs@amazon.com>
> Reviewed-by: Bjoern Doebel <doebel@amazon.de>
> Reviewed-by: Martin Pohlack <mpohlack@amazon.de>
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
> Changed since v1:
>    * added corresponding documentation
> 
>   docs/misc/livepatch.pandoc    |  24 +++++----
>   tools/libxc/include/xenctrl.h |  49 ++++++++++++------
>   tools/libxc/xc_misc.c         | 100 ++++++++++++++++++++++++++++---------
>   tools/misc/xen-livepatch.c    | 112 ++++++++++++++++++++++--------------------
>   xen/common/livepatch.c        |  31 +++++++++---
>   xen/include/public/sysctl.h   |  15 +++---
>   6 files changed, 219 insertions(+), 112 deletions(-)
> 
> diff --git a/docs/misc/livepatch.pandoc b/docs/misc/livepatch.pandoc
> index 406fb79df8..e7bcc70f5a 100644
> --- a/docs/misc/livepatch.pandoc
> +++ b/docs/misc/livepatch.pandoc
> @@ -717,17 +717,20 @@ The caller provides:
>    * `idx` Index iterator. The index into the hypervisor's payload count. It is
>       recommended that on first invocation zero be used so that `nr` (which the
>       hypervisor will update with the remaining payload count) be provided.
> -    Also the hypervisor will provide `version` with the most current value.
> +    Also the hypervisor will provide `version` with the most current value and
> +    calculated total size for all payloads' names.
>    * `nr` The max number of entries to populate. Can be zero which will result
>       in the hypercall being a probing one and return the number of payloads
>       (and update the `version`).
>    * `pad` - *MUST* be zero.
>    * `status` Virtual address of where to write `struct xen_livepatch_status`
>      structures. Caller *MUST* allocate up to `nr` of them.
> - * `name` - Virtual address of where to write the unique name of the payload.
> -   Caller *MUST* allocate up to `nr` of them. Each *MUST* be of
> -   **XEN_LIVEPATCH_NAME_SIZE** size. Note that **XEN_LIVEPATCH_NAME_SIZE** includes
> -   the NUL terminator.
> + * `name` - Virtual address of where to write the unique name of the payloads.
> +   Caller *MUST* allocate enough space to be able to store all received data
> +   (i.e. total allocated space *MUST* match the `name_total_size` value
> +   provided by the hypervisor). Individual payload name cannot be longer than
> +   **XEN_LIVEPATCH_NAME_SIZE** bytes. Note that **XEN_LIVEPATCH_NAME_SIZE**
> +   includes the NUL terminator.
>    * `len` - Virtual address of where to write the length of each unique name
>      of the payload. Caller *MUST* allocate up to `nr` of them. Each *MUST* be
>      of sizeof(uint32_t) (4 bytes).
> @@ -736,7 +739,8 @@ If the hypercall returns an positive number, it is the number (upto `nr`
>   provided to the hypercall) of the payloads returned, along with `nr` updated
>   with the number of remaining payloads, `version` updated (it may be the same
>   across hypercalls - if it varies the data is stale and further calls could
> -fail). The `status`, `name`, and `len` are updated at their designed index
> +fail) and the `name_total_size` containing total size of transfered data for

transferred

> +the array. The `status`, `name`, and `len` are updated at their designed index
>   value (`idx`) with the returned value of data.
>   
>   If the hypercall returns -XEN_E2BIG the `nr` is too big and should be
> @@ -775,11 +779,13 @@ The structure is as follow:
>                                                      amount of payloads and version.
>                                                      OUT: How many payloads left. */
>           uint32_t pad;                           /* IN: Must be zero. */
> +        uint64_t name_total_size;               /* OUT: Total size of all transfer names */
>           XEN_GUEST_HANDLE_64(xen_livepatch_status_t) status;  /* OUT. Must have enough
>                                                      space allocate for nr of them. */
> -        XEN_GUEST_HANDLE_64(char) id;           /* OUT: Array of names. Each member
> -                                                   MUST XEN_LIVEPATCH_NAME_SIZE in size.
> -                                                   Must have nr of them. */
> +        XEN_GUEST_HANDLE_64(char) name;         /* OUT: Array of names. Each member
> +                                                   may have an arbitrary length up to
> +                                                   XEN_LIVEPATCH_NAME_SIZE bytes. Must have
> +                                                   nr of them. */
>           XEN_GUEST_HANDLE_64(uint32) len;        /* OUT: Array of lengths of name's.
>                                                      Must have nr of them. */
>       };
> diff --git a/tools/misc/xen-livepatch.c b/tools/misc/xen-livepatch.c
> index a37b2457ff..8ac3d567fc 100644
> --- a/tools/misc/xen-livepatch.c
> +++ b/tools/misc/xen-livepatch.c
> @@ -64,14 +64,14 @@ static const char *state2str(unsigned int state)
>       return names[state];
>   }
>   
> -/* This value was choosen adhoc. It could be 42 too. */
> -#define MAX_LEN 11
>   static int list_func(int argc, char *argv[])
>   {
> -    unsigned int idx, done, left, i;
> +    unsigned int nr, done, left, i;
>       xen_livepatch_status_t *info = NULL;
>       char *name = NULL;
>       uint32_t *len = NULL;
> +    uint64_t name_total_size;
> +    off_t name_off;

If name_total_size becomes 32-bit, then I think you can replace the few 
usages of off_t with just a uint32_t (it doesn't need to be signed).

Otherwise looks good to me.

Thanks,
diff mbox series

Patch

diff --git a/docs/misc/livepatch.pandoc b/docs/misc/livepatch.pandoc
index 406fb79df8..e7bcc70f5a 100644
--- a/docs/misc/livepatch.pandoc
+++ b/docs/misc/livepatch.pandoc
@@ -717,17 +717,20 @@  The caller provides:
  * `idx` Index iterator. The index into the hypervisor's payload count. It is
     recommended that on first invocation zero be used so that `nr` (which the
     hypervisor will update with the remaining payload count) be provided.
-    Also the hypervisor will provide `version` with the most current value.
+    Also the hypervisor will provide `version` with the most current value and
+    calculated total size for all payloads' names.
  * `nr` The max number of entries to populate. Can be zero which will result
     in the hypercall being a probing one and return the number of payloads
     (and update the `version`).
  * `pad` - *MUST* be zero.
  * `status` Virtual address of where to write `struct xen_livepatch_status`
    structures. Caller *MUST* allocate up to `nr` of them.
- * `name` - Virtual address of where to write the unique name of the payload.
-   Caller *MUST* allocate up to `nr` of them. Each *MUST* be of
-   **XEN_LIVEPATCH_NAME_SIZE** size. Note that **XEN_LIVEPATCH_NAME_SIZE** includes
-   the NUL terminator.
+ * `name` - Virtual address of where to write the unique name of the payloads.
+   Caller *MUST* allocate enough space to be able to store all received data
+   (i.e. total allocated space *MUST* match the `name_total_size` value
+   provided by the hypervisor). Individual payload name cannot be longer than
+   **XEN_LIVEPATCH_NAME_SIZE** bytes. Note that **XEN_LIVEPATCH_NAME_SIZE**
+   includes the NUL terminator.
  * `len` - Virtual address of where to write the length of each unique name
    of the payload. Caller *MUST* allocate up to `nr` of them. Each *MUST* be
    of sizeof(uint32_t) (4 bytes).
@@ -736,7 +739,8 @@  If the hypercall returns an positive number, it is the number (upto `nr`
 provided to the hypercall) of the payloads returned, along with `nr` updated
 with the number of remaining payloads, `version` updated (it may be the same
 across hypercalls - if it varies the data is stale and further calls could
-fail). The `status`, `name`, and `len` are updated at their designed index
+fail) and the `name_total_size` containing total size of transfered data for
+the array. The `status`, `name`, and `len` are updated at their designed index
 value (`idx`) with the returned value of data.
 
 If the hypercall returns -XEN_E2BIG the `nr` is too big and should be
@@ -775,11 +779,13 @@  The structure is as follow:
                                                    amount of payloads and version.
                                                    OUT: How many payloads left. */
         uint32_t pad;                           /* IN: Must be zero. */
+        uint64_t name_total_size;               /* OUT: Total size of all transfer names */
         XEN_GUEST_HANDLE_64(xen_livepatch_status_t) status;  /* OUT. Must have enough
                                                    space allocate for nr of them. */
-        XEN_GUEST_HANDLE_64(char) id;           /* OUT: Array of names. Each member
-                                                   MUST XEN_LIVEPATCH_NAME_SIZE in size.
-                                                   Must have nr of them. */
+        XEN_GUEST_HANDLE_64(char) name;         /* OUT: Array of names. Each member
+                                                   may have an arbitrary length up to
+                                                   XEN_LIVEPATCH_NAME_SIZE bytes. Must have
+                                                   nr of them. */
         XEN_GUEST_HANDLE_64(uint32) len;        /* OUT: Array of lengths of name's.
                                                    Must have nr of them. */
     };
diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 2fc62422f5..252675f117 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -2551,7 +2551,25 @@  int xc_livepatch_get(xc_interface *xch,
                      xen_livepatch_status_t *status);
 
 /*
- * The heart of this function is to get an array of xen_livepatch_status_t.
+ * Get a number of available payloads and get actual total size of
+ * the payloads' name array.
+ *
+ * This functions is typically executed first before the xc_livepatch_list()
+ * to obtain the sizes and correctly allocate all necessary data resources.
+ *
+ * The return value is zero if the hypercall completed successfully.
+ *
+ * If there was an error performing the sysctl operation, the return value
+ * will contain the hypercall error code value.
+ */
+int xc_livepatch_list_get_sizes(xc_interface *xch, unsigned int *nr,
+                                uint64_t *name_total_size);
+
+/*
+ * The heart of this function is to get an array of the following objects:
+ *   - xen_livepatch_status_t: states and return codes of payloads
+ *   - name: names of payloads
+ *   - len: lengths of corresponding payloads' names
  *
  * However it is complex because it has to deal with the hypervisor
  * returning some of the requested data or data being stale
@@ -2562,21 +2580,20 @@  int xc_livepatch_get(xc_interface *xch,
  * 'left' are also updated with the number of entries filled out
  * and respectively the number of entries left to get from hypervisor.
  *
- * It is expected that the caller of this function will take the
- * 'left' and use the value for 'start'. This way we have an
- * cursor in the array. Note that the 'info','name', and 'len' will
- * be updated at the subsequent calls.
+ * It is expected that the caller of this function will first issue the
+ * xc_livepatch_list_get_sizes() in order to obtain total sizes of names
+ * as well as the current number of payload entries.
+ * The total sizes are required and supplied via the 'name_total_size'
+ * parameter.
  *
- * The 'max' is to be provided by the caller with the maximum
- * number of entries that 'info', 'name', and 'len' arrays can
- * be filled up with.
- *
- * Each entry in the 'name' array is expected to be of XEN_LIVEPATCH_NAME_SIZE
- * length.
+ * The 'max' is to be provided by the caller with the maximum number of
+ * entries that 'info', 'name', 'len' arrays can be filled up with.
  *
  * Each entry in the 'info' array is expected to be of xen_livepatch_status_t
  * structure size.
  *
+ * Each entry in the 'name' array may have an arbitrary size.
+ *
  * Each entry in the 'len' array is expected to be of uint32_t size.
  *
  * The return value is zero if the hypercall completed successfully.
@@ -2588,10 +2605,12 @@  int xc_livepatch_get(xc_interface *xch,
  * will contain the number of entries that had been succesfully
  * retrieved (if any).
  */
-int xc_livepatch_list(xc_interface *xch, unsigned int max, unsigned int start,
-                      xen_livepatch_status_t *info, char *name,
-                      uint32_t *len, unsigned int *done,
-                      unsigned int *left);
+int xc_livepatch_list(xc_interface *xch, const unsigned int max,
+                      const unsigned int start,
+                      struct xen_livepatch_status *info,
+                      char *name, uint32_t *len,
+                      const uint64_t name_total_size,
+                      unsigned int *done, unsigned int *left);
 
 /*
  * The operations are asynchronous and the hypervisor may take a while
diff --git a/tools/libxc/xc_misc.c b/tools/libxc/xc_misc.c
index a8e9e7d1e2..d787f3f29f 100644
--- a/tools/libxc/xc_misc.c
+++ b/tools/libxc/xc_misc.c
@@ -662,7 +662,48 @@  int xc_livepatch_get(xc_interface *xch,
 }
 
 /*
- * The heart of this function is to get an array of xen_livepatch_status_t.
+ * Get a number of available payloads and get actual total size of
+ * the payloads' name array.
+ *
+ * This functions is typically executed first before the xc_livepatch_list()
+ * to obtain the sizes and correctly allocate all necessary data resources.
+ *
+ * The return value is zero if the hypercall completed successfully.
+ *
+ * If there was an error performing the sysctl operation, the return value
+ * will contain the hypercall error code value.
+ */
+int xc_livepatch_list_get_sizes(xc_interface *xch, unsigned int *nr,
+                                uint64_t *name_total_size)
+{
+    DECLARE_SYSCTL;
+    int rc;
+
+    if ( !nr || !name_total_size )
+    {
+        errno = EINVAL;
+        return -1;
+    }
+
+    memset(&sysctl, 0, sizeof(sysctl));
+    sysctl.cmd = XEN_SYSCTL_livepatch_op;
+    sysctl.u.livepatch.cmd = XEN_SYSCTL_LIVEPATCH_LIST;
+
+    rc = do_sysctl(xch, &sysctl);
+    if ( rc )
+        return rc;
+
+    *nr = sysctl.u.livepatch.u.list.nr;
+    *name_total_size = sysctl.u.livepatch.u.list.name_total_size;
+
+    return 0;
+}
+
+/*
+ * The heart of this function is to get an array of the following objects:
+ *   - xen_livepatch_status_t: states and return codes of payloads
+ *   - name: names of payloads
+ *   - len: lengths of corresponding payloads' names
  *
  * However it is complex because it has to deal with the hypervisor
  * returning some of the requested data or data being stale
@@ -673,21 +714,20 @@  int xc_livepatch_get(xc_interface *xch,
  * 'left' are also updated with the number of entries filled out
  * and respectively the number of entries left to get from hypervisor.
  *
- * It is expected that the caller of this function will take the
- * 'left' and use the value for 'start'. This way we have an
- * cursor in the array. Note that the 'info','name', and 'len' will
- * be updated at the subsequent calls.
+ * It is expected that the caller of this function will first issue the
+ * xc_livepatch_list_get_sizes() in order to obtain total sizes of names
+ * as well as the current number of payload entries.
+ * The total sizes are required and supplied via the 'name_total_size'
+ * parameter.
  *
- * The 'max' is to be provided by the caller with the maximum
- * number of entries that 'info', 'name', and 'len' arrays can
- * be filled up with.
- *
- * Each entry in the 'name' array is expected to be of XEN_LIVEPATCH_NAME_SIZE
- * length.
+ * The 'max' is to be provided by the caller with the maximum number of
+ * entries that 'info', 'name', 'len' arrays can be filled up with.
  *
  * Each entry in the 'info' array is expected to be of xen_livepatch_status_t
  * structure size.
  *
+ * Each entry in the 'name' array may have an arbitrary size.
+ *
  * Each entry in the 'len' array is expected to be of uint32_t size.
  *
  * The return value is zero if the hypercall completed successfully.
@@ -699,11 +739,12 @@  int xc_livepatch_get(xc_interface *xch,
  * will contain the number of entries that had been succesfully
  * retrieved (if any).
  */
-int xc_livepatch_list(xc_interface *xch, unsigned int max, unsigned int start,
+int xc_livepatch_list(xc_interface *xch, const unsigned int max,
+                      const unsigned int start,
                       struct xen_livepatch_status *info,
                       char *name, uint32_t *len,
-                      unsigned int *done,
-                      unsigned int *left)
+                      const uint64_t name_total_size,
+                      unsigned int *done, unsigned int *left)
 {
     int rc;
     DECLARE_SYSCTL;
@@ -714,27 +755,33 @@  int xc_livepatch_list(xc_interface *xch, unsigned int max, unsigned int start,
     uint32_t max_batch_sz, nr;
     uint32_t version = 0, retries = 0;
     uint32_t adjust = 0;
-    ssize_t sz;
+    off_t name_off = 0;
+    uint64_t name_sz;
 
-    if ( !max || !info || !name || !len )
+    if ( !max || !info || !name || !len || !done || !left )
     {
         errno = EINVAL;
         return -1;
     }
 
+    if ( name_total_size == 0 )
+    {
+        errno = ENOENT;
+        return -1;
+    }
+
+    memset(&sysctl, 0, sizeof(sysctl));
     sysctl.cmd = XEN_SYSCTL_livepatch_op;
     sysctl.u.livepatch.cmd = XEN_SYSCTL_LIVEPATCH_LIST;
-    sysctl.u.livepatch.pad = 0;
-    sysctl.u.livepatch.u.list.version = 0;
     sysctl.u.livepatch.u.list.idx = start;
-    sysctl.u.livepatch.u.list.pad = 0;
 
     max_batch_sz = max;
-    /* Convience value. */
-    sz = sizeof(*name) * XEN_LIVEPATCH_NAME_SIZE;
+    name_sz = name_total_size;
     *done = 0;
     *left = 0;
     do {
+        uint64_t _name_sz;
+
         /*
          * The first time we go in this loop our 'max' may be bigger
          * than what the hypervisor is comfortable with - hence the first
@@ -754,11 +801,11 @@  int xc_livepatch_list(xc_interface *xch, unsigned int max, unsigned int start,
         sysctl.u.livepatch.u.list.nr = nr;
         /* Fix the size (may vary between hypercalls). */
         HYPERCALL_BOUNCE_SET_SIZE(info, nr * sizeof(*info));
-        HYPERCALL_BOUNCE_SET_SIZE(name, nr * nr);
+        HYPERCALL_BOUNCE_SET_SIZE(name, name_sz);
         HYPERCALL_BOUNCE_SET_SIZE(len, nr * sizeof(*len));
         /* Move the pointer to proper offset into 'info'. */
         (HYPERCALL_BUFFER(info))->ubuf = info + *done;
-        (HYPERCALL_BUFFER(name))->ubuf = name + (sz * *done);
+        (HYPERCALL_BUFFER(name))->ubuf = name + name_off;
         (HYPERCALL_BUFFER(len))->ubuf = len + *done;
         /* Allocate memory. */
         rc = xc_hypercall_bounce_pre(xch, info);
@@ -827,14 +874,19 @@  int xc_livepatch_list(xc_interface *xch, unsigned int max, unsigned int start,
             break;
         }
         *left = sysctl.u.livepatch.u.list.nr; /* Total remaining count. */
+        _name_sz = sysctl.u.livepatch.u.list.name_total_size; /* Total received name size. */
         /* Copy only up 'rc' of data' - we could add 'min(rc,nr) if desired. */
         HYPERCALL_BOUNCE_SET_SIZE(info, (rc * sizeof(*info)));
-        HYPERCALL_BOUNCE_SET_SIZE(name, (rc * sz));
+        HYPERCALL_BOUNCE_SET_SIZE(name, _name_sz);
         HYPERCALL_BOUNCE_SET_SIZE(len, (rc * sizeof(*len)));
         /* Bounce the data and free the bounce buffer. */
         xc_hypercall_bounce_post(xch, info);
         xc_hypercall_bounce_post(xch, name);
         xc_hypercall_bounce_post(xch, len);
+
+        name_sz -= _name_sz;
+        name_off += _name_sz;
+
         /* And update how many elements of info we have copied into. */
         *done += rc;
         /* Update idx. */
diff --git a/tools/misc/xen-livepatch.c b/tools/misc/xen-livepatch.c
index a37b2457ff..8ac3d567fc 100644
--- a/tools/misc/xen-livepatch.c
+++ b/tools/misc/xen-livepatch.c
@@ -64,14 +64,14 @@  static const char *state2str(unsigned int state)
     return names[state];
 }
 
-/* This value was choosen adhoc. It could be 42 too. */
-#define MAX_LEN 11
 static int list_func(int argc, char *argv[])
 {
-    unsigned int idx, done, left, i;
+    unsigned int nr, done, left, i;
     xen_livepatch_status_t *info = NULL;
     char *name = NULL;
     uint32_t *len = NULL;
+    uint64_t name_total_size;
+    off_t name_off;
     int rc = ENOMEM;
 
     if ( argc )
@@ -79,65 +79,73 @@  static int list_func(int argc, char *argv[])
         show_help();
         return -1;
     }
-    idx = left = 0;
-    info = malloc(sizeof(*info) * MAX_LEN);
-    if ( !info )
-        return rc;
-    name = malloc(sizeof(*name) * XEN_LIVEPATCH_NAME_SIZE * MAX_LEN);
-    if ( !name )
+    done = left = 0;
+
+    rc = xc_livepatch_list_get_sizes(xch, &nr, &name_total_size);
+    if ( rc )
     {
-        free(info);
+        rc = errno;
+        fprintf(stderr, "Failed to get list sizes.\n"
+                "Error %d: %s\n",
+                rc, strerror(rc));
         return rc;
     }
-    len = malloc(sizeof(*len) * MAX_LEN);
-    if ( !len ) {
-        free(name);
-        free(info);
+
+    if ( nr == 0 )
+    {
+        fprintf(stdout, "Nothing to list\n");
+        return 0;
+    }
+
+    info = malloc(nr * sizeof(*info));
+    if ( !info )
         return rc;
+
+    name = malloc(name_total_size * sizeof(*name));
+    if ( !name )
+        goto error_name;
+
+    len = malloc(nr * sizeof(*len));
+    if ( !len )
+        goto error_len;
+
+    memset(info, 'A', nr * sizeof(*info));
+    memset(name, 'B', name_total_size * sizeof(*name));
+    memset(len, 'C', nr * sizeof(*len));
+    name_off = 0;
+
+    rc = xc_livepatch_list(xch, nr, 0, info, name, len, name_total_size, &done, &left);
+    if ( rc || done != nr || left > 0)
+    {
+        rc = errno;
+        fprintf(stderr, "Failed to list %d/%d.\n"
+                "Error %d: %s\n",
+                left, nr, rc, strerror(rc));
+        goto error;
     }
 
-    do {
-        done = 0;
-        /* The memset is done to catch errors. */
-        memset(info, 'A', sizeof(*info) * MAX_LEN);
-        memset(name, 'B', sizeof(*name) * MAX_LEN * XEN_LIVEPATCH_NAME_SIZE);
-        memset(len, 'C', sizeof(*len) * MAX_LEN);
-        rc = xc_livepatch_list(xch, MAX_LEN, idx, info, name, len, &done, &left);
-        if ( rc )
-        {
-            rc = errno;
-            fprintf(stderr, "Failed to list %d/%d.\n"
-                            "Error %d: %s\n",
-                    idx, left, rc, strerror(rc));
-            break;
-        }
-        if ( !idx )
-            fprintf(stdout," ID                                     | status\n"
-                           "----------------------------------------+------------\n");
+    fprintf(stdout," ID                                     | status\n"
+                   "----------------------------------------+------------\n");
 
-        for ( i = 0; i < done; i++ )
-        {
-            unsigned int j;
-            uint32_t sz;
-            char *str;
-
-            sz = len[i];
-            str = name + (i * XEN_LIVEPATCH_NAME_SIZE);
-            for ( j = sz; j < XEN_LIVEPATCH_NAME_SIZE; j++ )
-                str[j] = '\0';
-
-            printf("%-40s| %s", str, state2str(info[i].state));
-            if ( info[i].rc )
-                printf(" (%d, %s)\n", -info[i].rc, strerror(-info[i].rc));
-            else
-                puts("");
-        }
-        idx += done;
-    } while ( left );
+    for ( i = 0; i < done; i++ )
+    {
+        char *name_str = name + name_off;
+
+        printf("%-40.*s| %s", len[i], name_str, state2str(info[i].state));
+        if ( info[i].rc )
+            printf(" (%d, %s)\n", -info[i].rc, strerror(-info[i].rc));
+        else
+            puts("");
+
+        name_off += len[i];
+    }
 
+error:
+    free(len);
+error_len:
     free(name);
+error_name:
     free(info);
-    free(len);
     return rc;
 }
 #undef MAX_LEN
diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
index 18935f608c..e800382479 100644
--- a/xen/common/livepatch.c
+++ b/xen/common/livepatch.c
@@ -1164,7 +1164,6 @@  static int livepatch_list(struct xen_sysctl_livepatch_list *list)
 
     if ( list->nr &&
          (!guest_handle_okay(list->status, list->nr) ||
-          !guest_handle_okay(list->name, XEN_LIVEPATCH_NAME_SIZE * list->nr) ||
           !guest_handle_okay(list->len, list->nr)) )
         return -EINVAL;
 
@@ -1175,23 +1174,35 @@  static int livepatch_list(struct xen_sysctl_livepatch_list *list)
         return -EINVAL;
     }
 
+    list->name_total_size = 0;
     if ( list->nr )
     {
+        uint64_t name_offset = 0;
+
         list_for_each_entry( data, &payload_list, list )
         {
-            uint32_t len;
+            uint32_t name_len;
 
             if ( list->idx > i++ )
                 continue;
 
             status.state = data->state;
             status.rc = data->rc;
-            len = strlen(data->name) + 1;
+
+            name_len = strlen(data->name) + 1;
+            list->name_total_size += name_len;
+
+            if ( !guest_handle_subrange_okay(list->name, name_offset,
+                                             name_offset + name_len - 1) )
+            {
+                rc = -EINVAL;
+                break;
+            }
 
             /* N.B. 'idx' != 'i'. */
-            if ( __copy_to_guest_offset(list->name, idx * XEN_LIVEPATCH_NAME_SIZE,
-                                        data->name, len) ||
-                __copy_to_guest_offset(list->len, idx, &len, 1) ||
+            if ( __copy_to_guest_offset(list->name, name_offset,
+                                        data->name, name_len) ||
+                __copy_to_guest_offset(list->len, idx, &name_len, 1) ||
                 __copy_to_guest_offset(list->status, idx, &status, 1) )
             {
                 rc = -EFAULT;
@@ -1199,11 +1210,19 @@  static int livepatch_list(struct xen_sysctl_livepatch_list *list)
             }
 
             idx++;
+            name_offset += name_len;
 
             if ( (idx >= list->nr) || hypercall_preempt_check() )
                 break;
         }
     }
+    else
+    {
+        list_for_each_entry( data, &payload_list, list )
+        {
+            list->name_total_size += strlen(data->name) + 1;
+        }
+    }
     list->nr = payload_cnt - i; /* Remaining amount. */
     list->version = payload_version;
     spin_unlock(&payload_lock);
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index bcdfc1fafe..503be68059 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -925,10 +925,11 @@  struct xen_sysctl_livepatch_get {
  *
  * If the hypercall returns an positive number, it is the number (up to `nr`)
  * of the payloads returned, along with `nr` updated with the number of remaining
- * payloads, `version` updated (it may be the same across hypercalls. If it
- * varies the data is stale and further calls could fail). The `status`,
- * `name`, and `len`' are updated at their designed index value (`idx`) with
- * the returned value of data.
+ * payloads, `version` updated (it may be the same across hypercalls. If it varies
+ * the data is stale and further calls could fail) and the name_total_size
+ * containing total size of transfered data for the array.
+ * The `status`, `name`, `len` are updated at their designed index value (`idx`)
+ * with the returned value of data.
  *
  * If the hypercall returns E2BIG the `nr` is too big and should be
  * lowered. The upper limit of `nr` is left to the implemention.
@@ -951,11 +952,13 @@  struct xen_sysctl_livepatch_list {
                                                amount of payloads and version.
                                                OUT: How many payloads left. */
     uint32_t pad;                           /* IN: Must be zero. */
+    uint64_t name_total_size;               /* OUT: Total size of all transfer names */
     XEN_GUEST_HANDLE_64(xen_livepatch_status_t) status;  /* OUT. Must have enough
                                                space allocate for nr of them. */
     XEN_GUEST_HANDLE_64(char) name;         /* OUT: Array of names. Each member
-                                               MUST XEN_LIVEPATCH_NAME_SIZE in size.
-                                               Must have nr of them. */
+                                               may have an arbitrary length up to
+                                               XEN_LIVEPATCH_NAME_SIZE bytes. Must have
+                                               nr of them. */
     XEN_GUEST_HANDLE_64(uint32) len;        /* OUT: Array of lengths of name's.
                                                Must have nr of them. */
 };