> > +int ethnl_cable_test_alloc(struct phy_device *phydev)
> > +{
> > + int err = -ENOMEM;
> > +
> > + phydev->skb = genlmsg_new(NLMSG_GOODSIZE, GFP_KERNEL);
> > + if (!phydev->skb)
> > + goto out;
> > +
> > + phydev->ehdr = ethnl_bcastmsg_put(phydev->skb,
> > + ETHTOOL_MSG_CABLE_TEST_NTF);
> > + if (!phydev->ehdr) {
> > + err = -EINVAL;
>
> This should be -EMSGSIZE.
>
> > + goto out;
> > + }
> > +
> > + err = ethnl_fill_reply_header(phydev->skb, phydev->attached_dev,
> > + ETHTOOL_A_CABLE_TEST_NTF_HEADER);
> > + if (err)
> > + goto out;
> > +
> > + err = nla_put_u8(phydev->skb, ETHTOOL_A_CABLE_TEST_NTF_STATUS,
> > + ETHTOOL_A_CABLE_TEST_NTF_STATUS_COMPLETED);
> > + if (err)
> > + goto out;
> > +
> > + phydev->nest = nla_nest_start(phydev->skb,
> > + ETHTOOL_A_CABLE_TEST_NTF_NEST);
> > + if (!phydev->nest)
> > + goto out;
>
> If nla_nest_start() fails, we still have 0 in err.
>
> > +
> > + return 0;
> > +
> > +out:
> > + nlmsg_free(phydev->skb);
> > + return err;
> > +}
> > +EXPORT_SYMBOL_GPL(ethnl_cable_test_alloc);
>
> Do you think it would make sense to set phydev->skb to NULL on failure
> and also in ethnl_cable_test_free() and ethnl_cable_test_finished() so
> that if driver messes up, it hits null pointer dereference which is much
> easier to debug than use after free?
Hi Michal
The use after free is not that hard to debug, i had to do it myself :-)
But yes, i can poison phydev->skb.
Andrew
