From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH 6/7] libxc: Fix misleading use of strncpy code in build_hvm_info() Date: Fri, 3 Jul 2015 15:41:49 +0100 Message-ID: <55969F2D.7050404@citrix.com> References: <1435772232-39085-1-git-send-email-Jennifer.Herbert@citrix.com> <1435772232-39085-7-git-send-email-Jennifer.Herbert@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1435772232-39085-7-git-send-email-Jennifer.Herbert@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jennifer Herbert , xen-devel@lists.xen.org Cc: ian.jackson@eu.citrix.com, wei.liu2@citrix.com, ian.campbell@citrix.com, stefano.stabellini@eu.citrix.com List-Id: xen-devel@lists.xenproject.org On 01/07/15 18:37, Jennifer Herbert wrote: > hvm_info->signature is not a string, but an 64 bit int, and is not > NULL terminated. The use of strncpy to populate it is inappropriate and > potentially misleading. A cursory glance might have you thinking someone > had miscounted the length of the string literal - not realising it was > intentionally cropping of the null termination. > Also, since we wish to initialise all of hvm_info->signature, and > certainly no more, the use of sizeof is safer. > > Signed-off-by: Jennifer Herbert Coverity-ID: 1198710 ~Andrew > --- > tools/libxc/xc_hvm_build_x86.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tools/libxc/xc_hvm_build_x86.c b/tools/libxc/xc_hvm_build_x86.c > index 003ea06..ec5ef4d 100644 > --- a/tools/libxc/xc_hvm_build_x86.c > +++ b/tools/libxc/xc_hvm_build_x86.c > @@ -99,7 +99,7 @@ static void build_hvm_info(void *hvm_info_page, > memset(hvm_info_page, 0, PAGE_SIZE); > > /* Fill in the header. */ > - strncpy(hvm_info->signature, "HVM INFO", 8); > + memcpy(hvm_info->signature, "HVM INFO", sizeof(hvm_info->signature)); > hvm_info->length = sizeof(struct hvm_info_table); > > /* Sensible defaults: these can be overridden by the caller. */