[PATCH bpf] bpf: Fix use-after-free in offloaded map/prog info fill

Jiayuan Chen posted 1 patch 2 months, 1 week ago
There is a newer version of this series
kernel/bpf/offload.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
[PATCH bpf] bpf: Fix use-after-free in offloaded map/prog info fill
Posted by Jiayuan Chen 2 months, 1 week ago
When querying info for an offloaded BPF map or program,
bpf_map_offload_info_fill_ns() and bpf_prog_offload_info_fill_ns()
obtain the network namespace with get_net(dev_net(offmap->netdev)).
However, the associated netdev's netns may be racing with teardown
during netns destruction. If the netns refcount has already reached 0,
get_net() performs a refcount_t increment on 0, triggering:

  refcount_t: addition on 0; use-after-free.

Although rtnl_lock and bpf_devs_lock ensure the netdev pointer remains
valid, they cannot prevent the netns refcount from reaching zero.

Fix this by using maybe_get_net() instead of get_net(). maybe_get_net()
uses refcount_inc_not_zero() and returns NULL if the refcount is already
zero, which causes ns_get_path_cb() to fail and the caller to return
-ENODEV -- the correct behavior when the netns is being destroyed.

Fixes: 675fc275a3a2d ("bpf: offload: report device information for offloaded programs")
Fixes: 52775b33bb507 ("bpf: offload: report device information about offloaded maps")
Reported-by: Yinhao Hu <dddddd@hust.edu.cn>
Reported-by: Kaiyan Mei <M202472210@hust.edu.cn>
Reported-by: Dongliang Mu <dzm91@hust.edu.cn>
Closes: https://lore.kernel.org/bpf/f0aa3678-79c9-47ae-9e8c-02a3d1df160a@hust.edu.cn/
Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
---
 kernel/bpf/offload.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/kernel/bpf/offload.c b/kernel/bpf/offload.c
