From: Dan Carpenter <dan.carpenter@oracle.com>
To: Krzysztof Halasa <khc@pm.waw.pl>, nan chen <whutchennan@gmail.com>
Cc: Jakub Kicinski <kuba@kernel.org>,
security@kernel.org, Greg KH <greg@kroah.com>,
"David S. Miller" <davem@davemloft.net>,
netdev@vger.kernel.org
Subject: [PATCH] hdlc_ppp: add range checks in ppp_cp_parse_cr()
Date: Tue, 8 Sep 2020 20:53:59 +0300 [thread overview]
Message-ID: <20200908175359.GA356675@mwanda> (raw)
In-Reply-To: <20200908153200.GB4165114@kroah.com>
There were two bugs here:
1) If opt[1] is zero then this results in a forever loop. If the value
is less than 2 then it is invalid.
2) We assume that "len" is more than sizeof(valid_accm) or 6 which can
result in memory corruption.
Reported-by: ChenNan Of Chaitin Security Research Lab <whutchennan@gmail.com>
Fixes: e022c2f07ae5 ("WAN: new synchronous PPP implementation for generic HDLC.")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
This was sent to the security list, but we normally just handle
networking driver bugs through the regular netdev list.
drivers/net/wan/hdlc_ppp.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/net/wan/hdlc_ppp.c b/drivers/net/wan/hdlc_ppp.c
index 48ced3912576..4e906b79a85f 100644
--- a/drivers/net/wan/hdlc_ppp.c
+++ b/drivers/net/wan/hdlc_ppp.c
@@ -383,11 +383,8 @@ static void ppp_cp_parse_cr(struct net_device *dev, u16 pid, u8 id,
}
for (opt = data; len; len -= opt[1], opt += opt[1]) {
- if (len < 2 || len < opt[1]) {
- dev->stats.rx_errors++;
- kfree(out);
- return; /* bad packet, drop silently */
- }
+ if (len < 2 || opt[1] < 2 || len < opt[1])
+ goto err_out;
if (pid == PID_LCP)
switch (opt[0]) {
@@ -395,6 +392,8 @@ static void ppp_cp_parse_cr(struct net_device *dev, u16 pid, u8 id,
continue; /* MRU always OK and > 1500 bytes? */
case LCP_OPTION_ACCM: /* async control character map */
+ if (len < sizeof(valid_accm))
+ goto err_out;
if (!memcmp(opt, valid_accm,
sizeof(valid_accm)))
continue;
@@ -406,6 +405,8 @@ static void ppp_cp_parse_cr(struct net_device *dev, u16 pid, u8 id,
}
break;
case LCP_OPTION_MAGIC:
+ if (len < 6)
+ goto err_out;
if (opt[1] != 6 || (!opt[2] && !opt[3] &&
!opt[4] && !opt[5]))
break; /* reject invalid magic number */
@@ -424,6 +425,11 @@ static void ppp_cp_parse_cr(struct net_device *dev, u16 pid, u8 id,
ppp_cp_event(dev, pid, RCR_GOOD, CP_CONF_ACK, id, req_len, data);
kfree(out);
+ return;
+
+err_out:
+ dev->stats.rx_errors++;
+ kfree(out);
}
static int ppp_rx(struct sk_buff *skb)
--
2.28.0
next parent reply other threads:[~2020-09-08 17:54 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20200908153200.GB4165114@kroah.com>
2020-09-08 17:53 ` Dan Carpenter [this message]
[not found] ` <CAMnVd19nWToENW3X7v_PZN4snoXAoLgqKqn=dezXnd=z89zL7Q@mail.gmail.com>
2020-09-09 7:19 ` [PATCH] hdlc_ppp: add range checks in ppp_cp_parse_cr() Dan Carpenter
2020-09-09 9:46 ` [PATCH v2 net] " Dan Carpenter
2020-09-10 20:00 ` David Miller
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=20200908175359.GA356675@mwanda \
--to=dan.carpenter@oracle.com \
--cc=davem@davemloft.net \
--cc=greg@kroah.com \
--cc=khc@pm.waw.pl \
--cc=kuba@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=security@kernel.org \
--cc=whutchennan@gmail.com \
/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.