[PATCH net 1/2] isdn_ppp: Add checks for allocation failure in isdn_ppp_open()

Subsystems: the rest

STALE3965d

6 messages, 3 authors, 2015-11-01 · open the first message on its own page

[PATCH net 1/2] isdn_ppp: Add checks for allocation failure in isdn_ppp_open()

From: Ben Hutchings <hidden>
Date: 2015-10-14 17:51:14

Compile-tested only.

Signed-off-by: Ben Hutchings <redacted>
---
 drivers/isdn/i4l/isdn_ppp.c | 6 ++++++
 1 file changed, 6 insertions(+)
diff --git a/drivers/isdn/i4l/isdn_ppp.c b/drivers/isdn/i4l/isdn_ppp.c
index c4198fa..86f9abe 100644
--- a/drivers/isdn/i4l/isdn_ppp.c
+++ b/drivers/isdn/i4l/isdn_ppp.c
@@ -301,6 +301,8 @@ isdn_ppp_open(int min, struct file *file)
 	is->compflags = 0;
 
 	is->reset = isdn_ppp_ccp_reset_alloc(is);
+	if (!is->reset)
+		return -ENOMEM;
 
 	is->lp = NULL;
 	is->mp_seqno = 0;       /* MP sequence number */
@@ -320,6 +322,10 @@ isdn_ppp_open(int min, struct file *file)
 	 * VJ header compression init
 	 */
 	is->slcomp = slhc_init(16, 16);	/* not necessary for 2. link in bundle */
+	if (!is->slcomp) {
+		isdn_ppp_ccp_reset_free(is);
+		return -ENOMEM;
+	}
 #endif
 #ifdef CONFIG_IPPP_FILTER
 	is->pass_filter = NULL;

-- 
Ben Hutchings
[W]e found...that it wasn't as easy to get programs right as we had thought.
... I realized that a large part of my life from then on was going to be spent
in finding mistakes in my own programs. - Maurice Wilkes, 1949

[PATCH 2/2] ppp, slip: Validate VJ compression slot parameters completely

From: Ben Hutchings <hidden>
Date: 2015-10-14 17:53:30

Currently slhc_init() treats out-of-range values of rslots and tslots
as equivalent to 0, except that if tslots is too large it will
dereference a null pointer (CVE-2015-7799).

Add a range-check at the top of the function and make it return an
ERR_PTR() on error instead of NULL.  Change the callers accordingly.

Compile-tested only.

Reported-by: 郭永刚 <redacted>
References: http://article.gmane.org/gmane.comp.security.oss.general/17908
Signed-off-by: Ben Hutchings <redacted>
---
 drivers/isdn/i4l/isdn_ppp.c   | 10 ++++------
 drivers/net/ppp/ppp_generic.c |  6 ++----
 drivers/net/slip/slhc.c       | 12 ++++++++----
 drivers/net/slip/slip.c       |  2 +-
 4 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/drivers/isdn/i4l/isdn_ppp.c b/drivers/isdn/i4l/isdn_ppp.c
index 86f9abe..9c1e8ad 100644
--- a/drivers/isdn/i4l/isdn_ppp.c
+++ b/drivers/isdn/i4l/isdn_ppp.c
@@ -322,9 +322,9 @@ isdn_ppp_open(int min, struct file *file)
 	 * VJ header compression init
 	 */
 	is->slcomp = slhc_init(16, 16);	/* not necessary for 2. link in bundle */
-	if (!is->slcomp) {
+	if (IS_ERR(is->slcomp)) {
 		isdn_ppp_ccp_reset_free(is);
-		return -ENOMEM;
+		return PTR_ERR(is->slcomp);
 	}
 #endif
 #ifdef CONFIG_IPPP_FILTER
@@ -573,10 +573,8 @@ isdn_ppp_ioctl(int min, struct file *file, unsigned int cmd, unsigned long arg)
 			is->maxcid = val;
 #ifdef CONFIG_ISDN_PPP_VJ
 			sltmp = slhc_init(16, val);
-			if (!sltmp) {
-				printk(KERN_ERR "ippp, can't realloc slhc struct\n");
-				return -ENOMEM;
-			}
+			if (IS_ERR(sltmp))
+				return PTR_ERR(sltmp);
 			if (is->slcomp)
 				slhc_free(is->slcomp);
 			is->slcomp = sltmp;
diff --git a/drivers/net/ppp/ppp_generic.c b/drivers/net/ppp/ppp_generic.c
index ed00446..9a863c6 100644
--- a/drivers/net/ppp/ppp_generic.c
+++ b/drivers/net/ppp/ppp_generic.c
@@ -721,10 +721,8 @@ static long ppp_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 			val &= 0xffff;
 		}
 		vj = slhc_init(val2+1, val+1);
-		if (!vj) {
-			netdev_err(ppp->dev,
-				   "PPP: no memory (VJ compressor)\n");
-			err = -ENOMEM;
+		if (IS_ERR(vj)) {
+			err = PTR_ERR(vj);
 			break;
 		}
 		ppp_lock(ppp);
diff --git a/drivers/net/slip/slhc.c b/drivers/net/slip/slhc.c
index 079f7ad..27ed252 100644
--- a/drivers/net/slip/slhc.c
+++ b/drivers/net/slip/slhc.c
@@ -84,8 +84,9 @@ static long decode(unsigned char **cpp);
 static unsigned char * put16(unsigned char *cp, unsigned short x);
 static unsigned short pull16(unsigned char **cpp);
 
-/* Initialize compression data structure
+/* Allocate compression data structure
  *	slots must be in range 0 to 255 (zero meaning no compression)
+ * Returns pointer to structure or ERR_PTR() on error.
  */
 struct slcompress *
 slhc_init(int rslots, int tslots)
@@ -94,11 +95,14 @@ slhc_init(int rslots, int tslots)
 	register struct cstate *ts;
 	struct slcompress *comp;
 
+	if (rslots < 0 || rslots > 255 || tslots < 0 || tslots > 255)
+		return ERR_PTR(-EINVAL);
+
 	comp = kzalloc(sizeof(struct slcompress), GFP_KERNEL);
 	if (! comp)
 		goto out_fail;
 
-	if ( rslots > 0  &&  rslots < 256 ) {
+	if (rslots > 0) {
 		size_t rsize = rslots * sizeof(struct cstate);
 		comp->rstate = kzalloc(rsize, GFP_KERNEL);
 		if (! comp->rstate)
@@ -106,7 +110,7 @@ slhc_init(int rslots, int tslots)
 		comp->rslot_limit = rslots - 1;
 	}
 
-	if ( tslots > 0  &&  tslots < 256 ) {
+	if (tslots > 0) {
 		size_t tsize = tslots * sizeof(struct cstate);
 		comp->tstate = kzalloc(tsize, GFP_KERNEL);
 		if (! comp->tstate)
@@ -141,7 +145,7 @@ out_free2:
 out_free:
 	kfree(comp);
 out_fail:
-	return NULL;
+	return ERR_PTR(-ENOMEM);
 }
 
 
diff --git a/drivers/net/slip/slip.c b/drivers/net/slip/slip.c
index 05387b1..a17d86a 100644
--- a/drivers/net/slip/slip.c
+++ b/drivers/net/slip/slip.c
@@ -164,7 +164,7 @@ static int sl_alloc_bufs(struct slip *sl, int mtu)
 	if (cbuff == NULL)
 		goto err_exit;
 	slcomp = slhc_init(16, 16);
-	if (slcomp == NULL)
+	if (IS_ERR(slcomp))
 		goto err_exit;
 #endif
 	spin_lock_bh(&sl->lock);
-- 
Ben Hutchings
[W]e found...that it wasn't as easy to get programs right as we had thought.
... I realized that a large part of my life from then on was going to be spent
in finding mistakes in my own programs. - Maurice Wilkes, 1949

Re: [PATCH net 1/2] isdn_ppp: Add checks for allocation failure in isdn_ppp_open()

From: David Miller <davem@davemloft.net>
Date: 2015-10-16 07:46:06

From: Ben Hutchings <redacted>
Date: Wed, 14 Oct 2015 18:51:14 +0100
quoted hunk
Compile-tested only.

Signed-off-by: Ben Hutchings <redacted>
---
 drivers/isdn/i4l/isdn_ppp.c | 6 ++++++
 1 file changed, 6 insertions(+)
diff --git a/drivers/isdn/i4l/isdn_ppp.c b/drivers/isdn/i4l/isdn_ppp.c
index c4198fa..86f9abe 100644
--- a/drivers/isdn/i4l/isdn_ppp.c
+++ b/drivers/isdn/i4l/isdn_ppp.c
@@ -301,6 +301,8 @@ isdn_ppp_open(int min, struct file *file)
 	is->compflags = 0;
 
 	is->reset = isdn_ppp_ccp_reset_alloc(is);
+	if (!is->reset)
+		return -ENOMEM;
Ben, your email client has corrupted both of these patches.

Please fix this up and resubmit, thanks.

Re: [PATCH net 1/2] isdn_ppp: Add checks for allocation failure in isdn_ppp_open()

From: Josh Boyer <hidden>
Date: 2015-10-30 13:03:45

On Fri, Oct 16, 2015 at 3:46 AM, David Miller [off-list ref] wrote:
From: Ben Hutchings <redacted>
Date: Wed, 14 Oct 2015 18:51:14 +0100
quoted
Compile-tested only.

Signed-off-by: Ben Hutchings <redacted>
---
 drivers/isdn/i4l/isdn_ppp.c | 6 ++++++
 1 file changed, 6 insertions(+)
diff --git a/drivers/isdn/i4l/isdn_ppp.c b/drivers/isdn/i4l/isdn_ppp.c
index c4198fa..86f9abe 100644
--- a/drivers/isdn/i4l/isdn_ppp.c
+++ b/drivers/isdn/i4l/isdn_ppp.c
@@ -301,6 +301,8 @@ isdn_ppp_open(int min, struct file *file)
      is->compflags = 0;

      is->reset = isdn_ppp_ccp_reset_alloc(is);
+     if (!is->reset)
+             return -ENOMEM;
Ben, your email client has corrupted both of these patches.

Please fix this up and resubmit, thanks.
Ben, did you resubmit these as David suggested?  I haven't found a v2 anywhere.

josh

Re: [PATCH net 1/2] isdn_ppp: Add checks for allocation failure in isdn_ppp_open()

From: Ben Hutchings <hidden>
Date: 2015-11-01 16:23:40

On Fri, 2015-10-16 at 00:46 -0700, David Miller wrote:
From: Ben Hutchings <redacted>
Date: Wed, 14 Oct 2015 18:51:14 +0100
quoted
Compile-tested only.

Signed-off-by: Ben Hutchings <redacted>
---
 drivers/isdn/i4l/isdn_ppp.c | 6 ++++++
 1 file changed, 6 insertions(+)
diff --git a/drivers/isdn/i4l/isdn_ppp.c
b/drivers/isdn/i4l/isdn_ppp.c
quoted
index c4198fa..86f9abe 100644
--- a/drivers/isdn/i4l/isdn_ppp.c
+++ b/drivers/isdn/i4l/isdn_ppp.c
@@ -301,6 +301,8 @@ isdn_ppp_open(int min, struct file *file)
      is->compflags = 0;
 
      is->reset = isdn_ppp_ccp_reset_alloc(is);
+     if (!is->reset)
+             return -ENOMEM;
Ben, your email client has corrupted both of these patches.

Please fix this up and resubmit, thanks.
Sorry about that; it is a regression in Evolution 3.18.

Ben.

-- 
Ben Hutchings
Nothing is ever a complete failure; it can always serve as a bad example.

Re: [PATCH net 1/2] isdn_ppp: Add checks for allocation failure in isdn_ppp_open()

From: Ben Hutchings <hidden>
Date: 2015-11-01 16:23:51

On Fri, 2015-10-30 at 09:03 -0400, Josh Boyer wrote:
On Fri, Oct 16, 2015 at 3:46 AM, David Miller [off-list ref]
wrote:
quoted
From: Ben Hutchings <redacted>
Date: Wed, 14 Oct 2015 18:51:14 +0100
quoted
Compile-tested only.

Signed-off-by: Ben Hutchings <redacted>
---
 drivers/isdn/i4l/isdn_ppp.c | 6 ++++++
 1 file changed, 6 insertions(+)
diff --git a/drivers/isdn/i4l/isdn_ppp.c
b/drivers/isdn/i4l/isdn_ppp.c
index c4198fa..86f9abe 100644
--- a/drivers/isdn/i4l/isdn_ppp.c
+++ b/drivers/isdn/i4l/isdn_ppp.c
@@ -301,6 +301,8 @@ isdn_ppp_open(int min, struct file *file)
      is->compflags = 0;

      is->reset = isdn_ppp_ccp_reset_alloc(is);
+     if (!is->reset)
+             return -ENOMEM;
Ben, your email client has corrupted both of these patches.

Please fix this up and resubmit, thanks.
Ben, did you resubmit these as David suggested?  I haven't found a v2
anywhere.
I've just done so.

Ben.

-- 
Ben Hutchings
Nothing is ever a complete failure; it can always serve as a bad example.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help