[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