Re: [Xen-devel] [PATCH] hvmloader/smbios: Change strncpy to memcpy for anchor strings.

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: xen-devel-bounces@xxxxxxxxxxxxx [mailto:xen-devel-
> bounces@xxxxxxxxxxxxx] On Behalf Of Andrew Cooper
> Sent: 23 September 2013 19:22
> To: Xen-devel
> Cc: Andrew Cooper; Keir (Xen.org); Jan Beulich
> Subject: [Xen-devel] [PATCH] hvmloader/smbios: Change strncpy to
> memcpy for anchor strings.
> 
> Coverity complains about the use of strncpy() to completely fill the anchor
> strings, resulting in an unterminated string.
> 
> Although the strncpy result is correct, the anchor strings are not strings in
> the C sense, and use of memcpy is the prevaling style elsewhere in
> hvmloader
> anyway.
> 
> While tidying up the style in this function, also remove some trailing
> whitespace and gratuitous cast.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> CC: Keir Fraser <keir@xxxxxxx>
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> ---
>  tools/firmware/hvmloader/smbios.c |    9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/firmware/hvmloader/smbios.c
> b/tools/firmware/hvmloader/smbios.c
> index 9f292cc..900f4e7 100644
> --- a/tools/firmware/hvmloader/smbios.c
> +++ b/tools/firmware/hvmloader/smbios.c
> @@ -347,18 +347,19 @@ smbios_entry_point_init(void *start,
>  {
>      uint8_t sum;
>      int i;
> -    struct smbios_entry_point *ep = (struct smbios_entry_point *)start;
> +    struct smbios_entry_point *ep = start;
> 
>      memset(ep, 0, sizeof(*ep));
> 
> -    strncpy(ep->anchor_string, "_SM_", 4);
> +    memcpy(ep->anchor_string, "_SM_", sizeof(ep->anchor_string));

Why the change from 4 to sizeof(ep->anchor_string) here (and similar below)? 
Setting the copy length based on the size of the destination rather than the 
source seems like the wrong thing to do.

  Paul

>      ep->length = 0x1f;
>      ep->smbios_major_version = 2;
>      ep->smbios_minor_version = 4;
>      ep->max_structure_size = max_structure_size;
>      ep->entry_point_revision = 0;
> -    strncpy(ep->intermediate_anchor_string, "_DMI_", 5);
> -
> +    memcpy(ep->intermediate_anchor_string, "_DMI_",
> +           sizeof(ep->intermediate_anchor_string));
> +
>      ep->structure_table_length = structure_table_length;
>      ep->structure_table_address = structure_table_address;
>      ep->number_of_structures = number_of_structures;
> --
> 1.7.10.4
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel