* [PATCH net-next] seg6: fix parameter passing when calling NF_HOOK() in End.DX4 and End.DX6 behaviors
@ 2024-05-30 7:43 Jianguo Wu
2024-06-03 18:21 ` Simon Horman
2024-06-03 19:23 ` Pablo Neira Ayuso
0 siblings, 2 replies; 6+ messages in thread
From: Jianguo Wu @ 2024-05-30 7:43 UTC (permalink / raw)
To: netdev; +Cc: contact, pablo, David Ahern, Paolo Abeni
From: Jianguo Wu <wujianguo@chinatelecom.cn>
input_action_end_dx4() and input_action_end_dx6() call NF_HOOK() for PREROUTING hook,
for PREROUTING hook, we should passing a valid indev, and a NULL outdev to NF_HOOK(),
otherwise may trigger a NULL pointer dereference, as below:
[74830.647293] BUG: kernel NULL pointer dereference, address: 0000000000000090
[74830.655633] #PF: supervisor read access in kernel mode
[74830.657888] #PF: error_code(0x0000) - not-present page
[74830.659500] PGD 0 P4D 0
[74830.660450] Oops: 0000 [#1] PREEMPT SMP PTI
...
[74830.664953] Hardware name: Red Hat KVM, BIOS 0.5.1 01/01/2011
[74830.666569] RIP: 0010:rpfilter_mt+0x44/0x15e [ipt_rpfilter]
...
[74830.689725] Call Trace:
[74830.690402] <IRQ>
[74830.690953] ? show_trace_log_lvl+0x1c4/0x2df
[74830.692020] ? show_trace_log_lvl+0x1c4/0x2df
[74830.693095] ? ipt_do_table+0x286/0x710 [ip_tables]
[74830.694275] ? __die_body.cold+0x8/0xd
[74830.695205] ? page_fault_oops+0xac/0x140
[74830.696244] ? exc_page_fault+0x62/0x150
[74830.697225] ? asm_exc_page_fault+0x22/0x30
[74830.698344] ? rpfilter_mt+0x44/0x15e [ipt_rpfilter]
[74830.699540] ipt_do_table+0x286/0x710 [ip_tables]
[74830.700758] ? ip6_route_input+0x19d/0x240
[74830.701752] nf_hook_slow+0x3f/0xb0
[74830.702678] input_action_end_dx4+0x19b/0x1e0
[74830.703735] ? input_action_end_t+0xe0/0xe0
[74830.704734] seg6_local_input_core+0x2d/0x60
[74830.705782] lwtunnel_input+0x5b/0xb0
[74830.706690] __netif_receive_skb_one_core+0x63/0xa0
[74830.707825] process_backlog+0x99/0x140
[74830.709538] __napi_poll+0x2c/0x160
[74830.710673] net_rx_action+0x296/0x350
[74830.711860] __do_softirq+0xcb/0x2ac
[74830.713049] do_softirq+0x63/0x90
input_action_end_dx4() passing a NULL indev to NF_HOOK(), and finally trigger a
NULL dereference in rpfilter_mt()->rpfilter_is_loopback():
static bool
rpfilter_is_loopback(const struct sk_buff *skb, const struct net_device *in)
{
// in is NULL
return skb->pkt_type == PACKET_LOOPBACK || in->flags & IFF_LOOPBACK;
}
Fixes: 7a3f5b0de364 ("netfilter: add netfilter hooks to SRv6 data plane")
Signed-off-by: Jianguo Wu <wujianguo@chinatelecom.cn>
---
net/ipv6/seg6_local.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/ipv6/seg6_local.c b/net/ipv6/seg6_local.c
index 24e2b4b..c434940 100644
--- a/net/ipv6/seg6_local.c
+++ b/net/ipv6/seg6_local.c
@@ -941,8 +941,8 @@ static int input_action_end_dx6(struct sk_buff *skb,
if (static_branch_unlikely(&nf_hooks_lwtunnel_enabled))
return NF_HOOK(NFPROTO_IPV6, NF_INET_PRE_ROUTING,
- dev_net(skb->dev), NULL, skb, NULL,
- skb_dst(skb)->dev, input_action_end_dx6_finish);
+ dev_net(skb->dev), NULL, skb, skb->dev,
+ NULL, input_action_end_dx6_finish);
return input_action_end_dx6_finish(dev_net(skb->dev), NULL, skb);
drop:
@@ -991,8 +991,8 @@ static int input_action_end_dx4(struct sk_buff *skb,
if (static_branch_unlikely(&nf_hooks_lwtunnel_enabled))
return NF_HOOK(NFPROTO_IPV4, NF_INET_PRE_ROUTING,
- dev_net(skb->dev), NULL, skb, NULL,
- skb_dst(skb)->dev, input_action_end_dx4_finish);
+ dev_net(skb->dev), NULL, skb, skb->dev,
+ NULL, input_action_end_dx4_finish);
return input_action_end_dx4_finish(dev_net(skb->dev), NULL, skb);
drop:
--
1.8.3.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH net-next] seg6: fix parameter passing when calling NF_HOOK() in End.DX4 and End.DX6 behaviors
2024-05-30 7:43 [PATCH net-next] seg6: fix parameter passing when calling NF_HOOK() in End.DX4 and End.DX6 behaviors Jianguo Wu
@ 2024-06-03 18:21 ` Simon Horman
2024-06-04 6:33 ` Jianguo Wu
2024-06-03 19:23 ` Pablo Neira Ayuso
1 sibling, 1 reply; 6+ messages in thread
From: Simon Horman @ 2024-06-03 18:21 UTC (permalink / raw)
To: Jianguo Wu; +Cc: netdev, contact, pablo, David Ahern, Paolo Abeni
On Thu, May 30, 2024 at 03:43:38PM +0800, Jianguo Wu wrote:
> From: Jianguo Wu <wujianguo@chinatelecom.cn>
>
> input_action_end_dx4() and input_action_end_dx6() call NF_HOOK() for PREROUTING hook,
> for PREROUTING hook, we should passing a valid indev, and a NULL outdev to NF_HOOK(),
> otherwise may trigger a NULL pointer dereference, as below:
nit: The text above should be line-wrapped so that it is
no more than 75 columns wide.
Link: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-your-changes
>
> [74830.647293] BUG: kernel NULL pointer dereference, address: 0000000000000090
> [74830.655633] #PF: supervisor read access in kernel mode
> [74830.657888] #PF: error_code(0x0000) - not-present page
> [74830.659500] PGD 0 P4D 0
> [74830.660450] Oops: 0000 [#1] PREEMPT SMP PTI
> ...
> [74830.664953] Hardware name: Red Hat KVM, BIOS 0.5.1 01/01/2011
> [74830.666569] RIP: 0010:rpfilter_mt+0x44/0x15e [ipt_rpfilter]
> ...
> [74830.689725] Call Trace:
> [74830.690402] <IRQ>
> [74830.690953] ? show_trace_log_lvl+0x1c4/0x2df
> [74830.692020] ? show_trace_log_lvl+0x1c4/0x2df
> [74830.693095] ? ipt_do_table+0x286/0x710 [ip_tables]
> [74830.694275] ? __die_body.cold+0x8/0xd
> [74830.695205] ? page_fault_oops+0xac/0x140
> [74830.696244] ? exc_page_fault+0x62/0x150
> [74830.697225] ? asm_exc_page_fault+0x22/0x30
> [74830.698344] ? rpfilter_mt+0x44/0x15e [ipt_rpfilter]
> [74830.699540] ipt_do_table+0x286/0x710 [ip_tables]
> [74830.700758] ? ip6_route_input+0x19d/0x240
> [74830.701752] nf_hook_slow+0x3f/0xb0
> [74830.702678] input_action_end_dx4+0x19b/0x1e0
> [74830.703735] ? input_action_end_t+0xe0/0xe0
> [74830.704734] seg6_local_input_core+0x2d/0x60
> [74830.705782] lwtunnel_input+0x5b/0xb0
> [74830.706690] __netif_receive_skb_one_core+0x63/0xa0
> [74830.707825] process_backlog+0x99/0x140
> [74830.709538] __napi_poll+0x2c/0x160
> [74830.710673] net_rx_action+0x296/0x350
> [74830.711860] __do_softirq+0xcb/0x2ac
> [74830.713049] do_softirq+0x63/0x90
>
> input_action_end_dx4() passing a NULL indev to NF_HOOK(), and finally trigger a
> NULL dereference in rpfilter_mt()->rpfilter_is_loopback():
> static bool
> rpfilter_is_loopback(const struct sk_buff *skb, const struct net_device *in)
> {
> // in is NULL
> return skb->pkt_type == PACKET_LOOPBACK || in->flags & IFF_LOOPBACK;
> }
>
> Fixes: 7a3f5b0de364 ("netfilter: add netfilter hooks to SRv6 data plane")
nit: no blank line here.
>
> Signed-off-by: Jianguo Wu <wujianguo@chinatelecom.cn>
I am slightly puzzled that this bug was in
the tree for so long without being noticed.
But the above not withstanding, this looks good to me.
Reviewed-by: Simon Horman <horms@kernel.org>
...
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH net-next] seg6: fix parameter passing when calling NF_HOOK() in End.DX4 and End.DX6 behaviors
2024-06-03 18:21 ` Simon Horman
@ 2024-06-04 6:33 ` Jianguo Wu
0 siblings, 0 replies; 6+ messages in thread
From: Jianguo Wu @ 2024-06-04 6:33 UTC (permalink / raw)
To: Simon Horman; +Cc: netdev, contact, pablo, David Ahern, Paolo Abeni
Hi Simon,
Thanks for your review, I will fix the commit log, and send v2 with your Reviewed-by tag.
On 2024/6/4 2:21, Simon Horman wrote:
> On Thu, May 30, 2024 at 03:43:38PM +0800, Jianguo Wu wrote:
>> From: Jianguo Wu <wujianguo@chinatelecom.cn>
>>
>> input_action_end_dx4() and input_action_end_dx6() call NF_HOOK() for PREROUTING hook,
>> for PREROUTING hook, we should passing a valid indev, and a NULL outdev to NF_HOOK(),
>> otherwise may trigger a NULL pointer dereference, as below:
>
> nit: The text above should be line-wrapped so that it is
> no more than 75 columns wide.
>
> Link: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-your-changes
>
>>
>> [74830.647293] BUG: kernel NULL pointer dereference, address: 0000000000000090
>> [74830.655633] #PF: supervisor read access in kernel mode
>> [74830.657888] #PF: error_code(0x0000) - not-present page
>> [74830.659500] PGD 0 P4D 0
>> [74830.660450] Oops: 0000 [#1] PREEMPT SMP PTI
>> ...
>> [74830.664953] Hardware name: Red Hat KVM, BIOS 0.5.1 01/01/2011
>> [74830.666569] RIP: 0010:rpfilter_mt+0x44/0x15e [ipt_rpfilter]
>> ...
>> [74830.689725] Call Trace:
>> [74830.690402] <IRQ>
>> [74830.690953] ? show_trace_log_lvl+0x1c4/0x2df
>> [74830.692020] ? show_trace_log_lvl+0x1c4/0x2df
>> [74830.693095] ? ipt_do_table+0x286/0x710 [ip_tables]
>> [74830.694275] ? __die_body.cold+0x8/0xd
>> [74830.695205] ? page_fault_oops+0xac/0x140
>> [74830.696244] ? exc_page_fault+0x62/0x150
>> [74830.697225] ? asm_exc_page_fault+0x22/0x30
>> [74830.698344] ? rpfilter_mt+0x44/0x15e [ipt_rpfilter]
>> [74830.699540] ipt_do_table+0x286/0x710 [ip_tables]
>> [74830.700758] ? ip6_route_input+0x19d/0x240
>> [74830.701752] nf_hook_slow+0x3f/0xb0
>> [74830.702678] input_action_end_dx4+0x19b/0x1e0
>> [74830.703735] ? input_action_end_t+0xe0/0xe0
>> [74830.704734] seg6_local_input_core+0x2d/0x60
>> [74830.705782] lwtunnel_input+0x5b/0xb0
>> [74830.706690] __netif_receive_skb_one_core+0x63/0xa0
>> [74830.707825] process_backlog+0x99/0x140
>> [74830.709538] __napi_poll+0x2c/0x160
>> [74830.710673] net_rx_action+0x296/0x350
>> [74830.711860] __do_softirq+0xcb/0x2ac
>> [74830.713049] do_softirq+0x63/0x90
>>
>> input_action_end_dx4() passing a NULL indev to NF_HOOK(), and finally trigger a
>> NULL dereference in rpfilter_mt()->rpfilter_is_loopback():
>> static bool
>> rpfilter_is_loopback(const struct sk_buff *skb, const struct net_device *in)
>> {
>> // in is NULL
>> return skb->pkt_type == PACKET_LOOPBACK || in->flags & IFF_LOOPBACK;
>> }
>>
>> Fixes: 7a3f5b0de364 ("netfilter: add netfilter hooks to SRv6 data plane")
>
> nit: no blank line here.
>
>>
>> Signed-off-by: Jianguo Wu <wujianguo@chinatelecom.cn>
>
> I am slightly puzzled that this bug was in
> the tree for so long without being noticed.
>
> But the above not withstanding, this looks good to me.
>
> Reviewed-by: Simon Horman <horms@kernel.org>
>
> ...
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] seg6: fix parameter passing when calling NF_HOOK() in End.DX4 and End.DX6 behaviors
2024-05-30 7:43 [PATCH net-next] seg6: fix parameter passing when calling NF_HOOK() in End.DX4 and End.DX6 behaviors Jianguo Wu
2024-06-03 18:21 ` Simon Horman
@ 2024-06-03 19:23 ` Pablo Neira Ayuso
2024-06-04 6:34 ` Jianguo Wu
1 sibling, 1 reply; 6+ messages in thread
From: Pablo Neira Ayuso @ 2024-06-03 19:23 UTC (permalink / raw)
To: Jianguo Wu; +Cc: netdev, contact, David Ahern, Paolo Abeni
Hi,
On Thu, May 30, 2024 at 03:43:38PM +0800, Jianguo Wu wrote:
> From: Jianguo Wu <wujianguo@chinatelecom.cn>
>
> input_action_end_dx4() and input_action_end_dx6() call NF_HOOK() for PREROUTING hook,
> for PREROUTING hook, we should passing a valid indev, and a NULL outdev to NF_HOOK(),
> otherwise may trigger a NULL pointer dereference, as below:
Could you also add a selftest to improve coverage of this infrastructure?
Thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] seg6: fix parameter passing when calling NF_HOOK() in End.DX4 and End.DX6 behaviors
2024-06-03 19:23 ` Pablo Neira Ayuso
@ 2024-06-04 6:34 ` Jianguo Wu
2024-06-04 9:14 ` Paolo Abeni
0 siblings, 1 reply; 6+ messages in thread
From: Jianguo Wu @ 2024-06-04 6:34 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netdev, contact, David Ahern, Paolo Abeni
Hi, Pablo
On 2024/6/4 3:23, Pablo Neira Ayuso wrote:
> Hi,
>
> On Thu, May 30, 2024 at 03:43:38PM +0800, Jianguo Wu wrote:
>> From: Jianguo Wu <wujianguo@chinatelecom.cn>
>>
>> input_action_end_dx4() and input_action_end_dx6() call NF_HOOK() for PREROUTING hook,
>> for PREROUTING hook, we should passing a valid indev, and a NULL outdev to NF_HOOK(),
>> otherwise may trigger a NULL pointer dereference, as below:
>
> Could you also add a selftest to improve coverage of this infrastructure?
>
Sure, I will add a selftest to cover this case.
> Thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] seg6: fix parameter passing when calling NF_HOOK() in End.DX4 and End.DX6 behaviors
2024-06-04 6:34 ` Jianguo Wu
@ 2024-06-04 9:14 ` Paolo Abeni
0 siblings, 0 replies; 6+ messages in thread
From: Paolo Abeni @ 2024-06-04 9:14 UTC (permalink / raw)
To: Jianguo Wu, Pablo Neira Ayuso; +Cc: netdev, contact, David Ahern
On Tue, 2024-06-04 at 14:34 +0800, Jianguo Wu wrote:
> Hi, Pablo
>
> On 2024/6/4 3:23, Pablo Neira Ayuso wrote:
> > Hi,
> >
> > On Thu, May 30, 2024 at 03:43:38PM +0800, Jianguo Wu wrote:
> > > From: Jianguo Wu <wujianguo@chinatelecom.cn>
> > >
> > > input_action_end_dx4() and input_action_end_dx6() call NF_HOOK() for PREROUTING hook,
> > > for PREROUTING hook, we should passing a valid indev, and a NULL outdev to NF_HOOK(),
> > > otherwise may trigger a NULL pointer dereference, as below:
> >
> > Could you also add a selftest to improve coverage of this infrastructure?
> >
>
> Sure, I will add a selftest to cover this case.
When re-submitting, please additionally target the 'net' tree,
replacing the 'net-next' prefix with 'net',
Thanks!
Paolo
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-06-04 9:14 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-30 7:43 [PATCH net-next] seg6: fix parameter passing when calling NF_HOOK() in End.DX4 and End.DX6 behaviors Jianguo Wu
2024-06-03 18:21 ` Simon Horman
2024-06-04 6:33 ` Jianguo Wu
2024-06-03 19:23 ` Pablo Neira Ayuso
2024-06-04 6:34 ` Jianguo Wu
2024-06-04 9:14 ` Paolo Abeni
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).