[libvirt] Sys::Virt Perl bindings - calling newSVpv with unsafe length

Daniel P. Berrange berrange at redhat.com
Tue Apr 6 09:46:00 UTC 2010


On Sat, Apr 03, 2010 at 09:21:41AM +0100, Richard W.M. Jones wrote:
> Dan, 
> 
> After tracking down a very obscure bug in hivex & libguestfs Perl
> bindings, I started looking for places where we call newSVpv with a
> variable length argument that could be zero.  In such cases, one
> should call newSVpvn instead, otherwise Perl will try to use strlen()
> to calculate the length of the data buffer when length is passed in as
> zero [I'm sure you are aware of this already].

Ewwww, totally forgot about that.

> There seems to be one such case in the Sys::Virt bindings:
> 
> http://cpansearch.perl.org/src/DANBERR/Sys-Virt-0.2.3/Virt.xs
> 
>     CODE:
>       if ((bytes = virSecretGetValue(sec, &len, flags)) == NULL) {
>       _croak_error(virConnGetLastError(virSecretGetConnect(sec)));
>       }
>       RETVAL = newSVpv((char*)bytes, len);
> 
> Maybe 'len' can never be zero here, but I think it's safer to turn
> this into a call to newSVpvn anyway.

Agreed, I think it can actually be zero.

> Also, if you look at the source to newSVpv:
> 
> http://github.com/mirrors/perl/blob/blead/sv.c#L7752
> 
> You'll see there is an extra test, not present with newSVpvn.  So it's
> better to always call newSVpvn wherever there is a constant-sized
> buffer, eg in code like this:
> 
>       if ((virDomainGetUUID(dom, rawuuid)) < 0) {
>         _croak_error(virConnGetLastError(virDomainGetConnect(dom)));
>       }
>       RETVAL = newSVpv((char*)rawuuid, sizeof(rawuuid));

Yep, will update this all.


Daniel
-- 
|: Red Hat, Engineering, London    -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org        -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|




More information about the libvir-list mailing list