|
1. As shown in the definition of nl_recv, its return value is of INT type. int nl_recv(struct nl_handle *handle, struct sockaddr_nl *nla, unsigned char **buf, struct ucred **creds) However, in function virNetlinkCommand, it uses an unsigned int param, respbuflen, to receive its return value. Thus, the branch "*respbuflen < 0" is unreachable, negative return values are handled incorrectly.
2. It's said in nl_recv's description that "The caller is responsible for freeing the buffer allocated * in \c *buf if a positive value is returned." which means, we needn't to free it in case it fails. Additionally, nl_recv has a BUG in handling buf: in the error branch, it frees the buf, but didn't set it to NULL. Freeing it outside in the caller function will cause double free. Thus, we set but(resp) to NULL if nl_recv fails. Signed-off-by: Zhang Bo <oscar.zhangbo@xxxxxxxxxx> --- src/util/virnetlink.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c index 0cf18f2..1a09567 100644 --- a/src/util/virnetlink.c +++ b/src/util/virnetlink.c @@ -192,6 +192,7 @@ int virNetlinkCommand(struct nl_msg *nl_msg, int n; struct nlmsghdr *nlmsg = nlmsg_hdr(nl_msg); virNetlinkHandle *nlhandle = NULL; + int length = 0; if (protocol >= MAX_LINKS) { virReportSystemError(EINVAL, @@ -257,12 +258,15 @@ int virNetlinkCommand(struct nl_msg *nl_msg, goto error; } - *respbuflen = nl_recv(nlhandle, &nladdr, - (unsigned char **)resp, NULL); + length = nl_recv(nlhandle, &nladdr, + (unsigned char **)resp, NULL); - if (*respbuflen <= 0) { + if (length <= 0) { virReportSystemError(errno, "%s", _("nl_recv failed")); + *resp = NULL; rc = -1; + } else { + *respbuflen = length; } error: if (rc == -1) { -- 1.7.12.4 |
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list