* [Linux-kernel-mentees] [PATCH v2 net] rose: Fix Null pointer dereference in rose_send_frame()
@ 2020-11-05 15:56 Anmol Karn
2020-11-06 21:04 ` Saeed Mahameed
2020-11-07 18:56 ` [Linux-kernel-mentees] [PATCH v2] net: " Anmol Karn
0 siblings, 2 replies; 13+ messages in thread
From: Anmol Karn @ 2020-11-05 15:56 UTC (permalink / raw)
To: ralf, davem, kuba
Cc: gregkh, netdev, linux-kernel, linux-hams, linux-kernel-mentees,
syzkaller-bugs, anmol.karan123, syzbot+a1c743815982d9496393
rose_send_frame() dereferences `neigh->dev` when called from
rose_transmit_clear_request(), and the first occurance of the `neigh`
is in rose_loopback_timer() as `rose_loopback_neigh`, and it is initialized
in rose_add_loopback_neigh() as NULL. i.e when `rose_loopback_neigh` used in
rose_loopback_timer() its `->dev` was still NULL and rose_loopback_timer()
was calling rose_rx_call_request() without checking for NULL.
- net/rose/rose_link.c
This bug seems to get triggered in this line:
rose_call = (ax25_address *)neigh->dev->dev_addr;
Fix it by adding NULL checking for `rose_loopback_neigh->dev` in rose_loopback_timer().
Reported-and-tested-by: syzbot+a1c743815982d9496393@syzkaller.appspotmail.com
Link: https://syzkaller.appspot.com/bug?id=9d2a7ca8c7f2e4b682c97578dfa3f236258300b3
Signed-off-by: Anmol Karn <anmol.karan123@gmail.com>
---
net/rose/rose_loopback.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/rose/rose_loopback.c b/net/rose/rose_loopback.c
index 7b094275ea8b..cd7774cb1d07 100644
--- a/net/rose/rose_loopback.c
+++ b/net/rose/rose_loopback.c
@@ -96,7 +96,7 @@ static void rose_loopback_timer(struct timer_list *unused)
}
if (frametype == ROSE_CALL_REQUEST) {
- if ((dev = rose_dev_get(dest)) != NULL) {
+ if (rose_loopback_neigh->dev && (dev = rose_dev_get(dest)) != NULL) {
if (rose_rx_call_request(skb, dev, rose_loopback_neigh, lci_o) == 0)
kfree_skb(skb);
} else {
--
2.29.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH v2 net] rose: Fix Null pointer dereference in rose_send_frame()
2020-11-05 15:56 [Linux-kernel-mentees] [PATCH v2 net] rose: Fix Null pointer dereference in rose_send_frame() Anmol Karn
@ 2020-11-06 21:04 ` Saeed Mahameed
2020-11-07 8:20 ` Anmol Karn
2020-11-07 18:56 ` [Linux-kernel-mentees] [PATCH v2] net: " Anmol Karn
1 sibling, 1 reply; 13+ messages in thread
From: Saeed Mahameed @ 2020-11-06 21:04 UTC (permalink / raw)
To: Anmol Karn, ralf, davem, kuba
Cc: gregkh, netdev, linux-kernel, linux-hams, linux-kernel-mentees,
syzkaller-bugs, syzbot+a1c743815982d9496393
On Thu, 2020-11-05 at 21:26 +0530, Anmol Karn wrote:
> rose_send_frame() dereferences `neigh->dev` when called from
> rose_transmit_clear_request(), and the first occurance of the `neigh`
> is in rose_loopback_timer() as `rose_loopback_neigh`, and it is
> initialized
> in rose_add_loopback_neigh() as NULL. i.e when `rose_loopback_neigh`
> used in
> rose_loopback_timer() its `->dev` was still NULL and
> rose_loopback_timer()
> was calling rose_rx_call_request() without checking for NULL.
>
> - net/rose/rose_link.c
> This bug seems to get triggered in this line:
>
> rose_call = (ax25_address *)neigh->dev->dev_addr;
>
> Fix it by adding NULL checking for `rose_loopback_neigh->dev` in
> rose_loopback_timer().
>
> Reported-and-tested-by:
> syzbot+a1c743815982d9496393@syzkaller.appspotmail.com
> Link:
> https://syzkaller.appspot.com/bug?id=9d2a7ca8c7f2e4b682c97578dfa3f236258300b3
>
> Signed-off-by: Anmol Karn <anmol.karan123@gmail.com>
missing proper fixes tag.
> ---
> net/rose/rose_loopback.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/rose/rose_loopback.c b/net/rose/rose_loopback.c
> index 7b094275ea8b..cd7774cb1d07 100644
> --- a/net/rose/rose_loopback.c
> +++ b/net/rose/rose_loopback.c
> @@ -96,7 +96,7 @@ static void rose_loopback_timer(struct timer_list
> *unused)
> }
>
> if (frametype == ROSE_CALL_REQUEST) {
> - if ((dev = rose_dev_get(dest)) != NULL) {
> + if (rose_loopback_neigh->dev && (dev =
> rose_dev_get(dest)) != NULL) {
> if (rose_rx_call_request(skb, dev,
> rose_loopback_neigh, lci_o) == 0)
> kfree_skb(skb);
> } else {
check patch is not happy:
WARNING:TYPO_SPELLING: 'occurance' may be misspelled - perhaps
'occurrence'?
#7:
rose_transmit_clear_request(), and the first occurance of the `neigh`
ERROR:ASSIGN_IN_IF: do not use assignment in if condition
#36: FILE: net/rose/rose_loopback.c:99:
+ if (rose_loopback_neigh->dev && (dev =
rose_dev_get(dest)) != NULL) {
total: 1 errors, 1 warnings, 0 checks, 8 lines checked
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH v2 net] rose: Fix Null pointer dereference in rose_send_frame()
2020-11-06 21:04 ` Saeed Mahameed
@ 2020-11-07 8:20 ` Anmol Karn
2020-11-07 19:18 ` [Linux-kernel-mentees] [PATCH v3 " Anmol Karn
0 siblings, 1 reply; 13+ messages in thread
From: Anmol Karn @ 2020-11-07 8:20 UTC (permalink / raw)
To: Saeed Mahameed
Cc: ralf, davem, kuba, gregkh, netdev, linux-kernel, linux-hams,
linux-kernel-mentees, syzkaller-bugs,
syzbot+a1c743815982d9496393
Hello Sir,
On Fri, Nov 06, 2020 at 01:04:27PM -0800, Saeed Mahameed wrote:
> On Thu, 2020-11-05 at 21:26 +0530, Anmol Karn wrote:
> > rose_send_frame() dereferences `neigh->dev` when called from
> > rose_transmit_clear_request(), and the first occurance of the `neigh`
> > is in rose_loopback_timer() as `rose_loopback_neigh`, and it is
> > initialized
> > in rose_add_loopback_neigh() as NULL. i.e when `rose_loopback_neigh`
> > used in
> > rose_loopback_timer() its `->dev` was still NULL and
> > rose_loopback_timer()
> > was calling rose_rx_call_request() without checking for NULL.
> >
> > - net/rose/rose_link.c
> > This bug seems to get triggered in this line:
> >
> > rose_call = (ax25_address *)neigh->dev->dev_addr;
> >
> > Fix it by adding NULL checking for `rose_loopback_neigh->dev` in
> > rose_loopback_timer().
> >
> > Reported-and-tested-by:
> > syzbot+a1c743815982d9496393@syzkaller.appspotmail.com
> > Link:
> > https://syzkaller.appspot.com/bug?id=9d2a7ca8c7f2e4b682c97578dfa3f236258300b3
> >
> > Signed-off-by: Anmol Karn <anmol.karan123@gmail.com>
>
> missing proper fixes tag.
>
> > ---
> > net/rose/rose_loopback.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/net/rose/rose_loopback.c b/net/rose/rose_loopback.c
> > index 7b094275ea8b..cd7774cb1d07 100644
> > --- a/net/rose/rose_loopback.c
> > +++ b/net/rose/rose_loopback.c
> > @@ -96,7 +96,7 @@ static void rose_loopback_timer(struct timer_list
> > *unused)
> > }
> >
> > if (frametype == ROSE_CALL_REQUEST) {
> > - if ((dev = rose_dev_get(dest)) != NULL) {
> > + if (rose_loopback_neigh->dev && (dev =
> > rose_dev_get(dest)) != NULL) {
> > if (rose_rx_call_request(skb, dev,
> > rose_loopback_neigh, lci_o) == 0)
> > kfree_skb(skb);
> > } else {
>
> check patch is not happy:
>
> WARNING:TYPO_SPELLING: 'occurance' may be misspelled - perhaps
> 'occurrence'?
> #7:
> rose_transmit_clear_request(), and the first occurance of the `neigh`
>
> ERROR:ASSIGN_IN_IF: do not use assignment in if condition
> #36: FILE: net/rose/rose_loopback.c:99:
> + if (rose_loopback_neigh->dev && (dev =
> rose_dev_get(dest)) != NULL) {
>
> total: 1 errors, 1 warnings, 0 checks, 8 lines checked
>
>
Thank you for your review will rectify these and send another version.
Thanks,
Anmol
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Linux-kernel-mentees] [PATCH v2] net: rose: Fix Null pointer dereference in rose_send_frame()
2020-11-05 15:56 [Linux-kernel-mentees] [PATCH v2 net] rose: Fix Null pointer dereference in rose_send_frame() Anmol Karn
2020-11-06 21:04 ` Saeed Mahameed
@ 2020-11-07 18:56 ` Anmol Karn
2020-11-07 19:02 ` Anmol karn
1 sibling, 1 reply; 13+ messages in thread
From: Anmol Karn @ 2020-11-07 18:56 UTC (permalink / raw)
To: ralf, davem, kuba
Cc: saeed, gregkh, netdev, linux-kernel, linux-hams,
linux-kernel-mentees, syzkaller-bugs, anmol.karan123,
syzbot+a1c743815982d9496393
rose_send_frame() dereferences `neigh->dev` when called from
rose_transmit_clear_request(), and the first occurrence of the
`neigh` is in rose_loopback_timer() as `rose_loopback_neigh`,
and it is initialized in rose_add_loopback_neigh() as NULL.
i.e when `rose_loopback_neigh` used in rose_loopback_timer()
its `->dev` was still NULL and rose_loopback_timer() was calling
rose_rx_call_request() without checking for NULL.
- net/rose/rose_link.c
This bug seems to get triggered in this line:
rose_call = (ax25_address *)neigh->dev->dev_addr;
Fix it by adding NULL checking for `rose_loopback_neigh->dev`
in rose_loopback_timer().
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: syzbot+a1c743815982d9496393@syzkaller.appspotmail.com
Tested-by: syzbot+a1c743815982d9496393@syzkaller.appspotmail.com
Link: https://syzkaller.appspot.com/bug?id=9d2a7ca8c7f2e4b682c97578dfa3f236258300b3
Signed-off-by: Anmol Karn <anmol.karan123@gmail.com>
---
Changes in v3:
- Corrected checkpatch warnings and errors (Suggested-by: Saeed Mahameed <saeed@kernel.org>)
- Added "Fixes:" tag (Suggested-by: Saeed Mahameed <saeed@kernel.org>)
Changes in v2:
- Added NULL check in rose_loopback_timer() (Suggested-by: Greg KH <gregkh@linuxfoundation.org>)
net/rose/rose_loopback.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/rose/rose_loopback.c b/net/rose/rose_loopback.c
index 7b094275ea8b..2c51756ed7bf 100644
--- a/net/rose/rose_loopback.c
+++ b/net/rose/rose_loopback.c
@@ -96,7 +96,8 @@ static void rose_loopback_timer(struct timer_list *unused)
}
if (frametype == ROSE_CALL_REQUEST) {
- if ((dev = rose_dev_get(dest)) != NULL) {
+ dev = rose_dev_get(dest);
+ if (rose_loopback_neigh->dev && dev) {
if (rose_rx_call_request(skb, dev, rose_loopback_neigh, lci_o) == 0)
kfree_skb(skb);
} else {
-
2.29.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH v2] net: rose: Fix Null pointer dereference in rose_send_frame()
2020-11-07 18:56 ` [Linux-kernel-mentees] [PATCH v2] net: " Anmol Karn
@ 2020-11-07 19:02 ` Anmol karn
0 siblings, 0 replies; 13+ messages in thread
From: Anmol karn @ 2020-11-07 19:02 UTC (permalink / raw)
To: ralf, David S. Miller, Jakub Kicinski
Cc: Saeed Mahameed, Greg KH, netdev, open list, linux-hams,
linux-kernel-mentees, syzkaller-bugs, syzbot
Hello,
Sorry for this wrong subject(It should be v3 instead of v2),
please ignore this patch.
I will send a new one with the corrected subject.
Thanks,
Anmol
On Sun, Nov 8, 2020 at 12:27 AM Anmol Karn <anmol.karan123@gmail.com> wrote:
>
> rose_send_frame() dereferences `neigh->dev` when called from
> rose_transmit_clear_request(), and the first occurrence of the
> `neigh` is in rose_loopback_timer() as `rose_loopback_neigh`,
> and it is initialized in rose_add_loopback_neigh() as NULL.
> i.e when `rose_loopback_neigh` used in rose_loopback_timer()
> its `->dev` was still NULL and rose_loopback_timer() was calling
> rose_rx_call_request() without checking for NULL.
>
> - net/rose/rose_link.c
> This bug seems to get triggered in this line:
>
> rose_call = (ax25_address *)neigh->dev->dev_addr;
>
> Fix it by adding NULL checking for `rose_loopback_neigh->dev`
> in rose_loopback_timer().
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Reported-by: syzbot+a1c743815982d9496393@syzkaller.appspotmail.com
> Tested-by: syzbot+a1c743815982d9496393@syzkaller.appspotmail.com
> Link: https://syzkaller.appspot.com/bug?id=9d2a7ca8c7f2e4b682c97578dfa3f236258300b3
> Signed-off-by: Anmol Karn <anmol.karan123@gmail.com>
> ---
> Changes in v3:
> - Corrected checkpatch warnings and errors (Suggested-by: Saeed Mahameed <saeed@kernel.org>)
> - Added "Fixes:" tag (Suggested-by: Saeed Mahameed <saeed@kernel.org>)
> Changes in v2:
> - Added NULL check in rose_loopback_timer() (Suggested-by: Greg KH <gregkh@linuxfoundation.org>)
>
> net/rose/rose_loopback.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/rose/rose_loopback.c b/net/rose/rose_loopback.c
> index 7b094275ea8b..2c51756ed7bf 100644
> --- a/net/rose/rose_loopback.c
> +++ b/net/rose/rose_loopback.c
> @@ -96,7 +96,8 @@ static void rose_loopback_timer(struct timer_list *unused)
> }
>
> if (frametype == ROSE_CALL_REQUEST) {
> - if ((dev = rose_dev_get(dest)) != NULL) {
> + dev = rose_dev_get(dest);
> + if (rose_loopback_neigh->dev && dev) {
> if (rose_rx_call_request(skb, dev, rose_loopback_neigh, lci_o) == 0)
> kfree_skb(skb);
> } else {
> -
> 2.29.2
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Linux-kernel-mentees] [PATCH v3 net] rose: Fix Null pointer dereference in rose_send_frame()
2020-11-07 8:20 ` Anmol Karn
@ 2020-11-07 19:18 ` Anmol Karn
2020-11-10 17:58 ` Jakub Kicinski
0 siblings, 1 reply; 13+ messages in thread
From: Anmol Karn @ 2020-11-07 19:18 UTC (permalink / raw)
To: ralf, davem, kuba
Cc: saeed, gregkh, netdev, linux-kernel, linux-hams,
linux-kernel-mentees, syzkaller-bugs, anmol.karan123,
syzbot+a1c743815982d9496393
rose_send_frame() dereferences `neigh->dev` when called from
rose_transmit_clear_request(), and the first occurrence of the
`neigh` is in rose_loopback_timer() as `rose_loopback_neigh`,
and it is initialized in rose_add_loopback_neigh() as NULL.
i.e when `rose_loopback_neigh` used in rose_loopback_timer()
its `->dev` was still NULL and rose_loopback_timer() was calling
rose_rx_call_request() without checking for NULL.
- net/rose/rose_link.c
This bug seems to get triggered in this line:
rose_call = (ax25_address *)neigh->dev->dev_addr;
Fix it by adding NULL checking for `rose_loopback_neigh->dev`
in rose_loopback_timer().
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: syzbot+a1c743815982d9496393@syzkaller.appspotmail.com
Tested-by: syzbot+a1c743815982d9496393@syzkaller.appspotmail.com
Link: https://syzkaller.appspot.com/bug?id=9d2a7ca8c7f2e4b682c97578dfa3f236258300b3
Signed-off-by: Anmol Karn <anmol.karan123@gmail.com>
---
Changes in v3:
- Corrected checkpatch warnings and errors (Suggested-by: Saeed Mahameed <saeed@kernel.org>)
- Added "Fixes:" tag (Suggested-by: Saeed Mahameed <saeed@kernel.org>)
Changes in v2:
- Added NULL check in rose_loopback_timer() (Suggested-by: Greg KH <gregkh@linuxfoundation.org>)
net/rose/rose_loopback.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/rose/rose_loopback.c b/net/rose/rose_loopback.c
index 7b094275ea8b..2c51756ed7bf 100644
--- a/net/rose/rose_loopback.c
+++ b/net/rose/rose_loopback.c
@@ -96,7 +96,8 @@ static void rose_loopback_timer(struct timer_list *unused)
}
if (frametype == ROSE_CALL_REQUEST) {
- if ((dev = rose_dev_get(dest)) != NULL) {
+ dev = rose_dev_get(dest);
+ if (rose_loopback_neigh->dev && dev) {
if (rose_rx_call_request(skb, dev, rose_loopback_neigh, lci_o) == 0)
kfree_skb(skb);
} else {
-
2.29.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH v3 net] rose: Fix Null pointer dereference in rose_send_frame()
2020-11-07 19:18 ` [Linux-kernel-mentees] [PATCH v3 " Anmol Karn
@ 2020-11-10 17:58 ` Jakub Kicinski
2020-11-10 19:45 ` Anmol Karn
0 siblings, 1 reply; 13+ messages in thread
From: Jakub Kicinski @ 2020-11-10 17:58 UTC (permalink / raw)
To: Anmol Karn
Cc: ralf, davem, saeed, gregkh, netdev, linux-kernel, linux-hams,
linux-kernel-mentees, syzkaller-bugs,
syzbot+a1c743815982d9496393
On Sun, 8 Nov 2020 00:48:35 +0530 Anmol Karn wrote:
> + dev = rose_dev_get(dest);
this calls dev_hold internally, you never release that reference in
case ..neigh->dev is NULL
> + if (rose_loopback_neigh->dev && dev) {
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH v3 net] rose: Fix Null pointer dereference in rose_send_frame()
2020-11-10 17:58 ` Jakub Kicinski
@ 2020-11-10 19:45 ` Anmol Karn
2020-11-11 16:59 ` [Linux-kernel-mentees] [PATCH v4 " Anmol Karn
0 siblings, 1 reply; 13+ messages in thread
From: Anmol Karn @ 2020-11-10 19:45 UTC (permalink / raw)
To: Jakub Kicinski
Cc: ralf, davem, saeed, gregkh, netdev, linux-kernel, linux-hams,
linux-kernel-mentees, syzkaller-bugs,
syzbot+a1c743815982d9496393
Hello Sir,
On Tue, Nov 10, 2020 at 09:58:15AM -0800, Jakub Kicinski wrote:
> On Sun, 8 Nov 2020 00:48:35 +0530 Anmol Karn wrote:
> > + dev = rose_dev_get(dest);
>
> this calls dev_hold internally, you never release that reference in
> case ..neigh->dev is NULL
>
> > + if (rose_loopback_neigh->dev && dev) {
Ah, I missed to `dev_put()` the `dev` after checking for, if neigh->dev is NULL,
I will fix it soon and send another version.
Thank you for review.
Anmol
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Linux-kernel-mentees] [PATCH v4 net] rose: Fix Null pointer dereference in rose_send_frame()
2020-11-10 19:45 ` Anmol Karn
@ 2020-11-11 16:59 ` Anmol Karn
2020-11-14 19:18 ` Jakub Kicinski
0 siblings, 1 reply; 13+ messages in thread
From: Anmol Karn @ 2020-11-11 16:59 UTC (permalink / raw)
To: ralf, davem, kuba
Cc: saeed, gregkh, netdev, linux-kernel, linux-hams,
linux-kernel-mentees, syzkaller-bugs, anmol.karan123,
syzbot+a1c743815982d9496393
rose_send_frame() dereferences `neigh->dev` when called from
rose_transmit_clear_request(), and the first occurrence of the
`neigh` is in rose_loopback_timer() as `rose_loopback_neigh`,
and it is initialized in rose_add_loopback_neigh() as NULL.
i.e when `rose_loopback_neigh` used in rose_loopback_timer()
its `->dev` was still NULL and rose_loopback_timer() was calling
rose_rx_call_request() without checking for NULL.
- net/rose/rose_link.c
This bug seems to get triggered in this line:
rose_call = (ax25_address *)neigh->dev->dev_addr;
Fix it by adding NULL checking for `rose_loopback_neigh->dev`
in rose_loopback_timer().
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: syzbot+a1c743815982d9496393@syzkaller.appspotmail.com
Tested-by: syzbot+a1c743815982d9496393@syzkaller.appspotmail.com
Link: https://syzkaller.appspot.com/bug?id=9d2a7ca8c7f2e4b682c97578dfa3f236258300b3
Signed-off-by: Anmol Karn <anmol.karan123@gmail.com>
---
Changes in v4:
- Free `dev`(on dev_hold()), when neigh->dev is NULL. (Suggested-by: Jakub Kicinski <kuba@kernel.org>)
Changes in v3:
- Corrected checkpatch warnings and errors (Suggested-by: Saeed Mahameed <saeed@kernel.org>)
- Added "Fixes:" tag (Suggested-by: Saeed Mahameed <saeed@kernel.org>)
Changes in v2:
- Added NULL check in rose_loopback_timer() (Suggested-by: Greg KH <gregkh@linuxfoundation.org>)
net/rose/rose_loopback.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/rose/rose_loopback.c b/net/rose/rose_loopback.c
index 7b094275ea8b..6a71b6947d92 100644
--- a/net/rose/rose_loopback.c
+++ b/net/rose/rose_loopback.c
@@ -96,10 +96,12 @@ static void rose_loopback_timer(struct timer_list *unused)
}
if (frametype == ROSE_CALL_REQUEST) {
- if ((dev = rose_dev_get(dest)) != NULL) {
+ dev = rose_dev_get(dest);
+ if (rose_loopback_neigh->dev && dev) {
if (rose_rx_call_request(skb, dev, rose_loopback_neigh, lci_o) == 0)
kfree_skb(skb);
} else {
+ dev_put(dev);
kfree_skb(skb);
}
} else {
--
2.29.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH v4 net] rose: Fix Null pointer dereference in rose_send_frame()
2020-11-11 16:59 ` [Linux-kernel-mentees] [PATCH v4 " Anmol Karn
@ 2020-11-14 19:18 ` Jakub Kicinski
2020-11-15 11:44 ` Anmol Karn
0 siblings, 1 reply; 13+ messages in thread
From: Jakub Kicinski @ 2020-11-14 19:18 UTC (permalink / raw)
To: Anmol Karn
Cc: ralf, davem, saeed, gregkh, netdev, linux-kernel, linux-hams,
linux-kernel-mentees, syzkaller-bugs,
syzbot+a1c743815982d9496393
On Wed, 11 Nov 2020 22:29:54 +0530 Anmol Karn wrote:
> rose_send_frame() dereferences `neigh->dev` when called from
> rose_transmit_clear_request(), and the first occurrence of the
> `neigh` is in rose_loopback_timer() as `rose_loopback_neigh`,
> and it is initialized in rose_add_loopback_neigh() as NULL.
> i.e when `rose_loopback_neigh` used in rose_loopback_timer()
> its `->dev` was still NULL and rose_loopback_timer() was calling
> rose_rx_call_request() without checking for NULL.
>
> - net/rose/rose_link.c
> This bug seems to get triggered in this line:
>
> rose_call = (ax25_address *)neigh->dev->dev_addr;
>
> Fix it by adding NULL checking for `rose_loopback_neigh->dev`
> in rose_loopback_timer().
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Reported-by: syzbot+a1c743815982d9496393@syzkaller.appspotmail.com
> Tested-by: syzbot+a1c743815982d9496393@syzkaller.appspotmail.com
> Link: https://syzkaller.appspot.com/bug?id=9d2a7ca8c7f2e4b682c97578dfa3f236258300b3
> Signed-off-by: Anmol Karn <anmol.karan123@gmail.com>
> diff --git a/net/rose/rose_loopback.c b/net/rose/rose_loopback.c
> index 7b094275ea8b..6a71b6947d92 100644
> --- a/net/rose/rose_loopback.c
> +++ b/net/rose/rose_loopback.c
> @@ -96,10 +96,12 @@ static void rose_loopback_timer(struct timer_list *unused)
> }
>
> if (frametype == ROSE_CALL_REQUEST) {
> - if ((dev = rose_dev_get(dest)) != NULL) {
> + dev = rose_dev_get(dest);
> + if (rose_loopback_neigh->dev && dev) {
> if (rose_rx_call_request(skb, dev, rose_loopback_neigh, lci_o) == 0)
> kfree_skb(skb);
> } else {
> + dev_put(dev);
> kfree_skb(skb);
> }
> } else {
This is still not correct. With this code dev_put() could be called with
NULL, which would cause a crash.
There is also a dev_put() missing if rose_rx_call_request() returns 0.
I think that this is the correct code:
diff --git a/net/rose/rose_loopback.c b/net/rose/rose_loopback.c
index 7b094275ea8b..ff252ef73592 100644
--- a/net/rose/rose_loopback.c
+++ b/net/rose/rose_loopback.c
@@ -96,11 +96,22 @@ static void rose_loopback_timer(struct timer_list *unused)
}
if (frametype == ROSE_CALL_REQUEST) {
- if ((dev = rose_dev_get(dest)) != NULL) {
- if (rose_rx_call_request(skb, dev, rose_loopback_neigh, lci_o) == 0)
- kfree_skb(skb);
- } else {
+ if (!rose_loopback_neigh->dev) {
kfree_skb(skb);
+ continue;
+ }
+
+ dev = rose_dev_get(dest);
+ if (!dev) {
+ kfree_skb(skb);
+ continue;
+ }
+
+ if (rose_rx_call_request(skb, dev, rose_loopback_neigh,
+ lci_o) == 0) {
+ dev_put(dev);
+ kfree_skb(skb);
}
} else {
kfree_skb(skb);
Please test this and resubmit it if it works.
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH v4 net] rose: Fix Null pointer dereference in rose_send_frame()
2020-11-14 19:18 ` Jakub Kicinski
@ 2020-11-15 11:44 ` Anmol Karn
2020-11-19 19:10 ` [Linux-kernel-mentees] [PATCH v5 " Anmol Karn
0 siblings, 1 reply; 13+ messages in thread
From: Anmol Karn @ 2020-11-15 11:44 UTC (permalink / raw)
To: Jakub Kicinski
Cc: ralf, davem, saeed, gregkh, netdev, linux-kernel, linux-hams,
linux-kernel-mentees, syzkaller-bugs,
syzbot+a1c743815982d9496393
On Sat, Nov 14, 2020 at 11:18:38AM -0800, Jakub Kicinski wrote:
> On Wed, 11 Nov 2020 22:29:54 +0530 Anmol Karn wrote:
> > rose_send_frame() dereferences `neigh->dev` when called from
> > rose_transmit_clear_request(), and the first occurrence of the
> > `neigh` is in rose_loopback_timer() as `rose_loopback_neigh`,
> > and it is initialized in rose_add_loopback_neigh() as NULL.
> > i.e when `rose_loopback_neigh` used in rose_loopback_timer()
> > its `->dev` was still NULL and rose_loopback_timer() was calling
> > rose_rx_call_request() without checking for NULL.
> >
> > - net/rose/rose_link.c
> > This bug seems to get triggered in this line:
> >
> > rose_call = (ax25_address *)neigh->dev->dev_addr;
> >
> > Fix it by adding NULL checking for `rose_loopback_neigh->dev`
> > in rose_loopback_timer().
> >
> > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > Reported-by: syzbot+a1c743815982d9496393@syzkaller.appspotmail.com
> > Tested-by: syzbot+a1c743815982d9496393@syzkaller.appspotmail.com
> > Link: https://syzkaller.appspot.com/bug?id=9d2a7ca8c7f2e4b682c97578dfa3f236258300b3
> > Signed-off-by: Anmol Karn <anmol.karan123@gmail.com>
>
> > diff --git a/net/rose/rose_loopback.c b/net/rose/rose_loopback.c
> > index 7b094275ea8b..6a71b6947d92 100644
> > --- a/net/rose/rose_loopback.c
> > +++ b/net/rose/rose_loopback.c
> > @@ -96,10 +96,12 @@ static void rose_loopback_timer(struct timer_list *unused)
> > }
> >
> > if (frametype == ROSE_CALL_REQUEST) {
> > - if ((dev = rose_dev_get(dest)) != NULL) {
> > + dev = rose_dev_get(dest);
> > + if (rose_loopback_neigh->dev && dev) {
> > if (rose_rx_call_request(skb, dev, rose_loopback_neigh, lci_o) == 0)
> > kfree_skb(skb);
> > } else {
> > + dev_put(dev);
> > kfree_skb(skb);
> > }
> > } else {
>
> This is still not correct. With this code dev_put() could be called with
> NULL, which would cause a crash.
>
> There is also a dev_put() missing if rose_rx_call_request() returns 0.
>
> I think that this is the correct code:
>
> diff --git a/net/rose/rose_loopback.c b/net/rose/rose_loopback.c
> index 7b094275ea8b..ff252ef73592 100644
> --- a/net/rose/rose_loopback.c
> +++ b/net/rose/rose_loopback.c
> @@ -96,11 +96,22 @@ static void rose_loopback_timer(struct timer_list *unused)
> }
>
> if (frametype == ROSE_CALL_REQUEST) {
> - if ((dev = rose_dev_get(dest)) != NULL) {
> - if (rose_rx_call_request(skb, dev, rose_loopback_neigh, lci_o) == 0)
> - kfree_skb(skb);
> - } else {
> + if (!rose_loopback_neigh->dev) {
> kfree_skb(skb);
> + continue;
> + }
> +
> + dev = rose_dev_get(dest);
> + if (!dev) {
> + kfree_skb(skb);
> + continue;
> + }
> +
> + if (rose_rx_call_request(skb, dev, rose_loopback_neigh,
> + lci_o) == 0) {
> + dev_put(dev);
> + kfree_skb(skb);
> }
> } else {
> kfree_skb(skb);
>
> Please test this and resubmit it if it works.
Sure sir, I will test it and resend, if it works.
Thanks,
Anmol
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Linux-kernel-mentees] [PATCH v5 net] rose: Fix Null pointer dereference in rose_send_frame()
2020-11-15 11:44 ` Anmol Karn
@ 2020-11-19 19:10 ` Anmol Karn
2020-11-20 18:06 ` Jakub Kicinski
0 siblings, 1 reply; 13+ messages in thread
From: Anmol Karn @ 2020-11-19 19:10 UTC (permalink / raw)
To: ralf, davem, kuba
Cc: saeed, gregkh, netdev, linux-kernel, linux-hams,
linux-kernel-mentees, syzkaller-bugs, anmol.karan123,
syzbot+a1c743815982d9496393
rose_send_frame() dereferences `neigh->dev` when called from
rose_transmit_clear_request(), and the first occurrence of the
`neigh` is in rose_loopback_timer() as `rose_loopback_neigh`,
and it is initialized in rose_add_loopback_neigh() as NULL.
i.e when `rose_loopback_neigh` used in rose_loopback_timer()
its `->dev` was still NULL and rose_loopback_timer() was calling
rose_rx_call_request() without checking for NULL.
- net/rose/rose_link.c
This bug seems to get triggered in this line:
rose_call = (ax25_address *)neigh->dev->dev_addr;
Fix it by adding NULL checking for `rose_loopback_neigh->dev`
in rose_loopback_timer().
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Suggested-by: Jakub Kicinski <kuba@kernel.org>
Reported-by: syzbot+a1c743815982d9496393@syzkaller.appspotmail.com
Tested-by: syzbot+a1c743815982d9496393@syzkaller.appspotmail.com
Link: https://syzkaller.appspot.com/bug?id=9d2a7ca8c7f2e4b682c97578dfa3f236258300b3
Signed-off-by: Anmol Karn <anmol.karan123@gmail.com>
---
Changes in v5:
- Free `dev` in rose_rx_call_request() and add NULL check for `dev` before freeing it.
(Suggested-by: Jakub Kicinski <kuba@kernel.org>)
Changes in v4:
- Free `dev`(on dev_hold()), when neigh->dev is NULL. (Suggested-by: Jakub Kicinski <kuba@kernel.org>)
Changes in v3:
- Corrected checkpatch warnings and errors (Suggested-by: Saeed Mahameed <saeed@kernel.org>)
- Added "Fixes:" tag (Suggested-by: Saeed Mahameed <saeed@kernel.org>)
Changes in v2:
- Added NULL check in rose_loopback_timer() (Suggested-by: Greg KH <gregkh@linuxfoundation.org>)
net/rose/rose_loopback.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/net/rose/rose_loopback.c b/net/rose/rose_loopback.c
index 7b094275ea8b..11c45c8c6c16 100644
--- a/net/rose/rose_loopback.c
+++ b/net/rose/rose_loopback.c
@@ -96,10 +96,19 @@ static void rose_loopback_timer(struct timer_list *unused)
}
if (frametype == ROSE_CALL_REQUEST) {
- if ((dev = rose_dev_get(dest)) != NULL) {
- if (rose_rx_call_request(skb, dev, rose_loopback_neigh, lci_o) == 0)
- kfree_skb(skb);
- } else {
+ if (!rose_loopback_neigh->dev) {
+ kfree_skb(skb);
+ continue;
+ }
+
+ dev = rose_dev_get(dest);
+ if (!dev) {
+ kfree_skb(skb);
+ continue;
+ }
+
+ if (rose_rx_call_request(skb, dev, rose_loopback_neigh, lci_o) == 0) {
+ dev_put(dev);
kfree_skb(skb);
}
} else {
--
2.29.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH v5 net] rose: Fix Null pointer dereference in rose_send_frame()
2020-11-19 19:10 ` [Linux-kernel-mentees] [PATCH v5 " Anmol Karn
@ 2020-11-20 18:06 ` Jakub Kicinski
0 siblings, 0 replies; 13+ messages in thread
From: Jakub Kicinski @ 2020-11-20 18:06 UTC (permalink / raw)
To: Anmol Karn
Cc: ralf, davem, saeed, gregkh, netdev, linux-kernel, linux-hams,
linux-kernel-mentees, syzkaller-bugs,
syzbot+a1c743815982d9496393
On Fri, 20 Nov 2020 00:40:43 +0530 Anmol Karn wrote:
> rose_send_frame() dereferences `neigh->dev` when called from
> rose_transmit_clear_request(), and the first occurrence of the
> `neigh` is in rose_loopback_timer() as `rose_loopback_neigh`,
> and it is initialized in rose_add_loopback_neigh() as NULL.
> i.e when `rose_loopback_neigh` used in rose_loopback_timer()
> its `->dev` was still NULL and rose_loopback_timer() was calling
> rose_rx_call_request() without checking for NULL.
>
> - net/rose/rose_link.c
> This bug seems to get triggered in this line:
>
> rose_call = (ax25_address *)neigh->dev->dev_addr;
>
> Fix it by adding NULL checking for `rose_loopback_neigh->dev`
> in rose_loopback_timer().
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Suggested-by: Jakub Kicinski <kuba@kernel.org>
> Reported-by: syzbot+a1c743815982d9496393@syzkaller.appspotmail.com
> Tested-by: syzbot+a1c743815982d9496393@syzkaller.appspotmail.com
> Link: https://syzkaller.appspot.com/bug?id=9d2a7ca8c7f2e4b682c97578dfa3f236258300b3
> Signed-off-by: Anmol Karn <anmol.karan123@gmail.com>
Applied to net, thanks!
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2020-11-20 18:06 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-05 15:56 [Linux-kernel-mentees] [PATCH v2 net] rose: Fix Null pointer dereference in rose_send_frame() Anmol Karn
2020-11-06 21:04 ` Saeed Mahameed
2020-11-07 8:20 ` Anmol Karn
2020-11-07 19:18 ` [Linux-kernel-mentees] [PATCH v3 " Anmol Karn
2020-11-10 17:58 ` Jakub Kicinski
2020-11-10 19:45 ` Anmol Karn
2020-11-11 16:59 ` [Linux-kernel-mentees] [PATCH v4 " Anmol Karn
2020-11-14 19:18 ` Jakub Kicinski
2020-11-15 11:44 ` Anmol Karn
2020-11-19 19:10 ` [Linux-kernel-mentees] [PATCH v5 " Anmol Karn
2020-11-20 18:06 ` Jakub Kicinski
2020-11-07 18:56 ` [Linux-kernel-mentees] [PATCH v2] net: " Anmol Karn
2020-11-07 19:02 ` Anmol karn
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).