From patchwork Mon Jul 14 06:01:10 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Christoph Schulz X-Patchwork-Id: 369501 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 5CF1E1400A8 for ; Mon, 14 Jul 2014 16:01:21 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752881AbaGNGBR (ORCPT ); Mon, 14 Jul 2014 02:01:17 -0400 Received: from server721-han.de-nserver.de ([85.158.180.102]:38522 "EHLO server721-han.de-nserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752616AbaGNGBP (ORCPT ); Mon, 14 Jul 2014 02:01:15 -0400 Received: (qmail 15778 invoked from network); 14 Jul 2014 08:01:14 +0200 X-Fcrdns: Yes Received: from a89-182-68-151.net-htp.de (HELO onion.schulz.ip-v6.eu) (89.182.68.151) (smtp-auth username mail@kristov.de, mechanism plain) by server721-han.de-nserver.de (qpsmtpd/0.92) with (ECDHE-RSA-AES256-GCM-SHA384 encrypted) ESMTPSA; Mon, 14 Jul 2014 08:01:13 +0200 Received: from [2001:6f8:13da:1:c08f:e2c9:3ca0:8734] (helo=peacock.schulz.ip-v6.eu) by onion.schulz.ip-v6.eu with esmtps (TLSv1.2:DHE-RSA-AES256-GCM-SHA384:256) (Exim 4.80.1) (envelope-from ) id 1X6ZK3-00071R-Nu; Mon, 14 Jul 2014 08:01:12 +0200 Received: from peacock.schulz.ip-v6.eu ([2001:6f8:13da:1:225:22ff:fe6c:68f4]) by peacock.schulz.ip-v6.eu with esmtps (TLSv1:DHE-RSA-AES128-SHA:128) (Exim 4.80.1) (envelope-from ) id 1X6ZK3-0002em-A6; Mon, 14 Jul 2014 08:01:11 +0200 Message-ID: <53C37226.2020106@kristov.de> Date: Mon, 14 Jul 2014 08:01:10 +0200 From: Christoph Schulz User-Agent: Mozilla/5.0 (X11; Linux i686; rv:24.0) Gecko/20100101 Thunderbird/24.6.0 MIME-Version: 1.0 To: netdev@vger.kernel.org CC: linux-ppp@vger.kernel.org, paulus@samba.org, isdn@linux-pingi.de Subject: [PATCH net v2 1/1] net: ppp: don't call sk_chk_filter twice X-User-Auth: Auth by mail@kristov.de through 89.182.68.151 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org From: Christoph Schulz Commit 568f194e8bd16c353ad50f9ab95d98b20578a39d ("net: ppp: use sk_unattached_filter api") causes sk_chk_filter() to be called twice when setting a PPP pass or active filter. This applies to both the generic PPP subsystem implemented by drivers/net/ppp/ppp_generic.c and the ISDN PPP subsystem implemented by drivers/isdn/i4l/isdn_ppp.c. The first call is from within get_filter(). The second one is through the call chain ppp_ioctl() or isdn_ppp_ioctl() --> sk_unattached_filter_create() --> __sk_prepare_filter() --> sk_chk_filter() The first call from within get_filter() should be deleted as get_filter() is called just before calling sk_unattached_filter_create() later on, which eventually calls sk_chk_filter() anyway. For 3.15.x, this proposed change is a bugfix rather than a pure optimization as in that branch, sk_chk_filter() may replace filter codes by other codes which are not recognized when executing sk_chk_filter() a second time. So with 3.15.x, if sk_chk_filter() is called twice, the second invocation may yield EINVAL (this depends on the filter codes found in the filter to be set, but because the replacement is done for frequently used codes, this is almost always the case). The net effect is that setting pass and/or active PPP filters does not work anymore, since sk_unattached_filter_create() always returns EINVAL due to the second call to sk_chk_filter(), regardless whether the filter was originally sane or not. Signed-off-by: Christoph Schulz Acked-by: Daniel Borkmann --- Changes to first version: - corrupted patch corrected -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/isdn/i4l/isdn_ppp.c b/drivers/isdn/i4l/isdn_ppp.c index 61ac632..a333b7f 100644 --- a/drivers/isdn/i4l/isdn_ppp.c +++ b/drivers/isdn/i4l/isdn_ppp.c @@ -442,7 +442,7 @@ static int get_filter(void __user *arg, struct sock_filter **p) { struct sock_fprog uprog; struct sock_filter *code = NULL; - int len, err; + int len; if (copy_from_user(&uprog, arg, sizeof(uprog))) return -EFAULT; @@ -458,12 +458,6 @@ static int get_filter(void __user *arg, struct sock_filter **p) if (IS_ERR(code)) return PTR_ERR(code); - err = sk_chk_filter(code, uprog.len); - if (err) { - kfree(code); - return err; - } - *p = code; return uprog.len; } diff --git a/drivers/net/ppp/ppp_generic.c b/drivers/net/ppp/ppp_generic.c index 91d6c12..e2f20f8 100644 --- a/drivers/net/ppp/ppp_generic.c +++ b/drivers/net/ppp/ppp_generic.c @@ -539,7 +539,7 @@ static int get_filter(void __user *arg, struct sock_filter **p) { struct sock_fprog uprog; struct sock_filter *code = NULL; - int len, err; + int len; if (copy_from_user(&uprog, arg, sizeof(uprog))) return -EFAULT; @@ -554,12 +554,6 @@ static int get_filter(void __user *arg, struct sock_filter **p) if (IS_ERR(code)) return PTR_ERR(code); - err = sk_chk_filter(code, uprog.len); - if (err) { - kfree(code); - return err; - } - *p = code; return uprog.len; }