Skip to content

Commit

Permalink
genetlink: disallow subscribing to unknown mcast groups
Browse files Browse the repository at this point in the history
Jeff Layton reported that he could trigger the multicast unbind warning
in generic netlink using trinity. I originally thought it was a race
condition between unregistering the generic netlink family and closing
the socket, but there's a far simpler explanation: genetlink currently
allows subscribing to groups that don't (yet) exist, and the warning is
triggered when unsubscribing again while the group still doesn't exist.

Originally, I had a warning in the subscribe case and accepted it out of
userspace API concerns, but the warning was of course wrong and removed
later.

However, I now think that allowing userspace to subscribe to groups that
don't exist is wrong and could possibly become a security problem:
Consider a (new) genetlink family implementing a permission check in
the mcast_bind() function similar to the like the audit code does today;
it would be possible to bypass the permission check by guessing the ID
and subscribing to the group it exists. This is only possible in case a
family like that would be dynamically loaded, but it doesn't seem like a
huge stretch, for example wireless may be loaded when you plug in a USB
device.

To avoid this reject such subscription attempts.

If this ends up causing userspace issues we may need to add a workaround
in af_netlink to deny such requests but not return an error.

Reported-by: Jeff Layton <jeff.layton@primarydata.com>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
Johannes Berg authored and David S. Miller committed Jan 16, 2015
1 parent f555f3d commit 5ad6300
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion net/netlink/genetlink.c
Original file line number Diff line number Diff line change
Expand Up @@ -985,7 +985,7 @@ static struct genl_multicast_group genl_ctrl_groups[] = {

static int genl_bind(struct net *net, int group)
{
int i, err = 0;
int i, err = -ENOENT;

down_read(&cb_lock);
for (i = 0; i < GENL_FAM_TAB_SIZE; i++) {
Expand Down

0 comments on commit 5ad6300

Please sign in to comment.