On 08/11/2015 02:05 PM, Laine Stump wrote: > This fixes the crash described here: > > https://www.redhat.com/archives/libvir-list/2015-August/msg00162.html > > In short, we were calling ioctl(SIOCETHTOOL) pointing to a too-short > object that was a local on the stack, resulting in the memory past the > end of the object being overwritten. This was because the struct used > by the ETHTOOL_GFEATURES command of SIOCETHTOOL ends with a 0-length > array, but we were telling ethtool that it could use 2 elements on the > array. > > The fix is to allocate the necessary memory with VIR_ALLOC_VAR(), > including the extra length needed for a 2 element array at the end. > --- > > V2 difference: eliminate all the other cleanups that were in V1, just > fix the crash. I had misunderstood that a "failure" of the ioctl > shouldn't really be considered a *failure*, and don't have time to > tweak around with all the inconsequential stuff right now. > > src/util/virnetdev.c | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) > Looks reasonable to me and since it fixes the crash... Not sure how I missed the static stack variable in the original patch (and only the dog has been wearing the cone of shame recently ;-)) ACK John > diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c > index 1e20789..c158ca3 100644 > --- a/src/util/virnetdev.c > +++ b/src/util/virnetdev.c > @@ -3130,7 +3130,7 @@ virNetDevGetFeatures(const char *ifname, > size_t i = -1; > struct ethtool_value cmd = { 0 }; > # if HAVE_DECL_ETHTOOL_GFEATURES > - struct ethtool_gfeatures g_cmd = { 0 }; > + struct ethtool_gfeatures *g_cmd; > # endif > struct elem{ > const int cmd; > @@ -3188,10 +3188,14 @@ virNetDevGetFeatures(const char *ifname, > # endif > > # if HAVE_DECL_ETHTOOL_GFEATURES > - g_cmd.cmd = ETHTOOL_GFEATURES; > - g_cmd.size = GFEATURES_SIZE; > - if (virNetDevGFeatureAvailable(ifname, &g_cmd)) > + if (VIR_ALLOC_VAR(g_cmd, > + struct ethtool_get_features_block, GFEATURES_SIZE) < 0) > + return -1; > + g_cmd->cmd = ETHTOOL_GFEATURES; > + g_cmd->size = GFEATURES_SIZE; > + if (virNetDevGFeatureAvailable(ifname, g_cmd)) > ignore_value(virBitmapSetBit(*out, VIR_NET_DEV_FEAT_TXUDPTNL)); > + VIR_FREE(g_cmd); > # endif > > if (virNetDevRDMAFeature(ifname, out) < 0) > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list