All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christoph Schulz <develop@kristov.de>
To: netdev@vger.kernel.org
Cc: linux-ppp@vger.kernel.org, paulus@samba.org, isdn@linux-pingi.de
Subject: [PATCH net-next] net: ppp: don't call sk_chk_filter twice
Date: Fri, 11 Jul 2014 07:10:36 +0000	[thread overview]
Message-ID: <53BF8DEC.4000307@kristov.de> (raw)

From: Christoph Schulz <develop@kristov.de>

Commit 568f194e8bd16c353ad50f9ab95d98b20578a39d ("net: ppp: use
sk_unattached_filter api") causes sk_chk_filter() to be called twice when
setting a pass or active filter. The first call is from within get_filter().
The second one is through the call chain

  ppp_ioctl() --> sk_unattached_filter_create()
              --> __sk_prepare_filter()
              --> sk_chk_filter()

However, sk_chk_filter() is not idempotent as it sometimes replaces filter
codes. So running it a second time over the same filter does not work and
yields EINVAL. 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 (and erroneous) call to sk_chk_filter(), regardless
whether the filter was sane or not. So this patch simply removes the call to
sk_chk_filter() from within get_filter(). This is safe as the filter will
be checked by sk_chk_filter() later anyway.

This error is found in exactly the same way in the isdn4linux PPP driver, so
it is fixed there the same way.

Signed-off-by: Christoph Schulz <develop@kristov.de>
---
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;
 }

WARNING: multiple messages have this Message-ID (diff)
From: Christoph Schulz <develop@kristov.de>
To: netdev@vger.kernel.org
Cc: linux-ppp@vger.kernel.org, paulus@samba.org, isdn@linux-pingi.de
Subject: [PATCH net-next] net: ppp: don't call sk_chk_filter twice
Date: Fri, 11 Jul 2014 09:10:36 +0200	[thread overview]
Message-ID: <53BF8DEC.4000307@kristov.de> (raw)

From: Christoph Schulz <develop@kristov.de>

Commit 568f194e8bd16c353ad50f9ab95d98b20578a39d ("net: ppp: use
sk_unattached_filter api") causes sk_chk_filter() to be called twice when
setting a pass or active filter. The first call is from within get_filter().
The second one is through the call chain

  ppp_ioctl() --> sk_unattached_filter_create()
              --> __sk_prepare_filter()
              --> sk_chk_filter()

However, sk_chk_filter() is not idempotent as it sometimes replaces filter
codes. So running it a second time over the same filter does not work and
yields EINVAL. 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 (and erroneous) call to sk_chk_filter(), regardless
whether the filter was sane or not. So this patch simply removes the call to
sk_chk_filter() from within get_filter(). This is safe as the filter will
be checked by sk_chk_filter() later anyway.

This error is found in exactly the same way in the isdn4linux PPP driver, so
it is fixed there the same way.

Signed-off-by: Christoph Schulz <develop@kristov.de>
---
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;
 }

             reply	other threads:[~2014-07-11  7:10 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-11  7:10 Christoph Schulz [this message]
2014-07-11  7:10 ` [PATCH net-next] net: ppp: don't call sk_chk_filter twice Christoph Schulz
2014-07-12  3:59 ` Alexei Starovoitov
2014-07-12  3:59   ` Alexei Starovoitov
2014-07-12 10:23   ` Daniel Borkmann
2014-07-12 10:23     ` Daniel Borkmann
2014-07-12 13:49     ` [PATCH net-next] net: filter: sk_chk_filter() no longer mangles filter Eric Dumazet
2014-07-12 13:49       ` Eric Dumazet
2014-07-12 15:23       ` Daniel Borkmann
2014-07-12 15:23         ` Daniel Borkmann
2014-07-13  1:29       ` Alexei Starovoitov
2014-07-13  1:29         ` Alexei Starovoitov
2014-07-14  6:28       ` David Miller
2014-07-14  6:28         ` David Miller
2014-07-12 21:11   ` [PATCH net-next] net: ppp: don't call sk_chk_filter twice Christoph Schulz
2014-07-12 21:11     ` Christoph Schulz
2014-07-13  1:44     ` Alexei Starovoitov
2014-07-13  1:44       ` Alexei Starovoitov
2014-07-13  7:03       ` Christoph Schulz
2014-07-13  7:03         ` Christoph Schulz
2014-07-13 10:27     ` Daniel Borkmann
2014-07-13 10:27       ` Daniel Borkmann
2014-07-13 13:06       ` Christoph Schulz
2014-07-13 13:06         ` Christoph Schulz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=53BF8DEC.4000307@kristov.de \
    --to=develop@kristov.de \
    --cc=isdn@linux-pingi.de \
    --cc=linux-ppp@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=paulus@samba.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.