index 0ad97d643bf49..d6dc684d0e872 100644
--- a/kernel/bpf/offload.c
+++ b/kernel/bpf/offload.c
@@ -435,9 +435,12 @@ static struct ns_common *bpf_prog_offload_info_fill_ns(void *private_data)
 
 	if (aux->offload) {
 		args->info->ifindex = aux->offload->netdev->ifindex;
-		net = dev_net(aux->offload->netdev);
-		get_net(net);
-		ns = &net->ns;
+		net = maybe_get_net(dev_net(aux->offload->netdev));
+		if (net) {
+			ns = &net->ns;
+		} else {
+			ns = NULL;
+		}
 	} else {
 		args->info->ifindex = 0;
 		ns = NULL;
@@ -647,9 +650,12 @@ static struct ns_common *bpf_map_offload_info_fill_ns(void *private_data)
 
 	if (args->offmap->netdev) {
 		args->info->ifindex = args->offmap->netdev->ifindex;
-		net = dev_net(args->offmap->netdev);
-		get_net(net);
-		ns = &net->ns;
+		net = maybe_get_net(dev_net(args->offmap->netdev));
+		if (net) {
+			ns = &net->ns;
+		} else {
+			ns = NULL;
+		}
 	} else {
 		args->info->ifindex = 0;
 		ns = NULL;
-- 
2.43.0
Re: [PATCH bpf] bpf: Fix use-after-free in offloaded map/prog info fill
Posted by bot+bpf-ci@kernel.org 2 months, 1 week ago
> Fix this by using maybe_get_net() instead of get_net(). maybe_get_net()
> uses refcount_inc_not_zero() and returns NULL if the refcount is already
> zero, which causes ns_get_path_cb() to fail and the caller to return
> -ENODEV -- the correct behavior when the netns is being destroyed.

Should this say -ENOENT rather than -ENODEV?  When maybe_get_net()
returns NULL, ns_get_path_cb() returns -ENOENT:

fs/nsfs.c:ns_get_path_cb() {
    ns = ns_get_cb(private_data);
    if (!ns)
        return -ENOENT;
}

Since the callback sets info->ifindex before maybe_get_net() fails,
bpf_prog_offload_info_fill() takes this path:

    if (res) {
        if (!info->ifindex)  /* false, ifindex was set */
            return -ENODEV;
        return res;          /* returns -ENOENT */
    }

Daniel Borkmann also noted this in his review of v1:
https://lore.kernel.org/bpf/1eccd7fc-8a1c-4499-ae81-a63fc8d0ff26@iogearbox.net/


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/24166717291
Re: [PATCH bpf] bpf: Fix use-after-free in offloaded map/prog info fill
Posted by Daniel Borkmann 2 months, 1 week ago
On 4/8/26 12:47 PM, Jiayuan Chen wrote:
> When querying info for an offloaded BPF map or program,
> bpf_map_offload_info_fill_ns() and bpf_prog_offload_info_fill_ns()
> obtain the network namespace with get_net(dev_net(offmap->netdev)).
> However, the associated netdev's netns may be racing with teardown
> during netns destruction. If the netns refcount has already reached 0,
> get_net() performs a refcount_t increment on 0, triggering:
> 
>    refcount_t: addition on 0; use-after-free.
> 
> Although rtnl_lock and bpf_devs_lock ensure the netdev pointer remains
> valid, they cannot prevent the netns refcount from reaching zero.
> 
> Fix this by using maybe_get_net() instead of get_net(). maybe_get_net()
> uses refcount_inc_not_zero() and returns NULL if the refcount is already
> zero, which causes ns_get_path_cb() to fail and the caller to return
> -ENODEV -- the correct behavior when the netns is being destroyed.

nit: -ENOENT

> Fixes: 675fc275a3a2d ("bpf: offload: report device information for offloaded programs")
> Fixes: 52775b33bb507 ("bpf: offload: report device information about offloaded maps")
> Reported-by: Yinhao Hu <dddddd@hust.edu.cn>
> Reported-by: Kaiyan Mei <M202472210@hust.edu.cn>
> Reported-by: Dongliang Mu <dzm91@hust.edu.cn>
> Closes: https://lore.kernel.org/bpf/f0aa3678-79c9-47ae-9e8c-02a3d1df160a@hust.edu.cn/
> Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
> ---
>   kernel/bpf/offload.c | 18 ++++++++++++------
>   1 file changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/kernel/bpf/offload.c b/kernel/bpf/offload.c
> index 0ad97d643bf49..d6dc684d0e872 100644
> --- a/kernel/bpf/offload.c
> +++ b/kernel/bpf/offload.c
> @@ -435,9 +435,12 @@ static struct ns_common *bpf_prog_offload_info_fill_ns(void *private_data)
>   
>   	if (aux->offload) {
>   		args->info->ifindex = aux->offload->netdev->ifindex;
> -		net = dev_net(aux->offload->netdev);
> -		get_net(net);
> -		ns = &net->ns;
> +		net = maybe_get_net(dev_net(aux->offload->netdev));
> +		if (net) {
> +			ns = &net->ns;
> +		} else {
> +			ns = NULL;
> +		}

nit: ns = net ? &net->ns : NULL;

>   	} else {
>   		args->info->ifindex = 0;
>   		ns = NULL;
> @@ -647,9 +650,12 @@ static struct ns_common *bpf_map_offload_info_fill_ns(void *private_data)
>   
>   	if (args->offmap->netdev) {
>   		args->info->ifindex = args->offmap->netdev->ifindex;
> -		net = dev_net(args->offmap->netdev);
> -		get_net(net);
> -		ns = &net->ns;
> +		net = maybe_get_net(dev_net(args->offmap->netdev));
> +		if (net) {
> +			ns = &net->ns;
> +		} else {
> +			ns = NULL;
> +		}

ditto, otherwise looks good to me

>   	} else {
>   		args->info->ifindex = 0;
>   		ns = NULL;
Re: [PATCH bpf] bpf: Fix use-after-free in offloaded map/prog info fill
Posted by Jiayuan Chen 2 months, 1 week ago
On 4/9/26 5:49 AM, Daniel Borkmann wrote:
> On 4/8/26 12:47 PM, Jiayuan Chen wrote:
>> When querying info for an offloaded BPF map or program,
>>
[...]
>> zero, which causes ns_get_path_cb() to fail and the caller to return
>> -ENODEV -- the correct behavior when the netns is being destroyed.
>
> nit: -ENOENT
>
>> Fixes: 675fc275a3a2d ("bpf: offload: report device information for 
>> offloaded programs")
>> Fixes: 52775b33bb507 ("bpf: offload: report device information about 
>> offloaded maps")
>> Reported-by: Yinhao Hu <dddddd@hust.edu.cn>
>> [...]
>> +        net = maybe_get_net(dev_net(aux->offload->netdev));
>> +        if (net) {
>> +            ns = &net->ns;
>> +        } else {
>> +            ns = NULL;
>> +        }
>
> nit: ns = net ? &net->ns : NULL;
>
>>
[...]
>> +        }
>
> ditto, otherwise looks good to me
>
>>       } else {
>>           args->info->ifindex = 0;
>>           ns = NULL;
>

  Thanks for the review!

  Both nits are valid -- will fix the commit message to say -ENOENT and
  simplify to ternary in the next version.