Commit
e408762f "netlink-socket: New function nl_lookup_genl_mcgroup()"
modified do_lookup_genl_family() to return the Netlink attributes to the
caller, but it still freed the Netlink message itself, which meant that
the attributes pointed into freed memory. This commit fixes the problem.
This commit is not a minimal fix. It refactors do_lookup_genl_family(),
changing the return value from "negative errno value or positive genl
family id" to the more common "zero or positive errno value".
Found by valgrind.
-do_lookup_genl_family(const char *name, struct nlattr **attrs)
+do_lookup_genl_family(const char *name, struct nlattr **attrs,
+ struct ofpbuf **replyp)
{
struct nl_sock *sock;
struct ofpbuf request, *reply;
{
struct nl_sock *sock;
struct ofpbuf request, *reply;
- retval = nl_sock_create(NETLINK_GENERIC, &sock);
- if (retval) {
- return -retval;
+ *replyp = NULL;
+ error = nl_sock_create(NETLINK_GENERIC, &sock);
+ if (error) {
+ return error;
}
ofpbuf_init(&request, 0);
nl_msg_put_genlmsghdr(&request, 0, GENL_ID_CTRL, NLM_F_REQUEST,
CTRL_CMD_GETFAMILY, 1);
nl_msg_put_string(&request, CTRL_ATTR_FAMILY_NAME, name);
}
ofpbuf_init(&request, 0);
nl_msg_put_genlmsghdr(&request, 0, GENL_ID_CTRL, NLM_F_REQUEST,
CTRL_CMD_GETFAMILY, 1);
nl_msg_put_string(&request, CTRL_ATTR_FAMILY_NAME, name);
- retval = nl_sock_transact(sock, &request, &reply);
+ error = nl_sock_transact(sock, &request, &reply);
}
if (!nl_policy_parse(reply, NLMSG_HDRLEN + GENL_HDRLEN,
}
if (!nl_policy_parse(reply, NLMSG_HDRLEN + GENL_HDRLEN,
- family_policy, attrs, ARRAY_SIZE(family_policy))) {
+ family_policy, attrs, ARRAY_SIZE(family_policy))
+ || nl_attr_get_u16(attrs[CTRL_ATTR_FAMILY_ID]) == 0) {
nl_sock_destroy(sock);
ofpbuf_delete(reply);
nl_sock_destroy(sock);
ofpbuf_delete(reply);
- retval = nl_attr_get_u16(attrs[CTRL_ATTR_FAMILY_ID]);
- if (retval == 0) {
- retval = -EPROTO;
- } else {
- define_genl_family(retval, name);
- }
- ofpbuf_delete(reply);
-
- return retval;
+ *replyp = reply;
+ return 0;
}
/* Finds the multicast group called 'group_name' in genl family 'family_name'.
}
/* Finds the multicast group called 'group_name' in genl family 'family_name'.
{
struct nlattr *family_attrs[ARRAY_SIZE(family_policy)];
struct ofpbuf all_mcs;
{
struct nlattr *family_attrs[ARRAY_SIZE(family_policy)];
struct ofpbuf all_mcs;
struct nlattr *mc;
unsigned int left;
struct nlattr *mc;
unsigned int left;
- retval = do_lookup_genl_family(family_name, family_attrs);
- if (retval <= 0) {
- assert(retval);
- return -retval;
+ error = do_lookup_genl_family(family_name, family_attrs, &reply);
+ if (error) {
+ return error;
}
nl_attr_get_nested(family_attrs[CTRL_ATTR_MCAST_GROUPS], &all_mcs);
}
nl_attr_get_nested(family_attrs[CTRL_ATTR_MCAST_GROUPS], &all_mcs);
const char *mc_name;
if (!nl_parse_nested(mc, mc_policy, mc_attrs, ARRAY_SIZE(mc_policy))) {
const char *mc_name;
if (!nl_parse_nested(mc, mc_policy, mc_attrs, ARRAY_SIZE(mc_policy))) {
+ error = EPROTO;
+ goto exit;
}
mc_name = nl_attr_get_string(mc_attrs[CTRL_ATTR_MCAST_GRP_NAME]);
if (!strcmp(group_name, mc_name)) {
*multicast_group =
nl_attr_get_u32(mc_attrs[CTRL_ATTR_MCAST_GRP_ID]);
}
mc_name = nl_attr_get_string(mc_attrs[CTRL_ATTR_MCAST_GRP_NAME]);
if (!strcmp(group_name, mc_name)) {
*multicast_group =
nl_attr_get_u32(mc_attrs[CTRL_ATTR_MCAST_GRP_ID]);
+ error = 0;
+ goto exit;
+exit:
+ ofpbuf_delete(reply);
+ return error;
}
/* If '*number' is 0, translates the given Generic Netlink family 'name' to a
}
/* If '*number' is 0, translates the given Generic Netlink family 'name' to a
int
nl_lookup_genl_family(const char *name, int *number)
{
int
nl_lookup_genl_family(const char *name, int *number)
{
- struct nlattr *attrs[ARRAY_SIZE(family_policy)];
-
- *number = do_lookup_genl_family(name, attrs);
+ struct nlattr *attrs[ARRAY_SIZE(family_policy)];
+ struct ofpbuf *reply;
+ int error;
+
+ error = do_lookup_genl_family(name, attrs, &reply);
+ if (!error) {
+ *number = nl_attr_get_u16(attrs[CTRL_ATTR_FAMILY_ID]);
+ define_genl_family(*number, name);
+ } else {
+ *number = -error;
+ }
+ ofpbuf_delete(reply);
+
assert(*number != 0);
}
return *number > 0 ? 0 : -*number;
assert(*number != 0);
}
return *number > 0 ? 0 : -*number;