* [PATCH 0/2] prevent potential access of uninitialized members in can_rcv() and canfd_rcv()
@ 2020-11-03 21:39 Anant Thazhemadam
2020-11-03 21:39 ` [PATCH 1/2] can: af_can: prevent potential access of uninitialized member in can_rcv() Anant Thazhemadam
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Anant Thazhemadam @ 2020-11-03 21:39 UTC (permalink / raw)
To: socketcan, mkl, davem, kuba
Cc: linux-can, netdev, linux-kernel, Anant Thazhemadam
In both can_rcv(), and canfd_rcv(), when skb->len = 0, cfd->len
(which is uninitialized) is accessed by pr_warn_once().
Performing the validation check for cfd->len separately, after the
validation check for skb->len is done, resolves this issue in both
instances, without compromising the degree of detail provided in the
log messages.
Anant Thazhemadam (2):
can: af_can: prevent potential access of uninitialized member in
can_rcv()
can: af_can: prevent potential access of uninitialized member in
canfd_rcv()
net/can/af_can.c | 38 ++++++++++++++++++++++++++++----------
1 file changed, 28 insertions(+), 10 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 1/2] can: af_can: prevent potential access of uninitialized member in can_rcv()
2020-11-03 21:39 [PATCH 0/2] prevent potential access of uninitialized members in can_rcv() and canfd_rcv() Anant Thazhemadam
@ 2020-11-03 21:39 ` Anant Thazhemadam
2020-11-03 21:39 ` [PATCH 2/2] can: af_can: prevent potential access of uninitialized member in canfd_rcv() Anant Thazhemadam
2020-11-03 22:17 ` [PATCH 0/2] prevent potential access of uninitialized members in can_rcv() and canfd_rcv() Marc Kleine-Budde
2 siblings, 0 replies; 4+ messages in thread
From: Anant Thazhemadam @ 2020-11-03 21:39 UTC (permalink / raw)
To: socketcan, mkl, davem, kuba
Cc: linux-can, netdev, linux-kernel, Anant Thazhemadam,
syzbot+9bcb0c9409066696d3aa
In can_rcv(), cfd->len is uninitialized when skb->len = 0, and this
uninitialized cfd->len is accessed nonetheless by pr_warn_once().
Fix this uninitialized variable access by checking cfd->len's validity
condition (cfd->len > CAN_MAX_DLEN) separately after the skb->len's
condition is checked, and appropriately modify the log messages that
are generated as well.
In case either of the required conditions fail, the skb is freed and
NET_RX_DROP is returned, same as before.
Fixes: 8cb68751c115 ("can: af_can: can_rcv(): replace WARN_ONCE by pr_warn_once")
Reported-by: syzbot+9bcb0c9409066696d3aa@syzkaller.appspotmail.com
Tested-by: Anant Thazhemadam <anant.thazhemadam@gmail.com>
Signed-off-by: Anant Thazhemadam <anant.thazhemadam@gmail.com>
---
net/can/af_can.c | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/net/can/af_can.c b/net/can/af_can.c
index ea29a6d97ef5..8ea01524f062 100644
--- a/net/can/af_can.c
+++ b/net/can/af_can.c
@@ -677,16 +677,25 @@ static int can_rcv(struct sk_buff *skb, struct net_device *dev,
{
struct canfd_frame *cfd = (struct canfd_frame *)skb->data;
- if (unlikely(dev->type != ARPHRD_CAN || skb->len != CAN_MTU ||
- cfd->len > CAN_MAX_DLEN)) {
- pr_warn_once("PF_CAN: dropped non conform CAN skbuf: dev type %d, len %d, datalen %d\n",
+ if (unlikely(dev->type != ARPHRD_CAN || skb->len != CAN_MTU)) {
+ pr_warn_once("PF_CAN: dropped non conform CAN skbuff: dev type %d, len %d\n",
+ dev->type, skb->len);
+ goto free_skb;
+ }
+
+ /* This check is made separately since cfd->len would be uninitialized if skb->len = 0. */
+ if (unlikely(cfd->len > CAN_MAX_DLEN)) {
+ pr_warn_once("PF_CAN: dropped non conform CAN skbuff: dev type %d, len %d, datalen %d\n",
dev->type, skb->len, cfd->len);
- kfree_skb(skb);
- return NET_RX_DROP;
+ goto free_skb;
}
can_receive(skb, dev);
return NET_RX_SUCCESS;
+
+free_skb:
+ kfree_skb(skb);
+ return NET_RX_DROP;
}
static int canfd_rcv(struct sk_buff *skb, struct net_device *dev,
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/2] can: af_can: prevent potential access of uninitialized member in canfd_rcv()
2020-11-03 21:39 [PATCH 0/2] prevent potential access of uninitialized members in can_rcv() and canfd_rcv() Anant Thazhemadam
2020-11-03 21:39 ` [PATCH 1/2] can: af_can: prevent potential access of uninitialized member in can_rcv() Anant Thazhemadam
@ 2020-11-03 21:39 ` Anant Thazhemadam
2020-11-03 22:17 ` [PATCH 0/2] prevent potential access of uninitialized members in can_rcv() and canfd_rcv() Marc Kleine-Budde
2 siblings, 0 replies; 4+ messages in thread
From: Anant Thazhemadam @ 2020-11-03 21:39 UTC (permalink / raw)
To: socketcan, mkl, davem, kuba
Cc: linux-can, netdev, linux-kernel, Anant Thazhemadam,
syzbot+9bcb0c9409066696d3aa
In canfd_rcv(), cfd->len is uninitialized when skb->len = 0, and this
uninitialized cfd->len is accessed nonetheless by pr_warn_once().
Fix this uninitialized variable access by checking cfd->len's validity
condition (cfd->len > CANFD_MAX_DLEN) separately after the skb->len's
condition is checked, and appropriately modify the log messages that
are generated as well.
In case either of the required conditions fail, the skb is freed and
NET_RX_DROP is returned, same as before.
Fixes: d4689846881d ("can: af_can: canfd_rcv(): replace WARN_ONCE by pr_warn_once")
Reported-by: syzbot+9bcb0c9409066696d3aa@syzkaller.appspotmail.com
Tested-by: Anant Thazhemadam <anant.thazhemadam@gmail.com>
Signed-off-by: Anant Thazhemadam <anant.thazhemadam@gmail.com>
---
This patch was locally tested using the reproducer and .config file
generated by syzbot.
net/can/af_can.c | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/net/can/af_can.c b/net/can/af_can.c
index 8ea01524f062..d759334f8843 100644
--- a/net/can/af_can.c
+++ b/net/can/af_can.c
@@ -703,16 +703,25 @@ static int canfd_rcv(struct sk_buff *skb, struct net_device *dev,
{
struct canfd_frame *cfd = (struct canfd_frame *)skb->data;
- if (unlikely(dev->type != ARPHRD_CAN || skb->len != CANFD_MTU ||
- cfd->len > CANFD_MAX_DLEN)) {
- pr_warn_once("PF_CAN: dropped non conform CAN FD skbuf: dev type %d, len %d, datalen %d\n",
+ if (unlikely(dev->type != ARPHRD_CAN || skb->len != CANFD_MTU)) {
+ pr_warn_once("PF_CAN: dropped non conform CAN FD skbuff: dev type %d, len %d\n",
+ dev->type, skb->len);
+ goto free_skb;
+ }
+
+ /* This check is made separately since cfd->len would be uninitialized if skb->len = 0. */
+ if (unlikely(cfd->len > CANFD_MAX_DLEN)) {
+ pr_warn_once("PF_CAN: dropped non conform CAN FD skbuff: dev type %d, len %d, datalen %d\n",
dev->type, skb->len, cfd->len);
- kfree_skb(skb);
- return NET_RX_DROP;
+ goto free_skb;
}
can_receive(skb, dev);
return NET_RX_SUCCESS;
+
+free_skb:
+ kfree_skb(skb);
+ return NET_RX_DROP;
}
/* af_can protocol functions */
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 0/2] prevent potential access of uninitialized members in can_rcv() and canfd_rcv()
2020-11-03 21:39 [PATCH 0/2] prevent potential access of uninitialized members in can_rcv() and canfd_rcv() Anant Thazhemadam
2020-11-03 21:39 ` [PATCH 1/2] can: af_can: prevent potential access of uninitialized member in can_rcv() Anant Thazhemadam
2020-11-03 21:39 ` [PATCH 2/2] can: af_can: prevent potential access of uninitialized member in canfd_rcv() Anant Thazhemadam
@ 2020-11-03 22:17 ` Marc Kleine-Budde
2 siblings, 0 replies; 4+ messages in thread
From: Marc Kleine-Budde @ 2020-11-03 22:17 UTC (permalink / raw)
To: Anant Thazhemadam, socketcan, davem, kuba; +Cc: linux-can, netdev, linux-kernel
[-- Attachment #1.1: Type: text/plain, Size: 1066 bytes --]
On 11/3/20 10:39 PM, Anant Thazhemadam wrote:
> In both can_rcv(), and canfd_rcv(), when skb->len = 0, cfd->len
> (which is uninitialized) is accessed by pr_warn_once().
>
> Performing the validation check for cfd->len separately, after the
> validation check for skb->len is done, resolves this issue in both
> instances, without compromising the degree of detail provided in the
> log messages.
>
> Anant Thazhemadam (2):
> can: af_can: prevent potential access of uninitialized member in
> can_rcv()
> can: af_can: prevent potential access of uninitialized member in
> canfd_rcv()
>
> net/can/af_can.c | 38 ++++++++++++++++++++++++++++----------
> 1 file changed, 28 insertions(+), 10 deletions(-)
>
Applied both to linux-can/testing
Tnx,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung West/Dortmund | Phone: +49-231-2826-924 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-11-03 22:17 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-03 21:39 [PATCH 0/2] prevent potential access of uninitialized members in can_rcv() and canfd_rcv() Anant Thazhemadam
2020-11-03 21:39 ` [PATCH 1/2] can: af_can: prevent potential access of uninitialized member in can_rcv() Anant Thazhemadam
2020-11-03 21:39 ` [PATCH 2/2] can: af_can: prevent potential access of uninitialized member in canfd_rcv() Anant Thazhemadam
2020-11-03 22:17 ` [PATCH 0/2] prevent potential access of uninitialized members in can_rcv() and canfd_rcv() Marc Kleine-Budde
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).