From: Daniel Borkmann <daniel@iogearbox.net>
To: David Ahern <dsahern@gmail.com>,
xiakaixu1987@gmail.com, ast@kernel.org, kafai@fb.com,
songliubraving@fb.com, yhs@fb.com, andrii@kernel.org,
john.fastabend@gmail.com, kpsingh@chromium.org
Cc: netdev@vger.kernel.org, bpf@vger.kernel.org,
linux-kernel@vger.kernel.org, Kaixu Xia <kaixuxia@tencent.com>
Subject: Re: [PATCH] bpf: Check the return value of dev_get_by_index_rcu()
Date: Fri, 20 Nov 2020 17:01:57 +0100 [thread overview]
Message-ID: <41aac020-b7af-e83f-0639-252efa4f7fff@iogearbox.net> (raw)
In-Reply-To: <f8ff26f0-b1b6-6dd1-738d-4c592a8efdb0@gmail.com>
On 11/20/20 4:19 PM, David Ahern wrote:
> On 11/20/20 8:13 AM, Daniel Borkmann wrote:
>> [ +David ]
>>
>> On 11/19/20 8:04 AM, xiakaixu1987@gmail.com wrote:
>>> From: Kaixu Xia <kaixuxia@tencent.com>
>>>
>>> The return value of dev_get_by_index_rcu() can be NULL, so here it
>>> is need to check the return value and return error code if it is NULL.
>>>
>>> Signed-off-by: Kaixu Xia <kaixuxia@tencent.com>
>>> ---
>>> net/core/filter.c | 2 ++
>>> 1 file changed, 2 insertions(+)
>>>
>>> diff --git a/net/core/filter.c b/net/core/filter.c
>>> index 2ca5eecebacf..1263fe07170a 100644
>>> --- a/net/core/filter.c
>>> +++ b/net/core/filter.c
>>> @@ -5573,6 +5573,8 @@ BPF_CALL_4(bpf_skb_fib_lookup, struct sk_buff *,
>>> skb,
>>> struct net_device *dev;
>>> dev = dev_get_by_index_rcu(net, params->ifindex);
>>> + if (unlikely(!dev))
>>> + return -EINVAL;
>>> if (!is_skb_forwardable(dev, skb))
>>> rc = BPF_FIB_LKUP_RET_FRAG_NEEDED;
>
> rcu lock is held right? It is impossible for dev to return NULL here.
Yes, we're under RCU read side. Was wondering whether we could unlink it
from the list but not yet free it, but also that seems not possible since
we'd first need to close it which already has a synchronize_net(). So not
an issue what Kaixu describes in the commit msg, agree.
prev parent reply other threads:[~2020-11-20 16:02 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-19 7:04 [PATCH] bpf: Check the return value of dev_get_by_index_rcu() xiakaixu1987
2020-11-20 15:13 ` Daniel Borkmann
2020-11-20 15:19 ` David Ahern
2020-11-20 16:01 ` Daniel Borkmann [this message]
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=41aac020-b7af-e83f-0639-252efa4f7fff@iogearbox.net \
--to=daniel@iogearbox.net \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=dsahern@gmail.com \
--cc=john.fastabend@gmail.com \
--cc=kafai@fb.com \
--cc=kaixuxia@tencent.com \
--cc=kpsingh@chromium.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=songliubraving@fb.com \
--cc=xiakaixu1987@gmail.com \
--cc=yhs@fb.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 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).