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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Nov 10, 2016 at 04:15:12PM +0100, G. Campana wrote:
> On 08/11/2016 03:08, Will Deacon wrote:
> > 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?
> > 
> Good catch. Indeed, I meant 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?
> > 
> It ensures that the string is null terminated.

Given that we already did a strlen on root_dir, and we know that abs_path
is big enough to hold it, I still think this isn't needed.

Will
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux