From patchwork Thu Jan 15 11:04:44 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Johannes Berg X-Patchwork-Id: 429350 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 4193714007F for ; Thu, 15 Jan 2015 22:05:28 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752171AbbAOLFW (ORCPT ); Thu, 15 Jan 2015 06:05:22 -0500 Received: from s3.sipsolutions.net ([5.9.151.49]:35469 "EHLO sipsolutions.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753121AbbAOLEv (ORCPT ); Thu, 15 Jan 2015 06:04:51 -0500 Received: by sipsolutions.net with esmtpsa (TLS1.2:ECDHE_RSA_AES_128_CBC_SHA256:128) (Exim 4.84) (envelope-from ) id 1YBiEL-0007RB-Ny; Thu, 15 Jan 2015 12:04:49 +0100 From: Johannes Berg To: netdev@vger.kernel.org Cc: Jeff Layton , Sedat Dilek , Johannes Berg Subject: [PATCH 2/3] genetlink: disallow subscribing to unknown mcast groups Date: Thu, 15 Jan 2015 12:04:44 +0100 Message-Id: <1421319885-31779-2-git-send-email-johannes@sipsolutions.net> X-Mailer: git-send-email 2.1.4 In-Reply-To: <1421319885-31779-1-git-send-email-johannes@sipsolutions.net> References: <1421319885-31779-1-git-send-email-johannes@sipsolutions.net> Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org From: Johannes Berg 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 Signed-off-by: Johannes Berg --- net/netlink/genetlink.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c index 2e11061ef885..c18d3f5624b2 100644 --- a/net/netlink/genetlink.c +++ b/net/netlink/genetlink.c @@ -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++) {