Re: [PATCH 3/7] kvmtool: fix strcpy vulnerabilities

[Will Deacon <will.deacon@arm.com>]

On Tue, Oct 18, 2016 at 06:02:52PM +0200, G. Campana wrote:
> ---
>  virtio/9p.c | 60 +++++++++++++++++++++++++++++++++++++++++++-----------------
>  1 file changed, 43 insertions(+), 17 deletions(-)
> 
> diff --git a/virtio/9p.c b/virtio/9p.c
> index 9695540..cd93d06 100644
> --- a/virtio/9p.c
> +++ b/virtio/9p.c
> @@ -28,6 +28,7 @@ static struct p9_fid *find_or_create_fid(struct p9_dev *dev, u32 fid)
>  {
>  	struct rb_node *node = dev->fids.rb_node;
>  	struct p9_fid *pfid = NULL;
> +	size_t len;
>  
>  	while (node) {
>  		struct p9_fid *cur = rb_entry(node, struct p9_fid, node);
> @@ -45,9 +46,15 @@ static struct p9_fid *find_or_create_fid(struct p9_dev *dev, u32 fid)
>  	if (!pfid)
>  		return NULL;
>  
> +	len = strlen(dev->root_dir);
> +	if (len >= sizeof(pfid->path)) {
> +		free(pfid);
> +		return NULL;
> +	}

This doesn't make sense to me -- pfid->path is just a NULL ptr at this
stage. Did you mean abs_path?

> +
>  	pfid->fid = fid;
> -	strcpy(pfid->abs_path, dev->root_dir);
> -	pfid->path = pfid->abs_path + strlen(dev->root_dir);
> +	strncpy(pfid->abs_path, dev->root_dir, sizeof(pfid->abs_path));

But if you did mean abs_path, why use strncpy here?

Will