[PATCH 6.1.y] bpf: avoid holding freeze_mutex during mmap operation

[PATCH 6.1.y] bpf: avoid holding freeze_mutex during mmap operation

[David Sauerwein]

From: Andrii Nakryiko <andrii@kernel.org>

[ Upstream commit bc27c52eea189e8f7492d40739b7746d67b65beb ]

We use map->freeze_mutex to prevent races between map_freeze() and
memory mapping BPF map contents with writable permissions. The way we
naively do this means we'll hold freeze_mutex for entire duration of all
the mm and VMA manipulations, which is completely unnecessary. This can
potentially also lead to deadlocks, as reported by syzbot in [0].

So, instead, hold freeze_mutex only during writeability checks, bump
(proactively) "write active" count for the map, unlock the mutex and
proceed with mmap logic. And only if something went wrong during mmap
logic, then undo that "write active" counter increment.

  [0] https://lore.kernel.org/bpf/678dcbc9.050a0220.303755.0066.GAE@google.com/

Fixes: fc9702273e2e ("bpf: Add mmap() support for BPF_MAP_TYPE_ARRAY")
Reported-by: syzbot+4dc041c686b7c816a71e@syzkaller.appspotmail.com
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/r/20250129012246.1515826-2-andrii@kernel.org
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
Signed-off-by: David Sauerwein <dssauerw@amazon.de>
---
 kernel/bpf/syscall.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 7a4004f09bae..27fdf1b2fc46 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -813,7 +813,7 @@ static const struct vm_operations_struct bpf_map_default_vmops = {
 static int bpf_map_mmap(struct file *filp, struct vm_area_struct *vma)
 {
 	struct bpf_map *map = filp->private_data;
-	int err;
+	int err = 0;
 
 	if (!map->ops->map_mmap || map_value_has_spin_lock(map) ||
 	    map_value_has_timer(map) || map_value_has_kptrs(map))
@@ -838,7 +838,12 @@ static int bpf_map_mmap(struct file *filp, struct vm_area_struct *vma)
 			err = -EACCES;
 			goto out;
 		}
+		bpf_map_write_active_inc(map);
 	}
+out:
+	mutex_unlock(&map->freeze_mutex);
+	if (err)
+		return err;
 
 	/* set default open/close callbacks */
 	vma->vm_ops = &bpf_map_default_vmops;
@@ -849,13 +854,11 @@ static int bpf_map_mmap(struct file *filp, struct vm_area_struct *vma)
 		vma->vm_flags &= ~VM_MAYWRITE;
 
 	err = map->ops->map_mmap(map, vma);
-	if (err)
-		goto out;
+	if (err) {
+		if (vma->vm_flags & VM_WRITE)
+			bpf_map_write_active_dec(map);
+	}
 
-	if (vma->vm_flags & VM_MAYWRITE)
-		bpf_map_write_active_inc(map);
-out:
-	mutex_unlock(&map->freeze_mutex);
 	return err;
 }
 
-- 
2.47.1

Re: [PATCH 6.1.y] bpf: avoid holding freeze_mutex during mmap operation

[Sasha Levin]

[ Sasha's backport helper bot ]

Hi,

✅ All tests passed successfully. No issues detected.
No action required from the submitter.

The upstream commit SHA1 provided is correct: bc27c52eea189e8f7492d40739b7746d67b65beb

WARNING: Author mismatch between patch and upstream commit:
Backport author: David Sauerwein<dssauerw@amazon.de>
Commit author: Andrii Nakryiko<andrii@kernel.org>

Status in newer kernel trees:
6.14.y | Present (exact SHA1)
6.13.y | Present (different SHA1: 271e49f8a58e)
6.12.y | Present (different SHA1: d95607a5f2f9)
6.6.y | Present (different SHA1: 29cfda62ab4d)

Note: The patch differs from the upstream commit:
---
1:  bc27c52eea189 ! 1:  dd13352656776 bpf: avoid holding freeze_mutex during mmap operation
    @@ Metadata
      ## Commit message ##
         bpf: avoid holding freeze_mutex during mmap operation
     
    +    [ Upstream commit bc27c52eea189e8f7492d40739b7746d67b65beb ]
    +
         We use map->freeze_mutex to prevent races between map_freeze() and
         memory mapping BPF map contents with writable permissions. The way we
         naively do this means we'll hold freeze_mutex for entire duration of all
    @@ Commit message
         Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
         Link: https://lore.kernel.org/r/20250129012246.1515826-2-andrii@kernel.org
         Signed-off-by: Alexei Starovoitov <ast@kernel.org>
    +    Signed-off-by: Sasha Levin <sashal@kernel.org>
    +    Signed-off-by: David Sauerwein <dssauerw@amazon.de>
     
      ## kernel/bpf/syscall.c ##
     @@ kernel/bpf/syscall.c: static const struct vm_operations_struct bpf_map_default_vmops = {
    @@ kernel/bpf/syscall.c: static const struct vm_operations_struct bpf_map_default_v
     -	int err;
     +	int err = 0;
      
    - 	if (!map->ops->map_mmap || !IS_ERR_OR_NULL(map->record))
    - 		return -ENOTSUPP;
    + 	if (!map->ops->map_mmap || map_value_has_spin_lock(map) ||
    + 	    map_value_has_timer(map) || map_value_has_kptrs(map))
     @@ kernel/bpf/syscall.c: static int bpf_map_mmap(struct file *filp, struct vm_area_struct *vma)
      			err = -EACCES;
      			goto out;
    @@ kernel/bpf/syscall.c: static int bpf_map_mmap(struct file *filp, struct vm_area_
      	/* set default open/close callbacks */
      	vma->vm_ops = &bpf_map_default_vmops;
     @@ kernel/bpf/syscall.c: static int bpf_map_mmap(struct file *filp, struct vm_area_struct *vma)
    - 		vm_flags_clear(vma, VM_MAYWRITE);
    + 		vma->vm_flags &= ~VM_MAYWRITE;
      
      	err = map->ops->map_mmap(map, vma);
     -	if (err)
    @@ kernel/bpf/syscall.c: static int bpf_map_mmap(struct file *filp, struct vm_area_
     +			bpf_map_write_active_dec(map);
     +	}
      
    --	if (vma->vm_flags & VM_WRITE)
    +-	if (vma->vm_flags & VM_MAYWRITE)
     -		bpf_map_write_active_inc(map);
     -out:
     -	mutex_unlock(&map->freeze_mutex);
---

Results of testing on various branches:

| Branch                    | Patch Apply | Build Test |
|---------------------------|-------------|------------|
| stable/linux-6.1.y        |  Success    |  Success   |