On 09/20/2017 07:41 AM, Erik Skultety wrote: > On Wed, Sep 20, 2017 at 12:48:01PM +0200, Jiri Denemark wrote: >> After commit 8708ca01c0d libvirtd consistently aborts with >> "stack smashing detected" when nodedev driver is initialized. >> >> Apparently this is caused by nlmsg_parse trying to write more than >> CTRL_ATTR_MAX bytes in tb because it is told tb can accommodate >> CTRL_CMD_MAX bytes. >> >> Signed-off-by: Jiri Denemark <[email protected]> >> --- >> src/util/virnetdev.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c >> index 41a659732b..be8f4b7938 100644 >> --- a/src/util/virnetdev.c >> +++ b/src/util/virnetdev.c >> @@ -3158,7 +3158,7 @@ virNetDevGetFamilyId(const char *family_name) >> struct nl_msg *nl_msg = NULL; >> struct nlmsghdr *resp = NULL; >> struct genlmsghdr* gmsgh = NULL; >> - struct nlattr *tb[CTRL_ATTR_MAX + 1] = {NULL, }; >> + struct nlattr *tb[CTRL_CMD_MAX + 1] = {NULL, }; > Shouldn't the correct fix be to instruct nlmsg_parse to accommodate up to > CTRL_ATTR_MAX bytes instead? Judging from the nature of the enum in getlink.h > it appears to me that that's the correct enum we want to use, instead of the > CTR_CMD_ enum ??
That is correct. tb is an array of *nlattr, with one pointer for each possible attribute that could be encountered. The netlink response that's being parsed here will contain CTRL_ATTR_* attributes, and nlmsg_parse will 1) put a NULL in all the entries of tb (that's what's causing the problem), then 2) fill in each tb[CTRL_ATTR_BLAH] with a pointer to that attribute in the netlink response. So Erik's suggested fix is the correct fix. > > I'm inclined to say ACK, but let me know what you think of the enum first. > > Erik > > -- > libvir-list mailing list > [email protected] > https://www.redhat.com/mailman/listinfo/libvir-list > -- libvir-list mailing list [email protected] https://www.redhat.com/mailman/listinfo/libvir-list
