* [PATCH v4 04/10] mm/mremap: cleanup post-processing stage of mremap
From: Lorenzo Stoakes @ 2025-07-17 16:55 UTC (permalink / raw)
To: Andrew Morton
Cc: Peter Xu, Alexander Viro, Christian Brauner, Jan Kara,
Liam R . Howlett, Vlastimil Babka, Jann Horn, Pedro Falcato,
Rik van Riel, linux-mm, linux-fsdevel, linux-kernel, linux-api
In-Reply-To: <cover.1752770784.git.lorenzo.stoakes@oracle.com>
Separate out the uffd bits so it clear's what's happening.
Don't bother setting vrm->mmap_locked after unlocking, because after this
we are done anyway.
The only time we drop the mmap lock is on VMA shrink, at which point
vrm->new_len will be < vrm->old_len and the operation will not be
performed anyway, so move this code out of the if (vrm->mmap_locked)
block.
All addresses returned by mremap() are page-aligned, so the
offset_in_page() check on ret seems only to be incorrectly trying to
detect whether an error occurred - explicitly check for this.
No functional change intended.
Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
---
mm/mremap.c | 22 +++++++++++++---------
1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/mm/mremap.c b/mm/mremap.c
index 60eb0ac8634b..53447761e55d 100644
--- a/mm/mremap.c
+++ b/mm/mremap.c
@@ -1729,6 +1729,15 @@ static int check_prep_vma(struct vma_remap_struct *vrm)
return 0;
}
+static void notify_uffd(struct vma_remap_struct *vrm, unsigned long to)
+{
+ struct mm_struct *mm = current->mm;
+
+ userfaultfd_unmap_complete(mm, vrm->uf_unmap_early);
+ mremap_userfaultfd_complete(vrm->uf, vrm->addr, to, vrm->old_len);
+ userfaultfd_unmap_complete(mm, vrm->uf_unmap);
+}
+
static unsigned long do_mremap(struct vma_remap_struct *vrm)
{
struct mm_struct *mm = current->mm;
@@ -1754,18 +1763,13 @@ static unsigned long do_mremap(struct vma_remap_struct *vrm)
res = vrm_implies_new_addr(vrm) ? mremap_to(vrm) : mremap_at(vrm);
out:
- if (vrm->mmap_locked) {
+ if (vrm->mmap_locked)
mmap_write_unlock(mm);
- vrm->mmap_locked = false;
-
- if (!offset_in_page(res) && vrm->mlocked && vrm->new_len > vrm->old_len)
- mm_populate(vrm->new_addr + vrm->old_len, vrm->delta);
- }
- userfaultfd_unmap_complete(mm, vrm->uf_unmap_early);
- mremap_userfaultfd_complete(vrm->uf, vrm->addr, res, vrm->old_len);
- userfaultfd_unmap_complete(mm, vrm->uf_unmap);
+ if (!IS_ERR_VALUE(res) && vrm->mlocked && vrm->new_len > vrm->old_len)
+ mm_populate(vrm->new_addr + vrm->old_len, vrm->delta);
+ notify_uffd(vrm, res);
return res;
}
--
2.50.1
^ permalink raw reply related
* [PATCH v4 06/10] mm/mremap: check remap conditions earlier
From: Lorenzo Stoakes @ 2025-07-17 16:55 UTC (permalink / raw)
To: Andrew Morton
Cc: Peter Xu, Alexander Viro, Christian Brauner, Jan Kara,
Liam R . Howlett, Vlastimil Babka, Jann Horn, Pedro Falcato,
Rik van Riel, linux-mm, linux-fsdevel, linux-kernel, linux-api
In-Reply-To: <cover.1752770784.git.lorenzo.stoakes@oracle.com>
When we expand or move a VMA, this requires a number of additional checks
to be performed.
Make it really obvious under what circumstances these checks must be
performed and aggregate all the checks in one place by invoking this in
check_prep_vma().
We have to adjust the checks to account for shrink + move operations by
checking new_len <= old_len rather than new_len == old_len.
No functional change intended.
Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
---
mm/mremap.c | 28 +++++++++++++++++++---------
1 file changed, 19 insertions(+), 9 deletions(-)
diff --git a/mm/mremap.c b/mm/mremap.c
index db7e773d0884..20844fb91755 100644
--- a/mm/mremap.c
+++ b/mm/mremap.c
@@ -1343,7 +1343,7 @@ static int remap_is_valid(struct vma_remap_struct *vrm)
if (old_len > vma->vm_end - addr)
return -EFAULT;
- if (new_len == old_len)
+ if (new_len <= old_len)
return 0;
/* Need to be careful about a growing mapping */
@@ -1443,10 +1443,6 @@ static unsigned long mremap_to(struct vma_remap_struct *vrm)
vrm->old_len = vrm->new_len;
}
- err = remap_is_valid(vrm);
- if (err)
- return err;
-
/* MREMAP_DONTUNMAP expands by old_len since old_len == new_len */
if (vrm->flags & MREMAP_DONTUNMAP) {
vm_flags_t vm_flags = vrm->vma->vm_flags;
@@ -1635,10 +1631,6 @@ static unsigned long expand_vma(struct vma_remap_struct *vrm)
{
unsigned long err;
- err = remap_is_valid(vrm);
- if (err)
- return err;
-
/*
* [addr, old_len) spans precisely to the end of the VMA, so try to
* expand it in-place.
@@ -1705,6 +1697,21 @@ static unsigned long mremap_at(struct vma_remap_struct *vrm)
return -EINVAL;
}
+/*
+ * Will this operation result in the VMA being expanded or moved and thus need
+ * to map a new portion of virtual address space?
+ */
+static bool vrm_will_map_new(struct vma_remap_struct *vrm)
+{
+ if (vrm->remap_type == MREMAP_EXPAND)
+ return true;
+
+ if (vrm_implies_new_addr(vrm))
+ return true;
+
+ return false;
+}
+
static int check_prep_vma(struct vma_remap_struct *vrm)
{
struct vm_area_struct *vma = vrm->vma;
@@ -1726,6 +1733,9 @@ static int check_prep_vma(struct vma_remap_struct *vrm)
if (!vrm_implies_new_addr(vrm))
vrm->new_addr = vrm->addr;
+ if (vrm_will_map_new(vrm))
+ return remap_is_valid(vrm);
+
return 0;
}
--
2.50.1
^ permalink raw reply related
* [PATCH v4 07/10] mm/mremap: move remap_is_valid() into check_prep_vma()
From: Lorenzo Stoakes @ 2025-07-17 16:55 UTC (permalink / raw)
To: Andrew Morton
Cc: Peter Xu, Alexander Viro, Christian Brauner, Jan Kara,
Liam R . Howlett, Vlastimil Babka, Jann Horn, Pedro Falcato,
Rik van Riel, linux-mm, linux-fsdevel, linux-kernel, linux-api
In-Reply-To: <cover.1752770784.git.lorenzo.stoakes@oracle.com>
Group parameter check logic together, moving check_mremap_params() next to
it.
This puts all such checks into a single place, and invokes them early so
we can simply bail out as soon as we are aware that a condition is not
met.
No functional change intended.
Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
---
mm/mremap.c | 273 +++++++++++++++++++++++++---------------------------
1 file changed, 131 insertions(+), 142 deletions(-)
diff --git a/mm/mremap.c b/mm/mremap.c
index 20844fb91755..3678f21c2c36 100644
--- a/mm/mremap.c
+++ b/mm/mremap.c
@@ -1306,64 +1306,6 @@ static unsigned long move_vma(struct vma_remap_struct *vrm)
return err ? (unsigned long)err : vrm->new_addr;
}
-/*
- * remap_is_valid() - Ensure the VMA can be moved or resized to the new length,
- * at the given address.
- *
- * Return 0 on success, error otherwise.
- */
-static int remap_is_valid(struct vma_remap_struct *vrm)
-{
- struct mm_struct *mm = current->mm;
- struct vm_area_struct *vma = vrm->vma;
- unsigned long addr = vrm->addr;
- unsigned long old_len = vrm->old_len;
- unsigned long new_len = vrm->new_len;
- unsigned long pgoff;
-
- /*
- * !old_len is a special case where an attempt is made to 'duplicate'
- * a mapping. This makes no sense for private mappings as it will
- * instead create a fresh/new mapping unrelated to the original. This
- * is contrary to the basic idea of mremap which creates new mappings
- * based on the original. There are no known use cases for this
- * behavior. As a result, fail such attempts.
- */
- if (!old_len && !(vma->vm_flags & (VM_SHARED | VM_MAYSHARE))) {
- pr_warn_once("%s (%d): attempted to duplicate a private mapping with mremap. This is not supported.\n",
- current->comm, current->pid);
- return -EINVAL;
- }
-
- if ((vrm->flags & MREMAP_DONTUNMAP) &&
- (vma->vm_flags & (VM_DONTEXPAND | VM_PFNMAP)))
- return -EINVAL;
-
- /* We can't remap across vm area boundaries */
- if (old_len > vma->vm_end - addr)
- return -EFAULT;
-
- if (new_len <= old_len)
- return 0;
-
- /* Need to be careful about a growing mapping */
- pgoff = (addr - vma->vm_start) >> PAGE_SHIFT;
- pgoff += vma->vm_pgoff;
- if (pgoff + (new_len >> PAGE_SHIFT) < pgoff)
- return -EINVAL;
-
- if (vma->vm_flags & (VM_DONTEXPAND | VM_PFNMAP))
- return -EFAULT;
-
- if (!mlock_future_ok(mm, vma->vm_flags, vrm->delta))
- return -EAGAIN;
-
- if (!may_expand_vm(mm, vma->vm_flags, vrm->delta >> PAGE_SHIFT))
- return -ENOMEM;
-
- return 0;
-}
-
/*
* The user has requested that the VMA be shrunk (i.e., old_len > new_len), so
* execute this, optionally dropping the mmap lock when we do so.
@@ -1490,77 +1432,6 @@ static bool vrm_can_expand_in_place(struct vma_remap_struct *vrm)
return true;
}
-/*
- * Are the parameters passed to mremap() valid? If so return 0, otherwise return
- * error.
- */
-static unsigned long check_mremap_params(struct vma_remap_struct *vrm)
-
-{
- unsigned long addr = vrm->addr;
- unsigned long flags = vrm->flags;
-
- /* Ensure no unexpected flag values. */
- if (flags & ~(MREMAP_FIXED | MREMAP_MAYMOVE | MREMAP_DONTUNMAP))
- return -EINVAL;
-
- /* Start address must be page-aligned. */
- if (offset_in_page(addr))
- return -EINVAL;
-
- /*
- * We allow a zero old-len as a special case
- * for DOS-emu "duplicate shm area" thing. But
- * a zero new-len is nonsensical.
- */
- if (!vrm->new_len)
- return -EINVAL;
-
- /* Is the new length or address silly? */
- if (vrm->new_len > TASK_SIZE ||
- vrm->new_addr > TASK_SIZE - vrm->new_len)
- return -EINVAL;
-
- /* Remainder of checks are for cases with specific new_addr. */
- if (!vrm_implies_new_addr(vrm))
- return 0;
-
- /* The new address must be page-aligned. */
- if (offset_in_page(vrm->new_addr))
- return -EINVAL;
-
- /* A fixed address implies a move. */
- if (!(flags & MREMAP_MAYMOVE))
- return -EINVAL;
-
- /* MREMAP_DONTUNMAP does not allow resizing in the process. */
- if (flags & MREMAP_DONTUNMAP && vrm->old_len != vrm->new_len)
- return -EINVAL;
-
- /* Target VMA must not overlap source VMA. */
- if (vrm_overlaps(vrm))
- return -EINVAL;
-
- /*
- * move_vma() need us to stay 4 maps below the threshold, otherwise
- * it will bail out at the very beginning.
- * That is a problem if we have already unmaped the regions here
- * (new_addr, and old_addr), because userspace will not know the
- * state of the vma's after it gets -ENOMEM.
- * So, to avoid such scenario we can pre-compute if the whole
- * operation has high chances to success map-wise.
- * Worst-scenario case is when both vma's (new_addr and old_addr) get
- * split in 3 before unmapping it.
- * That means 2 more maps (1 for each) to the ones we already hold.
- * Check whether current map count plus 2 still leads us to 4 maps below
- * the threshold, otherwise return -ENOMEM here to be more safe.
- */
- if ((current->mm->map_count + 2) >= sysctl_max_map_count - 3)
- return -ENOMEM;
-
- return 0;
-}
-
/*
* We know we can expand the VMA in-place by delta pages, so do so.
*
@@ -1712,9 +1583,26 @@ static bool vrm_will_map_new(struct vma_remap_struct *vrm)
return false;
}
+static void notify_uffd(struct vma_remap_struct *vrm, bool failed)
+{
+ struct mm_struct *mm = current->mm;
+
+ /* Regardless of success/failure, we always notify of any unmaps. */
+ userfaultfd_unmap_complete(mm, vrm->uf_unmap_early);
+ if (failed)
+ mremap_userfaultfd_fail(vrm->uf);
+ else
+ mremap_userfaultfd_complete(vrm->uf, vrm->addr,
+ vrm->new_addr, vrm->old_len);
+ userfaultfd_unmap_complete(mm, vrm->uf_unmap);
+}
+
static int check_prep_vma(struct vma_remap_struct *vrm)
{
struct vm_area_struct *vma = vrm->vma;
+ struct mm_struct *mm = current->mm;
+ unsigned long addr = vrm->addr;
+ unsigned long old_len, new_len, pgoff;
if (!vma)
return -EFAULT;
@@ -1731,26 +1619,127 @@ static int check_prep_vma(struct vma_remap_struct *vrm)
vrm->remap_type = vrm_remap_type(vrm);
/* For convenience, we set new_addr even if VMA won't move. */
if (!vrm_implies_new_addr(vrm))
- vrm->new_addr = vrm->addr;
+ vrm->new_addr = addr;
+
+ /* Below only meaningful if we expand or move a VMA. */
+ if (!vrm_will_map_new(vrm))
+ return 0;
- if (vrm_will_map_new(vrm))
- return remap_is_valid(vrm);
+ old_len = vrm->old_len;
+ new_len = vrm->new_len;
+
+ /*
+ * !old_len is a special case where an attempt is made to 'duplicate'
+ * a mapping. This makes no sense for private mappings as it will
+ * instead create a fresh/new mapping unrelated to the original. This
+ * is contrary to the basic idea of mremap which creates new mappings
+ * based on the original. There are no known use cases for this
+ * behavior. As a result, fail such attempts.
+ */
+ if (!old_len && !(vma->vm_flags & (VM_SHARED | VM_MAYSHARE))) {
+ pr_warn_once("%s (%d): attempted to duplicate a private mapping with mremap. This is not supported.\n",
+ current->comm, current->pid);
+ return -EINVAL;
+ }
+
+ if ((vrm->flags & MREMAP_DONTUNMAP) &&
+ (vma->vm_flags & (VM_DONTEXPAND | VM_PFNMAP)))
+ return -EINVAL;
+
+ /* We can't remap across vm area boundaries */
+ if (old_len > vma->vm_end - addr)
+ return -EFAULT;
+
+ if (new_len <= old_len)
+ return 0;
+
+ /* Need to be careful about a growing mapping */
+ pgoff = (addr - vma->vm_start) >> PAGE_SHIFT;
+ pgoff += vma->vm_pgoff;
+ if (pgoff + (new_len >> PAGE_SHIFT) < pgoff)
+ return -EINVAL;
+
+ if (vma->vm_flags & (VM_DONTEXPAND | VM_PFNMAP))
+ return -EFAULT;
+
+ if (!mlock_future_ok(mm, vma->vm_flags, vrm->delta))
+ return -EAGAIN;
+
+ if (!may_expand_vm(mm, vma->vm_flags, vrm->delta >> PAGE_SHIFT))
+ return -ENOMEM;
return 0;
}
-static void notify_uffd(struct vma_remap_struct *vrm, bool failed)
+/*
+ * Are the parameters passed to mremap() valid? If so return 0, otherwise return
+ * error.
+ */
+static unsigned long check_mremap_params(struct vma_remap_struct *vrm)
+
{
- struct mm_struct *mm = current->mm;
+ unsigned long addr = vrm->addr;
+ unsigned long flags = vrm->flags;
- /* Regardless of success/failure, we always notify of any unmaps. */
- userfaultfd_unmap_complete(mm, vrm->uf_unmap_early);
- if (failed)
- mremap_userfaultfd_fail(vrm->uf);
- else
- mremap_userfaultfd_complete(vrm->uf, vrm->addr,
- vrm->new_addr, vrm->old_len);
- userfaultfd_unmap_complete(mm, vrm->uf_unmap);
+ /* Ensure no unexpected flag values. */
+ if (flags & ~(MREMAP_FIXED | MREMAP_MAYMOVE | MREMAP_DONTUNMAP))
+ return -EINVAL;
+
+ /* Start address must be page-aligned. */
+ if (offset_in_page(addr))
+ return -EINVAL;
+
+ /*
+ * We allow a zero old-len as a special case
+ * for DOS-emu "duplicate shm area" thing. But
+ * a zero new-len is nonsensical.
+ */
+ if (!vrm->new_len)
+ return -EINVAL;
+
+ /* Is the new length or address silly? */
+ if (vrm->new_len > TASK_SIZE ||
+ vrm->new_addr > TASK_SIZE - vrm->new_len)
+ return -EINVAL;
+
+ /* Remainder of checks are for cases with specific new_addr. */
+ if (!vrm_implies_new_addr(vrm))
+ return 0;
+
+ /* The new address must be page-aligned. */
+ if (offset_in_page(vrm->new_addr))
+ return -EINVAL;
+
+ /* A fixed address implies a move. */
+ if (!(flags & MREMAP_MAYMOVE))
+ return -EINVAL;
+
+ /* MREMAP_DONTUNMAP does not allow resizing in the process. */
+ if (flags & MREMAP_DONTUNMAP && vrm->old_len != vrm->new_len)
+ return -EINVAL;
+
+ /* Target VMA must not overlap source VMA. */
+ if (vrm_overlaps(vrm))
+ return -EINVAL;
+
+ /*
+ * move_vma() need us to stay 4 maps below the threshold, otherwise
+ * it will bail out at the very beginning.
+ * That is a problem if we have already unmaped the regions here
+ * (new_addr, and old_addr), because userspace will not know the
+ * state of the vma's after it gets -ENOMEM.
+ * So, to avoid such scenario we can pre-compute if the whole
+ * operation has high chances to success map-wise.
+ * Worst-scenario case is when both vma's (new_addr and old_addr) get
+ * split in 3 before unmapping it.
+ * That means 2 more maps (1 for each) to the ones we already hold.
+ * Check whether current map count plus 2 still leads us to 4 maps below
+ * the threshold, otherwise return -ENOMEM here to be more safe.
+ */
+ if ((current->mm->map_count + 2) >= sysctl_max_map_count - 3)
+ return -ENOMEM;
+
+ return 0;
}
static unsigned long do_mremap(struct vma_remap_struct *vrm)
--
2.50.1
^ permalink raw reply related
* [PATCH v4 08/10] mm/mremap: clean up mlock populate behaviour
From: Lorenzo Stoakes @ 2025-07-17 16:55 UTC (permalink / raw)
To: Andrew Morton
Cc: Peter Xu, Alexander Viro, Christian Brauner, Jan Kara,
Liam R . Howlett, Vlastimil Babka, Jann Horn, Pedro Falcato,
Rik van Riel, linux-mm, linux-fsdevel, linux-kernel, linux-api
In-Reply-To: <cover.1752770784.git.lorenzo.stoakes@oracle.com>
When an mlock()'d VMA is expanded, we need to populate the expanded region
to maintain the contract that all mlock()'d memory is present (albeit -
with some period after mmap unlock where the expanded part of the mapping
remains unfaulted).
The current implementation is very unclear, so make it absolutely explicit
under what circumstances we do this.
Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
---
mm/mremap.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/mm/mremap.c b/mm/mremap.c
index 3678f21c2c36..28e776cddc08 100644
--- a/mm/mremap.c
+++ b/mm/mremap.c
@@ -65,7 +65,7 @@ struct vma_remap_struct {
/* Internal state, determined in do_mremap(). */
unsigned long delta; /* Absolute delta of old_len,new_len. */
- bool mlocked; /* Was the VMA mlock()'d? */
+ bool populate_expand; /* mlock()'d expanded, must populate. */
enum mremap_type remap_type; /* expand, shrink, etc. */
bool mmap_locked; /* Is mm currently write-locked? */
unsigned long charged; /* If VM_ACCOUNT, # pages to account. */
@@ -1010,10 +1010,8 @@ static void vrm_stat_account(struct vma_remap_struct *vrm,
struct vm_area_struct *vma = vrm->vma;
vm_stat_account(mm, vma->vm_flags, pages);
- if (vma->vm_flags & VM_LOCKED) {
+ if (vma->vm_flags & VM_LOCKED)
mm->locked_vm += pages;
- vrm->mlocked = true;
- }
}
/*
@@ -1653,6 +1651,10 @@ static int check_prep_vma(struct vma_remap_struct *vrm)
if (new_len <= old_len)
return 0;
+ /* We are expanding and the VMA is mlock()'d so we need to populate. */
+ if (vma->vm_flags & VM_LOCKED)
+ vrm->populate_expand = true;
+
/* Need to be careful about a growing mapping */
pgoff = (addr - vma->vm_start) >> PAGE_SHIFT;
pgoff += vma->vm_pgoff;
@@ -1773,7 +1775,8 @@ static unsigned long do_mremap(struct vma_remap_struct *vrm)
if (vrm->mmap_locked)
mmap_write_unlock(mm);
- if (!failed && vrm->mlocked && vrm->new_len > vrm->old_len)
+ /* VMA mlock'd + was expanded, so populated expanded region. */
+ if (!failed && vrm->populate_expand)
mm_populate(vrm->new_addr + vrm->old_len, vrm->delta);
notify_uffd(vrm, failed);
--
2.50.1
^ permalink raw reply related
* [PATCH v4 09/10] mm/mremap: permit mremap() move of multiple VMAs
From: Lorenzo Stoakes @ 2025-07-17 16:55 UTC (permalink / raw)
To: Andrew Morton
Cc: Peter Xu, Alexander Viro, Christian Brauner, Jan Kara,
Liam R . Howlett, Vlastimil Babka, Jann Horn, Pedro Falcato,
Rik van Riel, linux-mm, linux-fsdevel, linux-kernel, linux-api
In-Reply-To: <cover.1752770784.git.lorenzo.stoakes@oracle.com>
Historically we've made it a uAPI requirement that mremap() may only
operate on a single VMA at a time.
For instances where VMAs need to be resized, this makes sense, as it
becomes very difficult to determine what a user actually wants should they
indicate a desire to expand or shrink the size of multiple VMAs (truncate?
Adjust sizes individually? Some other strategy?).
However, in instances where a user is moving VMAs, it is restrictive to
disallow this.
This is especially the case when anonymous mapping remap may or may not be
mergeable depending on whether VMAs have or have not been faulted due to
anon_vma assignment and folio index alignment with vma->vm_pgoff.
Often this can result in surprising impact where a moved region is
faulted, then moved back and a user fails to observe a merge from
otherwise compatible, adjacent VMAs.
This change allows such cases to work without the user having to be
cognizant of whether a prior mremap() move or other VMA operations has
resulted in VMA fragmentation.
We only permit this for mremap() operations that do NOT change the size of
the VMA and DO specify MREMAP_MAYMOVE | MREMAP_FIXED.
Should no VMA exist in the range, -EFAULT is returned as usual.
If a VMA move spans a single VMA - then there is no functional change.
Otherwise, we place additional requirements upon VMAs:
* They must not have a userfaultfd context associated with them - this
requires dropping the lock to notify users, and we want to perform the
operation with the mmap write lock held throughout.
* If file-backed, they cannot have a custom get_unmapped_area handler -
this might result in MREMAP_FIXED not being honoured, which could result
in unexpected positioning of VMAs in the moved region.
There may be gaps in the range of VMAs that are moved:
X Y X Y
<---> <-> <---> <->
|-------| |-----| |-----| |-------| |-----| |-----|
| A | | B | | C | ---> | A' | | B' | | C' |
|-------| |-----| |-----| |-------| |-----| |-----|
addr new_addr
The move will preserve the gaps between each VMA.
Note that any failures encountered will result in a partial move. Since
an mremap() can fail at any time, this might result in only some of the
VMAs being moved.
Note that failures are very rare and typically require an out of a memory
condition or a mapping limit condition to be hit, assuming the VMAs being
moved are valid.
We don't try to assess ahead of time whether VMAs are valid according to
the multi VMA rules, as it would be rather unusual for a user to mix
uffd-enabled VMAs and/or VMAs which map unusual driver mappings that
specify custom get_unmapped_area() handlers in an aggregate operation.
So we optimise for the far, far more likely case of the operation being
entirely permissible.
In the case of the move of a single VMA, the above conditions are
permitted. This makes the behaviour identical for a single VMA as before.
Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
---
mm/mremap.c | 159 +++++++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 152 insertions(+), 7 deletions(-)
diff --git a/mm/mremap.c b/mm/mremap.c
index 28e776cddc08..e67cba4e6fc0 100644
--- a/mm/mremap.c
+++ b/mm/mremap.c
@@ -69,6 +69,7 @@ struct vma_remap_struct {
enum mremap_type remap_type; /* expand, shrink, etc. */
bool mmap_locked; /* Is mm currently write-locked? */
unsigned long charged; /* If VM_ACCOUNT, # pages to account. */
+ bool vmi_needs_invalidate; /* Is the VMA iterator invalidated? */
};
static pud_t *get_old_pud(struct mm_struct *mm, unsigned long addr)
@@ -1111,6 +1112,7 @@ static void unmap_source_vma(struct vma_remap_struct *vrm)
err = do_vmi_munmap(&vmi, mm, addr, len, vrm->uf_unmap, /* unlock= */false);
vrm->vma = NULL; /* Invalidated. */
+ vrm->vmi_needs_invalidate = true;
if (err) {
/* OOM: unable to split vma, just get accounts right */
vm_acct_memory(len >> PAGE_SHIFT);
@@ -1186,6 +1188,10 @@ static int copy_vma_and_data(struct vma_remap_struct *vrm,
*new_vma_ptr = NULL;
return -ENOMEM;
}
+ /* By merging, we may have invalidated any iterator in use. */
+ if (vma != vrm->vma)
+ vrm->vmi_needs_invalidate = true;
+
vrm->vma = vma;
pmc.old = vma;
pmc.new = new_vma;
@@ -1362,6 +1368,7 @@ static unsigned long mremap_to(struct vma_remap_struct *vrm)
err = do_munmap(mm, vrm->new_addr, vrm->new_len,
vrm->uf_unmap_early);
vrm->vma = NULL; /* Invalidated. */
+ vrm->vmi_needs_invalidate = true;
if (err)
return err;
@@ -1581,6 +1588,18 @@ static bool vrm_will_map_new(struct vma_remap_struct *vrm)
return false;
}
+/* Does this remap ONLY move mappings? */
+static bool vrm_move_only(struct vma_remap_struct *vrm)
+{
+ if (!(vrm->flags & MREMAP_FIXED))
+ return false;
+
+ if (vrm->old_len != vrm->new_len)
+ return false;
+
+ return true;
+}
+
static void notify_uffd(struct vma_remap_struct *vrm, bool failed)
{
struct mm_struct *mm = current->mm;
@@ -1595,6 +1614,32 @@ static void notify_uffd(struct vma_remap_struct *vrm, bool failed)
userfaultfd_unmap_complete(mm, vrm->uf_unmap);
}
+static bool vma_multi_allowed(struct vm_area_struct *vma)
+{
+ struct file *file;
+
+ /*
+ * We can't support moving multiple uffd VMAs as notify requires
+ * mmap lock to be dropped.
+ */
+ if (userfaultfd_armed(vma))
+ return false;
+
+ /*
+ * Custom get unmapped area might result in MREMAP_FIXED not
+ * being obeyed.
+ */
+ file = vma->vm_file;
+ if (file && !vma_is_shmem(vma) && !is_vm_hugetlb_page(vma)) {
+ const struct file_operations *fop = file->f_op;
+
+ if (fop->get_unmapped_area)
+ return false;
+ }
+
+ return true;
+}
+
static int check_prep_vma(struct vma_remap_struct *vrm)
{
struct vm_area_struct *vma = vrm->vma;
@@ -1644,7 +1689,19 @@ static int check_prep_vma(struct vma_remap_struct *vrm)
(vma->vm_flags & (VM_DONTEXPAND | VM_PFNMAP)))
return -EINVAL;
- /* We can't remap across vm area boundaries */
+ /*
+ * We can't remap across the end of VMAs, as another VMA may be
+ * adjacent:
+ *
+ * addr vma->vm_end
+ * |-----.----------|
+ * | . |
+ * |-----.----------|
+ * .<--------->xxx>
+ * old_len
+ *
+ * We also require that vma->vm_start <= addr < vma->vm_end.
+ */
if (old_len > vma->vm_end - addr)
return -EFAULT;
@@ -1744,6 +1801,90 @@ static unsigned long check_mremap_params(struct vma_remap_struct *vrm)
return 0;
}
+static unsigned long remap_move(struct vma_remap_struct *vrm)
+{
+ struct vm_area_struct *vma;
+ unsigned long start = vrm->addr;
+ unsigned long end = vrm->addr + vrm->old_len;
+ unsigned long new_addr = vrm->new_addr;
+ bool allowed = true, seen_vma = false;
+ unsigned long target_addr = new_addr;
+ unsigned long res = -EFAULT;
+ unsigned long last_end;
+ VMA_ITERATOR(vmi, current->mm, start);
+
+ /*
+ * When moving VMAs we allow for batched moves across multiple VMAs,
+ * with all VMAs in the input range [addr, addr + old_len) being moved
+ * (and split as necessary).
+ */
+ for_each_vma_range(vmi, vma, end) {
+ /* Account for start, end not aligned with VMA start, end. */
+ unsigned long addr = max(vma->vm_start, start);
+ unsigned long len = min(end, vma->vm_end) - addr;
+ unsigned long offset, res_vma;
+
+ if (!allowed)
+ return -EFAULT;
+
+ /* No gap permitted at the start of the range. */
+ if (!seen_vma && start < vma->vm_start)
+ return -EFAULT;
+
+ /*
+ * To sensibly move multiple VMAs, accounting for the fact that
+ * get_unmapped_area() may align even MAP_FIXED moves, we simply
+ * attempt to move such that the gaps between source VMAs remain
+ * consistent in destination VMAs, e.g.:
+ *
+ * X Y X Y
+ * <---> <-> <---> <->
+ * |-------| |-----| |-----| |-------| |-----| |-----|
+ * | A | | B | | C | ---> | A' | | B' | | C' |
+ * |-------| |-----| |-----| |-------| |-----| |-----|
+ * new_addr
+ *
+ * So we map B' at A'->vm_end + X, and C' at B'->vm_end + Y.
+ */
+ offset = seen_vma ? vma->vm_start - last_end : 0;
+ last_end = vma->vm_end;
+
+ vrm->vma = vma;
+ vrm->addr = addr;
+ vrm->new_addr = target_addr + offset;
+ vrm->old_len = vrm->new_len = len;
+
+ allowed = vma_multi_allowed(vma);
+ if (seen_vma && !allowed)
+ return -EFAULT;
+
+ res_vma = check_prep_vma(vrm);
+ if (!res_vma)
+ res_vma = mremap_to(vrm);
+ if (IS_ERR_VALUE(res_vma))
+ return res_vma;
+
+ if (!seen_vma) {
+ VM_WARN_ON_ONCE(allowed && res_vma != new_addr);
+ res = res_vma;
+ }
+
+ /* mmap lock is only dropped on shrink. */
+ VM_WARN_ON_ONCE(!vrm->mmap_locked);
+ /* This is a move, no expand should occur. */
+ VM_WARN_ON_ONCE(vrm->populate_expand);
+
+ if (vrm->vmi_needs_invalidate) {
+ vma_iter_invalidate(&vmi);
+ vrm->vmi_needs_invalidate = false;
+ }
+ seen_vma = true;
+ target_addr = res_vma + vrm->new_len;
+ }
+
+ return res;
+}
+
static unsigned long do_mremap(struct vma_remap_struct *vrm)
{
struct mm_struct *mm = current->mm;
@@ -1761,13 +1902,17 @@ static unsigned long do_mremap(struct vma_remap_struct *vrm)
return -EINTR;
vrm->mmap_locked = true;
- vrm->vma = vma_lookup(current->mm, vrm->addr);
- res = check_prep_vma(vrm);
- if (res)
- goto out;
+ if (vrm_move_only(vrm)) {
+ res = remap_move(vrm);
+ } else {
+ vrm->vma = vma_lookup(current->mm, vrm->addr);
+ res = check_prep_vma(vrm);
+ if (res)
+ goto out;
- /* Actually execute mremap. */
- res = vrm_implies_new_addr(vrm) ? mremap_to(vrm) : mremap_at(vrm);
+ /* Actually execute mremap. */
+ res = vrm_implies_new_addr(vrm) ? mremap_to(vrm) : mremap_at(vrm);
+ }
out:
failed = IS_ERR_VALUE(res);
--
2.50.1
^ permalink raw reply related
* [PATCH v4 10/10] tools/testing/selftests: extend mremap_test to test multi-VMA mremap
From: Lorenzo Stoakes @ 2025-07-17 16:56 UTC (permalink / raw)
To: Andrew Morton
Cc: Peter Xu, Alexander Viro, Christian Brauner, Jan Kara,
Liam R . Howlett, Vlastimil Babka, Jann Horn, Pedro Falcato,
Rik van Riel, linux-mm, linux-fsdevel, linux-kernel, linux-api
In-Reply-To: <cover.1752770784.git.lorenzo.stoakes@oracle.com>
Now that we have added the ability to move multiple VMAs at once, assert
that this functions correctly, both overwriting VMAs and moving backwards
and forwards with merge and VMA invalidation.
Additionally assert that page tables are correctly propagated by setting
random data and reading it back.
Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
---
tools/testing/selftests/mm/mremap_test.c | 146 ++++++++++++++++++++++-
1 file changed, 145 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/mm/mremap_test.c b/tools/testing/selftests/mm/mremap_test.c
index bb84476a177f..0a49be11e614 100644
--- a/tools/testing/selftests/mm/mremap_test.c
+++ b/tools/testing/selftests/mm/mremap_test.c
@@ -380,6 +380,149 @@ static void mremap_move_within_range(unsigned int pattern_seed, char *rand_addr)
ksft_test_result_fail("%s\n", test_name);
}
+static bool is_multiple_vma_range_ok(unsigned int pattern_seed,
+ char *ptr, unsigned long page_size)
+{
+ int i;
+
+ srand(pattern_seed);
+ for (i = 0; i <= 10; i += 2) {
+ int j;
+ char *buf = &ptr[i * page_size];
+ size_t size = i == 4 ? 2 * page_size : page_size;
+
+ for (j = 0; j < size; j++) {
+ char chr = rand();
+
+ if (chr != buf[j]) {
+ ksft_print_msg("page %d offset %d corrupted, expected %d got %d\n",
+ i, j, chr, buf[j]);
+ return false;
+ }
+ }
+ }
+
+ return true;
+}
+
+static void mremap_move_multiple_vmas(unsigned int pattern_seed,
+ unsigned long page_size)
+{
+ char *test_name = "mremap move multiple vmas";
+ const size_t size = 11 * page_size;
+ bool success = true;
+ char *ptr, *tgt_ptr;
+ int i;
+
+ ptr = mmap(NULL, size, PROT_READ | PROT_WRITE,
+ MAP_PRIVATE | MAP_ANON, -1, 0);
+ if (ptr == MAP_FAILED) {
+ perror("mmap");
+ success = false;
+ goto out;
+ }
+
+ tgt_ptr = mmap(NULL, 2 * size, PROT_READ | PROT_WRITE,
+ MAP_PRIVATE | MAP_ANON, -1, 0);
+ if (tgt_ptr == MAP_FAILED) {
+ perror("mmap");
+ success = false;
+ goto out;
+ }
+ if (munmap(tgt_ptr, 2 * size)) {
+ perror("munmap");
+ success = false;
+ goto out_unmap;
+ }
+
+ /*
+ * Unmap so we end up with:
+ *
+ * 0 2 4 5 6 8 10 offset in buffer
+ * |*| |*| |*****| |*| |*|
+ * |*| |*| |*****| |*| |*|
+ * 0 1 2 3 4 5 6 pattern offset
+ */
+ for (i = 1; i < 10; i += 2) {
+ if (i == 5)
+ continue;
+
+ if (munmap(&ptr[i * page_size], page_size)) {
+ perror("munmap");
+ success = false;
+ goto out_unmap;
+ }
+ }
+
+ srand(pattern_seed);
+
+ /* Set up random patterns. */
+ for (i = 0; i <= 10; i += 2) {
+ int j;
+ size_t size = i == 4 ? 2 * page_size : page_size;
+ char *buf = &ptr[i * page_size];
+
+ for (j = 0; j < size; j++)
+ buf[j] = rand();
+ }
+
+ /* First, just move the whole thing. */
+ if (mremap(ptr, size, size,
+ MREMAP_MAYMOVE | MREMAP_FIXED, tgt_ptr) == MAP_FAILED) {
+ perror("mremap");
+ success = false;
+ goto out_unmap;
+ }
+ /* Check move was ok. */
+ if (!is_multiple_vma_range_ok(pattern_seed, tgt_ptr, page_size)) {
+ success = false;
+ goto out_unmap;
+ }
+
+ /* Move next to itself. */
+ if (mremap(tgt_ptr, size, size,
+ MREMAP_MAYMOVE | MREMAP_FIXED, &tgt_ptr[size]) == MAP_FAILED) {
+ perror("mremap");
+ goto out_unmap;
+ }
+ /* Check that the move is ok. */
+ if (!is_multiple_vma_range_ok(pattern_seed, &tgt_ptr[size], page_size)) {
+ success = false;
+ goto out_unmap;
+ }
+
+ /* Map a range to overwrite. */
+ if (mmap(tgt_ptr, size, PROT_NONE,
+ MAP_PRIVATE | MAP_ANON | MAP_FIXED, -1, 0) == MAP_FAILED) {
+ perror("mmap tgt");
+ success = false;
+ goto out_unmap;
+ }
+ /* Move and overwrite. */
+ if (mremap(&tgt_ptr[size], size, size,
+ MREMAP_MAYMOVE | MREMAP_FIXED, tgt_ptr) == MAP_FAILED) {
+ perror("mremap");
+ goto out_unmap;
+ }
+ /* Check that the move is ok. */
+ if (!is_multiple_vma_range_ok(pattern_seed, tgt_ptr, page_size)) {
+ success = false;
+ goto out_unmap;
+ }
+
+out_unmap:
+ if (munmap(tgt_ptr, 2 * size))
+ perror("munmap tgt");
+ if (munmap(ptr, size))
+ perror("munmap src");
+
+out:
+ if (success)
+ ksft_test_result_pass("%s\n", test_name);
+ else
+ ksft_test_result_fail("%s\n", test_name);
+}
+
/* Returns the time taken for the remap on success else returns -1. */
static long long remap_region(struct config c, unsigned int threshold_mb,
char *rand_addr)
@@ -721,7 +864,7 @@ int main(int argc, char **argv)
char *rand_addr;
size_t rand_size;
int num_expand_tests = 2;
- int num_misc_tests = 2;
+ int num_misc_tests = 3;
struct test test_cases[MAX_TEST] = {};
struct test perf_test_cases[MAX_PERF_TEST];
int page_size;
@@ -848,6 +991,7 @@ int main(int argc, char **argv)
mremap_move_within_range(pattern_seed, rand_addr);
mremap_move_1mb_from_start(pattern_seed, rand_addr);
+ mremap_move_multiple_vmas(pattern_seed, page_size);
if (run_perf_tests) {
ksft_print_msg("\n%s\n",
--
2.50.1
^ permalink raw reply related
* [PATCH v4 01/10] mm/mremap: perform some simple cleanups
From: Lorenzo Stoakes @ 2025-07-17 16:55 UTC (permalink / raw)
To: Andrew Morton
Cc: Peter Xu, Alexander Viro, Christian Brauner, Jan Kara,
Liam R . Howlett, Vlastimil Babka, Jann Horn, Pedro Falcato,
Rik van Riel, linux-mm, linux-fsdevel, linux-kernel, linux-api
In-Reply-To: <cover.1752770784.git.lorenzo.stoakes@oracle.com>
We const-ify the vrm flags parameter to indicate this will never change.
We rename resize_is_valid() to remap_is_valid(), as this function does not
only apply to cases where we resize, so it's simply confusing to refer to
that here.
We remove the BUG() from mremap_at(), as we should not BUG() unless we are
certain it'll result in system instability.
We rename vrm_charge() to vrm_calc_charge() to make it clear this simply
calculates the charged number of pages rather than actually adjusting any
state.
We update the comment for vrm_implies_new_addr() to explain that
MREMAP_DONTUNMAP does not require a set address, but will always be moved.
Additionally consistently use 'res' rather than 'ret' for result values.
No functional change intended.
Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
---
mm/mremap.c | 55 +++++++++++++++++++++++++++++++----------------------
1 file changed, 32 insertions(+), 23 deletions(-)
diff --git a/mm/mremap.c b/mm/mremap.c
index 1f5bebbb9c0c..65c7f29b6116 100644
--- a/mm/mremap.c
+++ b/mm/mremap.c
@@ -52,7 +52,7 @@ struct vma_remap_struct {
unsigned long addr; /* User-specified address from which we remap. */
unsigned long old_len; /* Length of range being remapped. */
unsigned long new_len; /* Desired new length of mapping. */
- unsigned long flags; /* user-specified MREMAP_* flags. */
+ const unsigned long flags; /* user-specified MREMAP_* flags. */
unsigned long new_addr; /* Optionally, desired new address. */
/* uffd state. */
@@ -909,7 +909,11 @@ static bool vrm_overlaps(struct vma_remap_struct *vrm)
return false;
}
-/* Do the mremap() flags require that the new_addr parameter be specified? */
+/*
+ * Will a new address definitely be assigned? This either if the user specifies
+ * it via MREMAP_FIXED, or if MREMAP_DONTUNMAP is used, indicating we will
+ * always detemrine a target address.
+ */
static bool vrm_implies_new_addr(struct vma_remap_struct *vrm)
{
return vrm->flags & (MREMAP_FIXED | MREMAP_DONTUNMAP);
@@ -955,7 +959,7 @@ static unsigned long vrm_set_new_addr(struct vma_remap_struct *vrm)
*
* Returns true on success, false if insufficient memory to charge.
*/
-static bool vrm_charge(struct vma_remap_struct *vrm)
+static bool vrm_calc_charge(struct vma_remap_struct *vrm)
{
unsigned long charged;
@@ -1260,8 +1264,11 @@ static unsigned long move_vma(struct vma_remap_struct *vrm)
if (err)
return err;
- /* If accounted, charge the number of bytes the operation will use. */
- if (!vrm_charge(vrm))
+ /*
+ * If accounted, determine the number of bytes the operation will
+ * charge.
+ */
+ if (!vrm_calc_charge(vrm))
return -ENOMEM;
/* We don't want racing faults. */
@@ -1300,12 +1307,12 @@ static unsigned long move_vma(struct vma_remap_struct *vrm)
}
/*
- * resize_is_valid() - Ensure the vma can be resized to the new length at the give
- * address.
+ * remap_is_valid() - Ensure the VMA can be moved or resized to the new length,
+ * at the given address.
*
* Return 0 on success, error otherwise.
*/
-static int resize_is_valid(struct vma_remap_struct *vrm)
+static int remap_is_valid(struct vma_remap_struct *vrm)
{
struct mm_struct *mm = current->mm;
struct vm_area_struct *vma = vrm->vma;
@@ -1444,7 +1451,7 @@ static unsigned long mremap_to(struct vma_remap_struct *vrm)
vrm->old_len = vrm->new_len;
}
- err = resize_is_valid(vrm);
+ err = remap_is_valid(vrm);
if (err)
return err;
@@ -1569,7 +1576,7 @@ static unsigned long expand_vma_in_place(struct vma_remap_struct *vrm)
struct vm_area_struct *vma = vrm->vma;
VMA_ITERATOR(vmi, mm, vma->vm_end);
- if (!vrm_charge(vrm))
+ if (!vrm_calc_charge(vrm))
return -ENOMEM;
/*
@@ -1630,7 +1637,7 @@ static unsigned long expand_vma(struct vma_remap_struct *vrm)
unsigned long err;
unsigned long addr = vrm->addr;
- err = resize_is_valid(vrm);
+ err = remap_is_valid(vrm);
if (err)
return err;
@@ -1703,18 +1710,20 @@ static unsigned long mremap_at(struct vma_remap_struct *vrm)
return expand_vma(vrm);
}
- BUG();
+ /* Should not be possible. */
+ WARN_ON_ONCE(1);
+ return -EINVAL;
}
static unsigned long do_mremap(struct vma_remap_struct *vrm)
{
struct mm_struct *mm = current->mm;
struct vm_area_struct *vma;
- unsigned long ret;
+ unsigned long res;
- ret = check_mremap_params(vrm);
- if (ret)
- return ret;
+ res = check_mremap_params(vrm);
+ if (res)
+ return res;
vrm->old_len = PAGE_ALIGN(vrm->old_len);
vrm->new_len = PAGE_ALIGN(vrm->new_len);
@@ -1726,41 +1735,41 @@ static unsigned long do_mremap(struct vma_remap_struct *vrm)
vma = vrm->vma = vma_lookup(mm, vrm->addr);
if (!vma) {
- ret = -EFAULT;
+ res = -EFAULT;
goto out;
}
/* If mseal()'d, mremap() is prohibited. */
if (!can_modify_vma(vma)) {
- ret = -EPERM;
+ res = -EPERM;
goto out;
}
/* Align to hugetlb page size, if required. */
if (is_vm_hugetlb_page(vma) && !align_hugetlb(vrm)) {
- ret = -EINVAL;
+ res = -EINVAL;
goto out;
}
vrm->remap_type = vrm_remap_type(vrm);
/* Actually execute mremap. */
- ret = vrm_implies_new_addr(vrm) ? mremap_to(vrm) : mremap_at(vrm);
+ res = vrm_implies_new_addr(vrm) ? mremap_to(vrm) : mremap_at(vrm);
out:
if (vrm->mmap_locked) {
mmap_write_unlock(mm);
vrm->mmap_locked = false;
- if (!offset_in_page(ret) && vrm->mlocked && vrm->new_len > vrm->old_len)
+ if (!offset_in_page(res) && vrm->mlocked && vrm->new_len > vrm->old_len)
mm_populate(vrm->new_addr + vrm->old_len, vrm->delta);
}
userfaultfd_unmap_complete(mm, vrm->uf_unmap_early);
- mremap_userfaultfd_complete(vrm->uf, vrm->addr, ret, vrm->old_len);
+ mremap_userfaultfd_complete(vrm->uf, vrm->addr, res, vrm->old_len);
userfaultfd_unmap_complete(mm, vrm->uf_unmap);
- return ret;
+ return res;
}
/*
--
2.50.1
^ permalink raw reply related
* Re: [PATCH v4 06/10] mm/mremap: check remap conditions earlier
From: Lorenzo Stoakes @ 2025-07-20 11:04 UTC (permalink / raw)
To: Andrew Morton
Cc: Peter Xu, Alexander Viro, Christian Brauner, Jan Kara,
Liam R . Howlett, Vlastimil Babka, Jann Horn, Pedro Falcato,
Rik van Riel, linux-mm, linux-fsdevel, linux-kernel, linux-api
In-Reply-To: <8b4161ce074901e00602a446d81f182db92b0430.1752770784.git.lorenzo.stoakes@oracle.com>
Hi Andrew,
It turns out there's some undocumented, unusual behaviour in mremap()
around shrinking of a range which was previously missed, but an LTP test
flagged up (seemingly by accident).
Basically, if you specify an input range that spans multiple VMAs, this is
in nearly all cases rejected (this is the point of this series, after all,
for VMA moves).
However, it turns out if you a. shrink a range and b. the new size spans
only a single VMA in the original range - then this requirement is entirely
dropped.
So I need to slightly adjust the logic to account for this. I will also be
documenting this in the man page as it appears the man page contradicts
this or is at least very unclear.
I attach a fix-patch, however there's some very trivial conflicts caused
due to code being moved around.
If you'd therefore prefer me to send a respin, I can do so.
This doesn't reflect on the series itself, which with the corner-case VMA
iterator stuff sorted is fine, but is rather just an undocumented and
unusual behaviour that it seems very few were aware of.
With all other tests passing this series should be fine with this fix
applied. I've run all self-tests and the LTP tests against this.
Cheers, Lorenzo
[0]: https://lore.kernel.org/all/202507201002.69144b74-lkp@intel.com/
----8<----
From 23b95070152b22f7432c4a9da9e4b5718f9d115f Mon Sep 17 00:00:00 2001
From: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Date: Sun, 20 Jul 2025 11:41:48 +0100
Subject: [PATCH] mm/mremap: allow undocumented mremap() shrink behaviour
It turns out that, in apparent contradiction to the man page, and at odds
with every other mremap() operation - we are allowed to specify an input
addr, old_len range that spans any number of VMAs and any number of gaps,
as long as we shrink that range to the point at which the new range spans
only one.
In order to accommodate this, adjust the remap validity check to account
for this.
Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Reported-by: kernel test robot <oliver.sang@intel.com>
Closes: https://lore.kernel.org/oe-lkp/202507201002.69144b74-lkp@intel.com
---
mm/mremap.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/mm/mremap.c b/mm/mremap.c
index 20844fb91755..11a8321a90b8 100644
--- a/mm/mremap.c
+++ b/mm/mremap.c
@@ -1339,11 +1339,18 @@ static int remap_is_valid(struct vma_remap_struct *vrm)
(vma->vm_flags & (VM_DONTEXPAND | VM_PFNMAP)))
return -EINVAL;
+ /*
+ * We permit crossing of boundaries for the range being unmapped due to
+ * a shrink.
+ */
+ if (vrm->remap_type == MREMAP_SHRINK)
+ old_len = new_len;
+
/* We can't remap across vm area boundaries */
if (old_len > vma->vm_end - addr)
return -EFAULT;
- if (new_len <= old_len)
+ if (new_len == old_len)
return 0;
/* Need to be careful about a growing mapping */
--
2.50.1
^ permalink raw reply related
* Re: [PATCH v4 06/10] mm/mremap: check remap conditions earlier
From: Andrew Morton @ 2025-07-20 18:29 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: Peter Xu, Alexander Viro, Christian Brauner, Jan Kara,
Liam R . Howlett, Vlastimil Babka, Jann Horn, Pedro Falcato,
Rik van Riel, linux-mm, linux-fsdevel, linux-kernel, linux-api
In-Reply-To: <8fc92a38-c636-465e-9a2f-2c6ac9cb49b8@lucifer.local>
On Sun, 20 Jul 2025 12:04:42 +0100 Lorenzo Stoakes <lorenzo.stoakes@oracle.com> wrote:
> Hi Andrew,
>
> It turns out there's some undocumented, unusual behaviour in mremap()
> around shrinking of a range which was previously missed, but an LTP test
> flagged up (seemingly by accident).
>
> Basically, if you specify an input range that spans multiple VMAs, this is
> in nearly all cases rejected (this is the point of this series, after all,
> for VMA moves).
>
> However, it turns out if you a. shrink a range and b. the new size spans
> only a single VMA in the original range - then this requirement is entirely
> dropped.
>
> So I need to slightly adjust the logic to account for this. I will also be
> documenting this in the man page as it appears the man page contradicts
> this or is at least very unclear.
>
> I attach a fix-patch, however there's some very trivial conflicts caused
> due to code being moved around.
>
OK, I applied this as a -fix.
Moved the two new hunks into check_prep_vma().
Made sure the "We are expanding and the VMA .." hunk landed properly in
check_prep_vma().
I've pushed out the result, please check current mm-unstable.
^ permalink raw reply
* Re: [PATCH v4 06/10] mm/mremap: check remap conditions earlier
From: Lorenzo Stoakes @ 2025-07-20 18:52 UTC (permalink / raw)
To: Andrew Morton
Cc: Peter Xu, Alexander Viro, Christian Brauner, Jan Kara,
Liam R . Howlett, Vlastimil Babka, Jann Horn, Pedro Falcato,
Rik van Riel, linux-mm, linux-fsdevel, linux-kernel, linux-api
In-Reply-To: <20250720112914.6692e658f4c5b7f4349214be@linux-foundation.org>
On Sun, Jul 20, 2025 at 11:29:14AM -0700, Andrew Morton wrote:
> On Sun, 20 Jul 2025 12:04:42 +0100 Lorenzo Stoakes <lorenzo.stoakes@oracle.com> wrote:
>
> > Hi Andrew,
> >
> > It turns out there's some undocumented, unusual behaviour in mremap()
> > around shrinking of a range which was previously missed, but an LTP test
> > flagged up (seemingly by accident).
> >
> > Basically, if you specify an input range that spans multiple VMAs, this is
> > in nearly all cases rejected (this is the point of this series, after all,
> > for VMA moves).
> >
> > However, it turns out if you a. shrink a range and b. the new size spans
> > only a single VMA in the original range - then this requirement is entirely
> > dropped.
> >
> > So I need to slightly adjust the logic to account for this. I will also be
> > documenting this in the man page as it appears the man page contradicts
> > this or is at least very unclear.
> >
> > I attach a fix-patch, however there's some very trivial conflicts caused
> > due to code being moved around.
> >
>
> OK, I applied this as a -fix.
>
> Moved the two new hunks into check_prep_vma().
>
> Made sure the "We are expanding and the VMA .." hunk landed properly in
> check_prep_vma().
>
> I've pushed out the result, please check current mm-unstable.
Thanks, all looks good to me!
^ permalink raw reply
* Re: [RFC v3 2/4] kernel/api: enable kerneldoc-based API specifications
From: Randy Dunlap @ 2025-07-21 0:55 UTC (permalink / raw)
To: Sasha Levin, linux-kernel; +Cc: linux-doc, linux-api, tools
In-Reply-To: <20250711114248.2288591-3-sashal@kernel.org>
Hi Sasha,
I would like to ask a few questions to try to clarify/understand, please.
On 7/11/25 4:42 AM, Sasha Levin wrote:
> Allow kernel developers to write API specifications directly in
> kerneldoc comments, which are automatically converted to machine-readable
> C macros during build.
>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
> ---
> kernel/api/Makefile | 21 +-
> scripts/Makefile.build | 28 ++
> scripts/generate_api_specs.sh | 59 +++
> scripts/kernel-doc.py | 5 +
> scripts/lib/kdoc/kdoc_apispec.py | 623 +++++++++++++++++++++++++++++++
> scripts/lib/kdoc/kdoc_output.py | 5 +-
> scripts/lib/kdoc/kdoc_parser.py | 54 ++-
> 7 files changed, 791 insertions(+), 4 deletions(-)
> create mode 100755 scripts/generate_api_specs.sh
> create mode 100644 scripts/lib/kdoc/kdoc_apispec.py
>
[snip]
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index a6461ea411f7a..5c0e44d1b6dbc 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -172,6 +172,34 @@ ifneq ($(KBUILD_EXTRA_WARN),)
> $<
> endif
>
> +# Generate API spec headers from kernel-doc comments
> +ifeq ($(CONFIG_KAPI_SPEC),y)
> +# Function to check if a file has API specifications
> +has-apispec = $(shell grep -qE '^\s*\*\s*(api-type|long-desc|context-flags|param-type|error-code|capability|signal|lock|side-effect|state-trans):' $(src)/$(1) 2>/dev/null && echo $(1))
> +
> +# Get base names without directory prefix
> +c-objs-base := $(notdir $(real-obj-y) $(real-obj-m))
> +# Filter to only .o files with corresponding .c source files
> +c-files := $(foreach o,$(c-objs-base),$(if $(wildcard $(src)/$(o:.o=.c)),$(o:.o=.c)))
1. One must build a kernel (with some desired .config file) to use/test this,
correct?
2. It looks like it only checks .c files, omitting header files. (?)
Some APIs are only present in header files (e.g., all of <linux/list.h> is
either macros or inline functions).
> +# Also check for any additional .c files that contain API specs but are included
> +extra-c-files := $(shell find $(src) -maxdepth 1 -name "*.c" -exec grep -l '^\s*\*\s*\(api-type\|long-desc\|context-flags\|param-type\|error-code\|capability\|signal\|lock\|side-effect\|state-trans\):' {} \; 2>/dev/null | xargs -r basename -a)
3a. included files: does this catch the (rare) use of a C file doing
#include <path to some other C file> ?
3b. Quite a few makefiles generate final .o files with a different name
from the source files. It looks like that is handled above by looking
for the first (or intermediate) .o file for each .c file, so the final
.o file with a different name is ignored (or at least doesn't come into
play here). Yes?
> +# Combine both lists and remove duplicates
> +all-c-files := $(sort $(c-files) $(extra-c-files))
> +# Only include files that actually have API specifications
> +apispec-files := $(foreach f,$(all-c-files),$(call has-apispec,$(f)))
> +# Generate apispec targets with proper directory prefix
> +apispec-y := $(addprefix $(obj)/,$(apispec-files:.c=.apispec.h))
> +always-y += $(apispec-y)
> +targets += $(apispec-y)
> +
> +quiet_cmd_apispec = APISPEC $@
> + cmd_apispec = PYTHONDONTWRITEBYTECODE=1 $(KERNELDOC) -apispec \
> + $(KDOCFLAGS) $< > $@ 2>/dev/null || rm -f $@
> +
> +$(obj)/%.apispec.h: $(src)/%.c FORCE
> + $(call if_changed,apispec)
> +endif
> +
> # Compile C sources (.c)
> # ---------------------------------------------------------------------------
>
[snip]
Thanks.
--
~Randy
^ permalink raw reply
* Re: [RFC v3 2/4] kernel/api: enable kerneldoc-based API specifications
From: Sasha Levin @ 2025-07-21 2:54 UTC (permalink / raw)
To: Randy Dunlap; +Cc: linux-kernel, linux-doc, linux-api, tools
In-Reply-To: <4777f4d7-f1c6-4345-92b2-0ba5d6563ee2@infradead.org>
On Sun, Jul 20, 2025 at 05:55:05PM -0700, Randy Dunlap wrote:
>Hi Sasha,
>
>I would like to ask a few questions to try to clarify/understand, please.
>
>On 7/11/25 4:42 AM, Sasha Levin wrote:
>> Allow kernel developers to write API specifications directly in
>> kerneldoc comments, which are automatically converted to machine-readable
>> C macros during build.
>>
>> Signed-off-by: Sasha Levin <sashal@kernel.org>
>> ---
>> kernel/api/Makefile | 21 +-
>> scripts/Makefile.build | 28 ++
>> scripts/generate_api_specs.sh | 59 +++
>> scripts/kernel-doc.py | 5 +
>> scripts/lib/kdoc/kdoc_apispec.py | 623 +++++++++++++++++++++++++++++++
>> scripts/lib/kdoc/kdoc_output.py | 5 +-
>> scripts/lib/kdoc/kdoc_parser.py | 54 ++-
>> 7 files changed, 791 insertions(+), 4 deletions(-)
>> create mode 100755 scripts/generate_api_specs.sh
>> create mode 100644 scripts/lib/kdoc/kdoc_apispec.py
>>
>
>[snip]
>
>> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
>> index a6461ea411f7a..5c0e44d1b6dbc 100644
>> --- a/scripts/Makefile.build
>> +++ b/scripts/Makefile.build
>> @@ -172,6 +172,34 @@ ifneq ($(KBUILD_EXTRA_WARN),)
>> $<
>> endif
>>
>> +# Generate API spec headers from kernel-doc comments
>> +ifeq ($(CONFIG_KAPI_SPEC),y)
>> +# Function to check if a file has API specifications
>> +has-apispec = $(shell grep -qE '^\s*\*\s*(api-type|long-desc|context-flags|param-type|error-code|capability|signal|lock|side-effect|state-trans):' $(src)/$(1) 2>/dev/null && echo $(1))
>> +
>> +# Get base names without directory prefix
>> +c-objs-base := $(notdir $(real-obj-y) $(real-obj-m))
>> +# Filter to only .o files with corresponding .c source files
>> +c-files := $(foreach o,$(c-objs-base),$(if $(wildcard $(src)/$(o:.o=.c)),$(o:.o=.c)))
>
>1. One must build a kernel (with some desired .config file) to use/test this,
>correct?
Mostly yes, but another option is to use the kapi tool (which I haven't
updated and sent out as part of v3, but you can see it in v2:
https://lore.kernel.org/tools/20250624180742.5795-23-sashal@kernel.org/T/#u).
With the kapi tool, you can also run it on a source tree to extract the
same information that you'd have in a built vmlinux.
>2. It looks like it only checks .c files, omitting header files. (?)
>Some APIs are only present in header files (e.g., all of <linux/list.h> is
>either macros or inline functions).
I was trying to focus on the userspace side of things, so I didn't think
we'll have anything in header files, but I also don't have an objection
to extending it to scan headers too.
>> +# Also check for any additional .c files that contain API specs but are included
>> +extra-c-files := $(shell find $(src) -maxdepth 1 -name "*.c" -exec grep -l '^\s*\*\s*\(api-type\|long-desc\|context-flags\|param-type\|error-code\|capability\|signal\|lock\|side-effect\|state-trans\):' {} \; 2>/dev/null | xargs -r basename -a)
>
>3a. included files: does this catch the (rare) use of a C file doing
>#include <path to some other C file> ?
Yes!
Jon suggested I apply this to a kernel/sched/ syscall to see how it
handles complex syscalls, but it also added another complexity because
sched does exactly what you described (kernel/sched/build_policy.c
includes a bunch of other .c files including syscalls.c). See the 4th
patch in the series.
>3b. Quite a few makefiles generate final .o files with a different name
>from the source files. It looks like that is handled above by looking
>for the first (or intermediate) .o file for each .c file, so the final
>.o file with a different name is ignored (or at least doesn't come into
>play here). Yes?
Yes, exactly. The code looks for .o files that have directly
corresponding .c files with the same base name. When makefiles generate
final .o files with different names (like when multiple .c files are
combined into a single .o, or when the output name is changed), those
renamed .o files won't have a matching .c file and will be filtered out
by the $(if $(wildcard $(src)/$(o:.o=.c)),$(o:.o=.c)) check.
>Thanks.
Thanks for looking into it!
--
Thanks,
Sasha
^ permalink raw reply
* Re: [RFC v3 2/4] kernel/api: enable kerneldoc-based API specifications
From: Randy Dunlap @ 2025-07-21 3:17 UTC (permalink / raw)
To: Sasha Levin; +Cc: linux-kernel, linux-doc, linux-api, tools
In-Reply-To: <aH2r55bjqqtKxOEQ@lappy>
On 7/20/25 7:54 PM, Sasha Levin wrote:
>> 2. It looks like it only checks .c files, omitting header files. (?)
>> Some APIs are only present in header files (e.g., all of <linux/list.h> is
>> either macros or inline functions).
>
> I was trying to focus on the userspace side of things, so I didn't think
> we'll have anything in header files, but I also don't have an objection
> to extending it to scan headers too.
That's probably sufficient, at least for now.
Thanks.
--
~Randy
^ permalink raw reply
* [PATCH RFC 0/4] procfs: make reference pidns more user-visible
From: Aleksa Sarai @ 2025-07-21 8:44 UTC (permalink / raw)
To: Alexander Viro, Christian Brauner, Jan Kara, Jonathan Corbet,
Shuah Khan
Cc: linux-kernel, linux-fsdevel, linux-api, linux-doc,
linux-kselftest, Aleksa Sarai
Ever since the introduction of pid namespaces, procfs has had very
implicit behaviour surrounding them (the pidns used by a procfs mount is
auto-selected based on the mounting process's active pidns, and the
pidns itself is basically hidden once the mount has been constructed).
This has historically meant that userspace was required to do some
special dances in order to configure the pidns of a procfs mount as
desired. Examples include:
* In order to bypass the mnt_too_revealing() check, Kubernetes creates
a procfs mount from an empty pidns so that user namespaced containers
can be nested (without this, the nested containers would fail to
mount procfs). But this requires forking off a helper process because
you cannot just one-shot this using mount(2).
* Container runtimes in general need to fork into a container before
configuring its mounts, which can lead to security issues in the case
of shared-pidns containers (a privileged process in the pidns can
interact with your container runtime process). While
SUID_DUMP_DISABLE and user namespaces make this less of an issue, the
strict need for this due to a minor uAPI wart is kind of unfortunate.
Things would be much easier if there was a way for userspace to just
specify the pidns they want. Patch 1 implements a new "pidns" argument
which can be set using fsconfig(2):
fsconfig(procfd, FSCONFIG_SET_FD, "pidns", NULL, nsfd);
fsconfig(procfd, FSCONFIG_SET_STRING, "pidns", "/proc/self/ns/pid", 0);
or classic mount(2) / mount(8):
// mount -t proc -o pidns=/proc/self/ns/pid proc /tmp/proc
mount("proc", "/tmp/proc", "proc", MS_..., "pidns=/proc/self/ns/pid");
The initial security model I have in this RFC is to be as conservative
as possible and just mirror the security model for setns(2) -- which
means that you can only set pidns=... to pid namespaces that your
current pid namespace is a direct ancestor of. This fulfils the
requirements of container runtimes, but I suspect that this may be too
strict for some usecases.
The pidns argument is not displayed in mountinfo -- it's not clear to me
what value it would make sense to show (maybe we could just use ns_dname
to provide an identifier for the namespace, but this number would be
fairly useless to userspace). I'm open to suggestions.
In addition, being able to figure out what pid namespace is being used
by a procfs mount is quite useful when you have an administrative
process (such as a container runtime) which wants to figure out the
correct way of mapping PIDs between its own namespace and the namespace
for procfs (using NS_GET_{PID,TGID}_{IN,FROM}_PIDNS). There are
alternative ways to do this, but they all rely on ancillary information
that third-party libraries and tools do not necessarily have access to.
To make this easier, add a new ioctl (PROCFS_GET_PID_NAMESPACE) which
can be used to get a reference to the pidns that a procfs is using.
It's not quite clear what is the correct security model for this API,
but the current approach I've taken is to:
* Make the ioctl only valid on the root (meaning that a process without
access to the procfs root -- such as only having an fd to a procfs
file or some open_tree(2)-like subset -- cannot use this API).
* Require that the process requesting either has access to
/proc/1/ns/pid anyway (i.e. has ptrace-read access to the pidns
pid1), has CAP_SYS_ADMIN access to the pidns (i.e. has administrative
access to it and can join it if they had a handle), or is in a pidns
that is a direct ancestor of the target pidns (i.e. all of the pids
are already visible in the procfs for the current process's pidns).
The security model for this is a little loose, as it seems to me that
all of the cases mentioned are valid cases to allow access, but I'm open
to suggestions for whether we need to make this stricter or looser.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
---
Aleksa Sarai (4):
pidns: move is-ancestor logic to helper
procfs: add pidns= mount option
procfs: add PROCFS_GET_PID_NAMESPACE ioctl
selftests/proc: add tests for new pidns APIs
Documentation/filesystems/proc.rst | 10 ++
fs/proc/root.c | 132 +++++++++++++-
include/linux/pid_namespace.h | 9 +
include/uapi/linux/fs.h | 3 +
kernel/pid_namespace.c | 21 ++-
tools/testing/selftests/proc/.gitignore | 1 +
tools/testing/selftests/proc/Makefile | 1 +
tools/testing/selftests/proc/proc-pidns.c | 286 ++++++++++++++++++++++++++++++
8 files changed, 448 insertions(+), 15 deletions(-)
---
base-commit: 4c838c7672c39ec6ec48456c6ce22d14a68f4cda
change-id: 20250717-procfs-pidns-api-8ed1583431f0
Best regards,
--
Aleksa Sarai <cyphar@cyphar.com>
^ permalink raw reply
* [PATCH RFC 1/4] pidns: move is-ancestor logic to helper
From: Aleksa Sarai @ 2025-07-21 8:44 UTC (permalink / raw)
To: Alexander Viro, Christian Brauner, Jan Kara, Jonathan Corbet,
Shuah Khan
Cc: linux-kernel, linux-fsdevel, linux-api, linux-doc,
linux-kselftest, Aleksa Sarai
In-Reply-To: <20250721-procfs-pidns-api-v1-0-5cd9007e512d@cyphar.com>
This check will be needed in later patches, and there's no point
open-coding it each time.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
---
include/linux/pid_namespace.h | 9 +++++++++
kernel/pid_namespace.c | 21 ++++++++++++++-------
2 files changed, 23 insertions(+), 7 deletions(-)
diff --git a/include/linux/pid_namespace.h b/include/linux/pid_namespace.h
index 7c67a5811199..df421a1e3e0b 100644
--- a/include/linux/pid_namespace.h
+++ b/include/linux/pid_namespace.h
@@ -84,6 +84,9 @@ extern void zap_pid_ns_processes(struct pid_namespace *pid_ns);
extern int reboot_pid_ns(struct pid_namespace *pid_ns, int cmd);
extern void put_pid_ns(struct pid_namespace *ns);
+extern bool pidns_is_ancestor(struct pid_namespace *child,
+ struct pid_namespace *ancestor);
+
#else /* !CONFIG_PID_NS */
#include <linux/err.h>
@@ -118,6 +121,12 @@ static inline int reboot_pid_ns(struct pid_namespace *pid_ns, int cmd)
{
return 0;
}
+
+bool pidns_is_ancestor(struct pid_namespace *child,
+ struct pid_namespace *ancestor)
+{
+ return false;
+}
#endif /* CONFIG_PID_NS */
extern struct pid_namespace *task_active_pid_ns(struct task_struct *tsk);
diff --git a/kernel/pid_namespace.c b/kernel/pid_namespace.c
index 7098ed44e717..5719b1f679ad 100644
--- a/kernel/pid_namespace.c
+++ b/kernel/pid_namespace.c
@@ -390,6 +390,19 @@ static void pidns_put(struct ns_common *ns)
put_pid_ns(to_pid_ns(ns));
}
+bool pidns_is_ancestor(struct pid_namespace *child,
+ struct pid_namespace *ancestor)
+{
+ struct pid_namespace *ns;
+
+ if (child->level < ancestor->level)
+ return false;
+ for (ns = child; ns->level > ancestor->level; ns = ns->parent)
+ ;
+ return ns == ancestor;
+}
+EXPORT_SYMBOL_GPL(pidns_is_ancestor);
+
static int pidns_install(struct nsset *nsset, struct ns_common *ns)
{
struct nsproxy *nsproxy = nsset->nsproxy;
@@ -408,13 +421,7 @@ static int pidns_install(struct nsset *nsset, struct ns_common *ns)
* this maintains the property that processes and their
* children can not escape their current pid namespace.
*/
- if (new->level < active->level)
- return -EINVAL;
-
- ancestor = new;
- while (ancestor->level > active->level)
- ancestor = ancestor->parent;
- if (ancestor != active)
+ if (!pidns_is_ancestor(new, active))
return -EINVAL;
put_pid_ns(nsproxy->pid_ns_for_children);
--
2.50.0
^ permalink raw reply related
* [PATCH RFC 2/4] procfs: add pidns= mount option
From: Aleksa Sarai @ 2025-07-21 8:44 UTC (permalink / raw)
To: Alexander Viro, Christian Brauner, Jan Kara, Jonathan Corbet,
Shuah Khan
Cc: linux-kernel, linux-fsdevel, linux-api, linux-doc,
linux-kselftest, Aleksa Sarai
In-Reply-To: <20250721-procfs-pidns-api-v1-0-5cd9007e512d@cyphar.com>
Since the introduction of pid namespaces, their interaction with procfs
has been entirely implicit in ways that require a lot of dancing around
by programs that need to construct sandboxes with different PID
namespaces.
Being able to explicitly specify the pid namespace to use when
constructing a procfs super block will allow programs to no longer need
to fork off a process which does then does unshare(2) / setns(2) and
forks again in order to construct a procfs in a pidns.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
---
Documentation/filesystems/proc.rst | 6 +++
fs/proc/root.c | 80 +++++++++++++++++++++++++++++++++++---
2 files changed, 80 insertions(+), 6 deletions(-)
diff --git a/Documentation/filesystems/proc.rst b/Documentation/filesystems/proc.rst
index 5236cb52e357..c520b9f8a3fd 100644
--- a/Documentation/filesystems/proc.rst
+++ b/Documentation/filesystems/proc.rst
@@ -2360,6 +2360,7 @@ The following mount options are supported:
hidepid= Set /proc/<pid>/ access mode.
gid= Set the group authorized to learn processes information.
subset= Show only the specified subset of procfs.
+ pidns= Specify a the namespace used by this procfs.
========= ========================================================
hidepid=off or hidepid=0 means classic mode - everybody may access all
@@ -2392,6 +2393,11 @@ information about processes information, just add identd to this group.
subset=pid hides all top level files and directories in the procfs that
are not related to tasks.
+pidns= specifies a pid namespace (either as a string path to something like
+`/proc/$pid/ns/pid`, or a file descriptor when using `FSCONFIG_SET_FD`) that
+will be used by the procfs instance when translating pids. By default, procfs
+will use the calling process's active pid namespace.
+
Chapter 5: Filesystem behavior
==============================
diff --git a/fs/proc/root.c b/fs/proc/root.c
index ed86ac710384..10ca94be0eef 100644
--- a/fs/proc/root.c
+++ b/fs/proc/root.c
@@ -38,12 +38,14 @@ enum proc_param {
Opt_gid,
Opt_hidepid,
Opt_subset,
+ Opt_pidns,
};
static const struct fs_parameter_spec proc_fs_parameters[] = {
- fsparam_u32("gid", Opt_gid),
+ fsparam_u32("gid", Opt_gid),
fsparam_string("hidepid", Opt_hidepid),
fsparam_string("subset", Opt_subset),
+ fsparam_file_or_string("pidns", Opt_pidns),
{}
};
@@ -109,11 +111,65 @@ static int proc_parse_subset_param(struct fs_context *fc, char *value)
return 0;
}
+static int proc_parse_pidns_param(struct fs_context *fc,
+ struct fs_parameter *param,
+ struct fs_parse_result *result)
+{
+ struct proc_fs_context *ctx = fc->fs_private;
+ struct pid_namespace *target, *active = task_active_pid_ns(current);
+ struct ns_common *ns;
+ struct file *ns_filp __free(fput) = NULL;
+
+ switch (param->type) {
+ case fs_value_is_file:
+ /* came throug fsconfig, steal the file reference */
+ ns_filp = param->file;
+ param->file = NULL;
+ break;
+ case fs_value_is_string:
+ ns_filp = filp_open(param->string, O_RDONLY, 0);
+ break;
+ default:
+ WARN_ON_ONCE(true);
+ break;
+ }
+ if (!ns_filp)
+ ns_filp = ERR_PTR(-EBADF);
+ if (IS_ERR(ns_filp)) {
+ errorfc(fc, "could not get file from pidns argument");
+ return PTR_ERR(ns_filp);
+ }
+
+ if (!proc_ns_file(ns_filp))
+ return invalfc(fc, "pidns argument is not an nsfs file");
+ ns = get_proc_ns(file_inode(ns_filp));
+ if (ns->ops->type != CLONE_NEWPID)
+ return invalfc(fc, "pidns argument is not a pidns file");
+ target = container_of(ns, struct pid_namespace, ns);
+
+ /*
+ * pidns= is shorthand for joining the pidns to get a fsopen fd, so the
+ * permission model should be the same as pidns_install().
+ */
+ if (!ns_capable(target->user_ns, CAP_SYS_ADMIN)) {
+ errorfc(fc, "insufficient permissions to set pidns");
+ return -EPERM;
+ }
+ if (!pidns_is_ancestor(target, active))
+ return invalfc(fc, "cannot set pidns to non-descendant pidns");
+
+ put_pid_ns(ctx->pid_ns);
+ ctx->pid_ns = get_pid_ns(target);
+ put_user_ns(fc->user_ns);
+ fc->user_ns = get_user_ns(ctx->pid_ns->user_ns);
+ return 0;
+}
+
static int proc_parse_param(struct fs_context *fc, struct fs_parameter *param)
{
struct proc_fs_context *ctx = fc->fs_private;
struct fs_parse_result result;
- int opt;
+ int opt, err;
opt = fs_parse(fc, proc_fs_parameters, param, &result);
if (opt < 0)
@@ -125,13 +181,21 @@ static int proc_parse_param(struct fs_context *fc, struct fs_parameter *param)
break;
case Opt_hidepid:
- if (proc_parse_hidepid_param(fc, param))
- return -EINVAL;
+ err = proc_parse_hidepid_param(fc, param);
+ if (err)
+ return err;
break;
case Opt_subset:
- if (proc_parse_subset_param(fc, param->string) < 0)
- return -EINVAL;
+ err = proc_parse_subset_param(fc, param->string);
+ if (err)
+ return err;
+ break;
+
+ case Opt_pidns:
+ err = proc_parse_pidns_param(fc, param, &result);
+ if (err)
+ return err;
break;
default:
@@ -154,6 +218,10 @@ static void proc_apply_options(struct proc_fs_info *fs_info,
fs_info->hide_pid = ctx->hidepid;
if (ctx->mask & (1 << Opt_subset))
fs_info->pidonly = ctx->pidonly;
+ if (ctx->mask & (1 << Opt_pidns)) {
+ put_pid_ns(fs_info->pid_ns);
+ fs_info->pid_ns = get_pid_ns(ctx->pid_ns);
+ }
}
static int proc_fill_super(struct super_block *s, struct fs_context *fc)
--
2.50.0
^ permalink raw reply related
* [PATCH RFC 3/4] procfs: add PROCFS_GET_PID_NAMESPACE ioctl
From: Aleksa Sarai @ 2025-07-21 8:44 UTC (permalink / raw)
To: Alexander Viro, Christian Brauner, Jan Kara, Jonathan Corbet,
Shuah Khan
Cc: linux-kernel, linux-fsdevel, linux-api, linux-doc,
linux-kselftest, Aleksa Sarai
In-Reply-To: <20250721-procfs-pidns-api-v1-0-5cd9007e512d@cyphar.com>
/proc has historically had very opaque semantics about PID namespaces,
which is a little unfortunate for container runtimes and other programs
that deal with switching namespaces very often. One common issue is that
of converting between PIDs in the process's namespace and PIDs in the
namespace of /proc.
In principle, it is possible to do this today by opening a pidfd with
pidfd_open(2) and then looking at /proc/self/fdinfo/$n (which will
contain a PID value translated to the pid namespace associated with that
procfs superblock).
However, allocating a new file for each PID to be converted is less than
ideal for programs that may need to scan procfs, and it is generally
useful for userspace to be able to finally get this information from
procfs. This also acts as a sister feature to the pidns= mount option,
finally allowing userspace full control of the pid namespaces associated
with /proc instances.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
---
Documentation/filesystems/proc.rst | 4 +++
fs/proc/root.c | 52 ++++++++++++++++++++++++++++++++++++--
include/uapi/linux/fs.h | 3 +++
3 files changed, 57 insertions(+), 2 deletions(-)
diff --git a/Documentation/filesystems/proc.rst b/Documentation/filesystems/proc.rst
index c520b9f8a3fd..506383273c9d 100644
--- a/Documentation/filesystems/proc.rst
+++ b/Documentation/filesystems/proc.rst
@@ -2398,6 +2398,10 @@ pidns= specifies a pid namespace (either as a string path to something like
will be used by the procfs instance when translating pids. By default, procfs
will use the calling process's active pid namespace.
+Processes can check which pid namespace is used by a procfs instance by using
+the `PROCFS_GET_PID_NAMESPACE` ioctl() on the root directory of the procfs
+instance.
+
Chapter 5: Filesystem behavior
==============================
diff --git a/fs/proc/root.c b/fs/proc/root.c
index 10ca94be0eef..ee90749ccd8e 100644
--- a/fs/proc/root.c
+++ b/fs/proc/root.c
@@ -23,8 +23,10 @@
#include <linux/cred.h>
#include <linux/magic.h>
#include <linux/slab.h>
+#include <linux/ptrace.h>
#include "internal.h"
+#include "../internal.h"
struct proc_fs_context {
struct pid_namespace *pid_ns;
@@ -408,15 +410,61 @@ static int proc_root_readdir(struct file *file, struct dir_context *ctx)
return proc_pid_readdir(file, ctx);
}
+static long int proc_root_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
+{
+ switch (cmd) {
+ case PROCFS_GET_PID_NAMESPACE: {
+ struct pid_namespace *active = task_active_pid_ns(current);
+ struct pid_namespace *ns = proc_pid_ns(file_inode(filp)->i_sb);
+ bool can_access_pidns = false;
+
+ /*
+ * If we are in an ancestors of the pidns, or have join
+ * privileges (CAP_SYS_ADMIN), then it makes sense that we
+ * would be able to grab a handle to the pidns.
+ *
+ * Otherwise, if there is a root process, then being able to
+ * access /proc/$pid/ns/pid is equivalent to this ioctl and so
+ * we should probably match the permission model. For empty
+ * namespaces it seems unlikely for there to be a downside to
+ * allowing unprivileged users to open a handle to it (setns
+ * will fail for unprivileged users anyway).
+ */
+ can_access_pidns = pidns_is_ancestor(ns, active) ||
+ ns_capable(ns->user_ns, CAP_SYS_ADMIN);
+ if (!can_access_pidns) {
+ bool cannot_ptrace_pid1 = false;
+
+ read_lock(&tasklist_lock);
+ if (ns->child_reaper)
+ cannot_ptrace_pid1 = ptrace_may_access(ns->child_reaper,
+ PTRACE_MODE_READ_FSCREDS);
+ read_unlock(&tasklist_lock);
+ can_access_pidns = !cannot_ptrace_pid1;
+ }
+ if (!can_access_pidns)
+ return -EPERM;
+
+ /* open_namespace() unconditionally consumes the reference. */
+ get_pid_ns(ns);
+ return open_namespace(to_ns_common(ns));
+ }
+ default:
+ return -ENOIOCTLCMD;
+ }
+}
+
/*
* The root /proc directory is special, as it has the
* <pid> directories. Thus we don't use the generic
* directory handling functions for that..
*/
static const struct file_operations proc_root_operations = {
- .read = generic_read_dir,
- .iterate_shared = proc_root_readdir,
+ .read = generic_read_dir,
+ .iterate_shared = proc_root_readdir,
.llseek = generic_file_llseek,
+ .unlocked_ioctl = proc_root_ioctl,
+ .compat_ioctl = compat_ptr_ioctl,
};
/*
diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
index 0bd678a4a10e..aa642cb48feb 100644
--- a/include/uapi/linux/fs.h
+++ b/include/uapi/linux/fs.h
@@ -437,6 +437,9 @@ typedef int __bitwise __kernel_rwf_t;
#define PROCFS_IOCTL_MAGIC 'f'
+/* procfs root ioctls */
+#define PROCFS_GET_PID_NAMESPACE _IO(PROCFS_IOCTL_MAGIC, 1)
+
/* Pagemap ioctl */
#define PAGEMAP_SCAN _IOWR(PROCFS_IOCTL_MAGIC, 16, struct pm_scan_arg)
--
2.50.0
^ permalink raw reply related
* [PATCH RFC 4/4] selftests/proc: add tests for new pidns APIs
From: Aleksa Sarai @ 2025-07-21 8:44 UTC (permalink / raw)
To: Alexander Viro, Christian Brauner, Jan Kara, Jonathan Corbet,
Shuah Khan
Cc: linux-kernel, linux-fsdevel, linux-api, linux-doc,
linux-kselftest, Aleksa Sarai
In-Reply-To: <20250721-procfs-pidns-api-v1-0-5cd9007e512d@cyphar.com>
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
---
tools/testing/selftests/proc/.gitignore | 1 +
tools/testing/selftests/proc/Makefile | 1 +
tools/testing/selftests/proc/proc-pidns.c | 286 ++++++++++++++++++++++++++++++
3 files changed, 288 insertions(+)
diff --git a/tools/testing/selftests/proc/.gitignore b/tools/testing/selftests/proc/.gitignore
index 973968f45bba..2dced03e9e0e 100644
--- a/tools/testing/selftests/proc/.gitignore
+++ b/tools/testing/selftests/proc/.gitignore
@@ -17,6 +17,7 @@
/proc-tid0
/proc-uptime-001
/proc-uptime-002
+/proc-pidns
/read
/self
/setns-dcache
diff --git a/tools/testing/selftests/proc/Makefile b/tools/testing/selftests/proc/Makefile
index b12921b9794b..c6f7046b9860 100644
--- a/tools/testing/selftests/proc/Makefile
+++ b/tools/testing/selftests/proc/Makefile
@@ -27,5 +27,6 @@ TEST_GEN_PROGS += setns-sysvipc
TEST_GEN_PROGS += thread-self
TEST_GEN_PROGS += proc-multiple-procfs
TEST_GEN_PROGS += proc-fsconfig-hidepid
+TEST_GEN_PROGS += proc-pidns
include ../lib.mk
diff --git a/tools/testing/selftests/proc/proc-pidns.c b/tools/testing/selftests/proc/proc-pidns.c
new file mode 100644
index 000000000000..e7e34c78d383
--- /dev/null
+++ b/tools/testing/selftests/proc/proc-pidns.c
@@ -0,0 +1,286 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Author: Aleksa Sarai <cyphar@cyphar.com>
+ * Copyright (C) 2025 SUSE LLC.
+ */
+
+#include <assert.h>
+#include <errno.h>
+#include <sched.h>
+#include <stdbool.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <stdio.h>
+#include <sys/mount.h>
+#include <sys/stat.h>
+#include <sys/ioctl.h>
+#include <sys/prctl.h>
+
+#include "../kselftest_harness.h"
+
+#define bail(fmt, ...) \
+ do { \
+ fprintf(stderr, fmt ": %m", __VA_ARGS__); \
+ exit(1); \
+ } while (0)
+
+#define ASSERT_SUCCESS ASSERT_FALSE
+#define ASSERT_FAIL ASSERT_TRUE
+
+int touch(char *path)
+{
+ int fd = open(path, O_WRONLY|O_CREAT|O_CLOEXEC, 0644);
+ if (fd < 0 || close(fd) < 0)
+ return -errno;
+ return 0;
+}
+
+FIXTURE(ns)
+{
+ int host_mntns, host_pidns;
+ int dummy_pidns;
+};
+
+FIXTURE_SETUP(ns)
+{
+ /* Stash the old mntns. */
+ self->host_mntns = open("/proc/self/ns/mnt", O_RDONLY|O_CLOEXEC);
+ ASSERT_GE(self->host_mntns, 0);
+
+ /* Create a new mount namespace and make it private. */
+ ASSERT_SUCCESS(unshare(CLONE_NEWNS));
+ ASSERT_SUCCESS(mount(NULL, "/", NULL, MS_PRIVATE|MS_REC, NULL));
+
+ /*
+ * Create a proper tmpfs that we can use and will disappear once we
+ * leave this mntns.
+ */
+ ASSERT_SUCCESS(mount("tmpfs", "/tmp", "tmpfs", 0, NULL));
+
+ /*
+ * Create a pidns we can use for later tests. We need to fork off a
+ * child so that we get a usable nsfd that we can bind-mount and open.
+ */
+ ASSERT_SUCCESS(touch("/tmp/dummy-pidns"));
+
+ self->host_pidns = open("/proc/self/ns/pid", O_RDONLY|O_CLOEXEC);
+ ASSERT_GE(self->host_pidns, 0);
+ ASSERT_SUCCESS(unshare(CLONE_NEWPID));
+
+ pid_t pid = fork();
+ ASSERT_GE(pid, 0);
+ if (!pid) {
+ prctl(PR_SET_PDEATHSIG, SIGKILL);
+ ASSERT_SUCCESS(mount("/proc/self/ns/pid", "/tmp/dummy-pidns", NULL, MS_BIND, 0));
+ exit(0);
+ }
+
+ int wstatus;
+ ASSERT_EQ(waitpid(pid, &wstatus, 0), pid);
+ ASSERT_TRUE(WIFEXITED(wstatus));
+ ASSERT_EQ(WEXITSTATUS(wstatus), 0);
+
+ ASSERT_SUCCESS(setns(self->host_pidns, CLONE_NEWPID));
+
+ self->dummy_pidns = open("/tmp/dummy-pidns", O_RDONLY|O_CLOEXEC);
+ ASSERT_GE(self->dummy_pidns, 0);
+}
+
+FIXTURE_TEARDOWN(ns)
+{
+ ASSERT_SUCCESS(setns(self->host_mntns, CLONE_NEWNS));
+ ASSERT_SUCCESS(close(self->host_mntns));
+
+ ASSERT_SUCCESS(close(self->host_pidns));
+ ASSERT_SUCCESS(close(self->dummy_pidns));
+}
+
+TEST_F(ns, pidns_mount_string_path)
+{
+ ASSERT_SUCCESS(mkdir("/tmp/proc-host", 0755));
+ ASSERT_SUCCESS(mount("proc", "/tmp/proc-host", "proc", 0, "pidns=/proc/self/ns/pid"));
+ ASSERT_SUCCESS(access("/tmp/proc-host/self/", X_OK));
+
+ ASSERT_SUCCESS(mkdir("/tmp/proc-dummy", 0755));
+ ASSERT_SUCCESS(mount("proc", "/tmp/proc-dummy", "proc", 0, "pidns=/tmp/dummy-pidns"));
+ ASSERT_FAIL(access("/tmp/proc-dummy/1/", X_OK));
+ ASSERT_FAIL(access("/tmp/proc-dummy/self/", X_OK));
+}
+
+TEST_F(ns, pidns_fsconfig_string_path)
+{
+ int fsfd = fsopen("proc", FSOPEN_CLOEXEC);
+ ASSERT_GE(fsfd, 0);
+
+ ASSERT_SUCCESS(fsconfig(fsfd, FSCONFIG_SET_STRING, "pidns", "/tmp/dummy-pidns", 0));
+ ASSERT_SUCCESS(fsconfig(fsfd, FSCONFIG_CMD_CREATE, NULL, NULL, 0));
+
+ int mountfd = fsmount(fsfd, FSMOUNT_CLOEXEC, 0);
+ ASSERT_GE(mountfd, 0);
+
+ ASSERT_FAIL(faccessat(mountfd, "1/", X_OK, 0));
+ ASSERT_FAIL(faccessat(mountfd, "self/", X_OK, 0));
+
+ ASSERT_SUCCESS(close(fsfd));
+ ASSERT_SUCCESS(close(mountfd));
+}
+
+TEST_F(ns, pidns_fsconfig_fd)
+{
+ int fsfd = fsopen("proc", FSOPEN_CLOEXEC);
+ ASSERT_GE(fsfd, 0);
+
+ ASSERT_SUCCESS(fsconfig(fsfd, FSCONFIG_SET_FD, "pidns", NULL, self->dummy_pidns));
+ ASSERT_SUCCESS(fsconfig(fsfd, FSCONFIG_CMD_CREATE, NULL, NULL, 0));
+
+ int mountfd = fsmount(fsfd, FSMOUNT_CLOEXEC, 0);
+ ASSERT_GE(mountfd, 0);
+
+ ASSERT_FAIL(faccessat(mountfd, "1/", X_OK, 0));
+ ASSERT_FAIL(faccessat(mountfd, "self/", X_OK, 0));
+
+ ASSERT_SUCCESS(close(fsfd));
+ ASSERT_SUCCESS(close(mountfd));
+}
+
+TEST_F(ns, pidns_reconfigure_remount)
+{
+ ASSERT_SUCCESS(mkdir("/tmp/proc", 0755));
+ ASSERT_SUCCESS(mount("proc", "/tmp/proc", "proc", 0, ""));
+ ASSERT_SUCCESS(access("/tmp/proc/self/", X_OK));
+
+ ASSERT_SUCCESS(mount(NULL, "/tmp/proc", NULL, MS_REMOUNT, "pidns=/tmp/dummy-pidns"));
+ ASSERT_FAIL(access("/tmp/proc/self/", X_OK));
+}
+
+TEST_F(ns, pidns_reconfigure_fsconfig_string_path)
+{
+ int fsfd = fsopen("proc", FSOPEN_CLOEXEC);
+ ASSERT_GE(fsfd, 0);
+
+ ASSERT_SUCCESS(fsconfig(fsfd, FSCONFIG_CMD_CREATE, NULL, NULL, 0));
+
+ int mountfd = fsmount(fsfd, FSMOUNT_CLOEXEC, 0);
+ ASSERT_GE(mountfd, 0);
+
+ ASSERT_SUCCESS(faccessat(mountfd, "1/", X_OK, 0));
+ ASSERT_SUCCESS(faccessat(mountfd, "self/", X_OK, 0));
+
+ ASSERT_SUCCESS(fsconfig(fsfd, FSCONFIG_SET_STRING, "pidns", "/tmp/dummy-pidns", 0));
+ ASSERT_SUCCESS(fsconfig(fsfd, FSCONFIG_CMD_RECONFIGURE, NULL, NULL, 0));
+
+ ASSERT_FAIL(faccessat(mountfd, "1/", X_OK, 0));
+ ASSERT_FAIL(faccessat(mountfd, "self/", X_OK, 0));
+
+ ASSERT_SUCCESS(close(fsfd));
+ ASSERT_SUCCESS(close(mountfd));
+}
+
+TEST_F(ns, pidns_reconfigure_fsconfig_fd)
+{
+ int fsfd = fsopen("proc", FSOPEN_CLOEXEC);
+ ASSERT_GE(fsfd, 0);
+
+ ASSERT_SUCCESS(fsconfig(fsfd, FSCONFIG_CMD_CREATE, NULL, NULL, 0));
+
+ int mountfd = fsmount(fsfd, FSMOUNT_CLOEXEC, 0);
+ ASSERT_GE(mountfd, 0);
+
+ ASSERT_SUCCESS(faccessat(mountfd, "1/", X_OK, 0));
+ ASSERT_SUCCESS(faccessat(mountfd, "self/", X_OK, 0));
+
+ ASSERT_SUCCESS(fsconfig(fsfd, FSCONFIG_SET_FD, "pidns", NULL, self->dummy_pidns));
+ ASSERT_SUCCESS(fsconfig(fsfd, FSCONFIG_CMD_RECONFIGURE, NULL, NULL, 0));
+
+ ASSERT_FAIL(faccessat(mountfd, "1/", X_OK, 0));
+ ASSERT_FAIL(faccessat(mountfd, "self/", X_OK, 0));
+
+ ASSERT_SUCCESS(close(fsfd));
+ ASSERT_SUCCESS(close(mountfd));
+}
+
+int is_same_inode(int fd1, int fd2)
+{
+ struct stat stat1, stat2;
+
+ assert(fstat(fd1, &stat1) == 0);
+ assert(fstat(fd2, &stat2) == 0);
+
+ return stat1.st_ino == stat2.st_ino && stat1.st_dev == stat2.st_dev;
+}
+
+#define PROCFS_IOCTL_MAGIC 'f'
+#define PROCFS_GET_PID_NAMESPACE _IO(PROCFS_IOCTL_MAGIC, 1)
+
+TEST_F(ns, get_pidns_ioctl)
+{
+ int fsfd = fsopen("proc", FSOPEN_CLOEXEC);
+ ASSERT_GE(fsfd, 0);
+
+ ASSERT_SUCCESS(fsconfig(fsfd, FSCONFIG_SET_FD, "pidns", NULL, self->dummy_pidns));
+ ASSERT_SUCCESS(fsconfig(fsfd, FSCONFIG_CMD_CREATE, NULL, NULL, 0));
+
+ int mountfd = fsmount(fsfd, FSMOUNT_CLOEXEC, 0);
+ ASSERT_GE(mountfd, 0);
+
+ /* fsmount returns an O_PATH, which ioctl(2) doesn't accept. */
+ int new_mountfd = openat(mountfd, ".", O_RDONLY|O_DIRECTORY|O_CLOEXEC);
+ ASSERT_GE(new_mountfd, 0);
+
+ ASSERT_SUCCESS(close(mountfd));
+ mountfd = -EBADF;
+
+ int procfs_pidns = ioctl(new_mountfd, PROCFS_GET_PID_NAMESPACE);
+ ASSERT_GE(procfs_pidns, 0);
+
+ ASSERT_NE(self->dummy_pidns, procfs_pidns);
+ ASSERT_FALSE(is_same_inode(self->host_pidns, procfs_pidns));
+ ASSERT_TRUE(is_same_inode(self->dummy_pidns, procfs_pidns));
+
+ ASSERT_SUCCESS(close(fsfd));
+ ASSERT_SUCCESS(close(new_mountfd));
+ ASSERT_SUCCESS(close(procfs_pidns));
+}
+
+TEST_F(ns, reconfigure_get_pidns_ioctl)
+{
+ int fsfd = fsopen("proc", FSOPEN_CLOEXEC);
+ ASSERT_GE(fsfd, 0);
+
+ ASSERT_SUCCESS(fsconfig(fsfd, FSCONFIG_CMD_CREATE, NULL, NULL, 0));
+
+ int mountfd = fsmount(fsfd, FSMOUNT_CLOEXEC, 0);
+ ASSERT_GE(mountfd, 0);
+
+ /* fsmount returns an O_PATH, which ioctl(2) doesn't accept. */
+ int new_mountfd = openat(mountfd, ".", O_RDONLY|O_DIRECTORY|O_CLOEXEC);
+ ASSERT_GE(new_mountfd, 0);
+
+ ASSERT_SUCCESS(close(mountfd));
+ mountfd = -EBADF;
+
+ int procfs_pidns1 = ioctl(new_mountfd, PROCFS_GET_PID_NAMESPACE);
+ ASSERT_GE(procfs_pidns1, 0);
+
+ ASSERT_NE(self->dummy_pidns, procfs_pidns1);
+ ASSERT_TRUE(is_same_inode(self->host_pidns, procfs_pidns1));
+ ASSERT_FALSE(is_same_inode(self->dummy_pidns, procfs_pidns1));
+
+ ASSERT_SUCCESS(fsconfig(fsfd, FSCONFIG_SET_STRING, "pidns", "/tmp/dummy-pidns", 0));
+ ASSERT_SUCCESS(fsconfig(fsfd, FSCONFIG_CMD_RECONFIGURE, NULL, NULL, 0));
+
+ int procfs_pidns2 = ioctl(new_mountfd, PROCFS_GET_PID_NAMESPACE);
+ ASSERT_GE(procfs_pidns2, 0);
+
+ ASSERT_NE(self->dummy_pidns, procfs_pidns2);
+ ASSERT_FALSE(is_same_inode(self->host_pidns, procfs_pidns2));
+ ASSERT_TRUE(is_same_inode(self->dummy_pidns, procfs_pidns2));
+
+ ASSERT_SUCCESS(close(fsfd));
+ ASSERT_SUCCESS(close(new_mountfd));
+ ASSERT_SUCCESS(close(procfs_pidns1));
+ ASSERT_SUCCESS(close(procfs_pidns2));
+}
+
+TEST_HARNESS_MAIN
--
2.50.0
^ permalink raw reply related
* Re: [PATCH RFC 0/4] procfs: make reference pidns more user-visible
From: Andy Lutomirski @ 2025-07-21 14:54 UTC (permalink / raw)
To: Aleksa Sarai
Cc: Alexander Viro, Christian Brauner, Jan Kara, Jonathan Corbet,
Shuah Khan, linux-kernel, linux-fsdevel, linux-api, linux-doc,
linux-kselftest
In-Reply-To: <20250721-procfs-pidns-api-v1-0-5cd9007e512d@cyphar.com>
On Mon, Jul 21, 2025 at 1:44 AM Aleksa Sarai <cyphar@cyphar.com> wrote:
>
> Ever since the introduction of pid namespaces, procfs has had very
> implicit behaviour surrounding them (the pidns used by a procfs mount is
> auto-selected based on the mounting process's active pidns, and the
> pidns itself is basically hidden once the mount has been constructed).
> This has historically meant that userspace was required to do some
> special dances in order to configure the pidns of a procfs mount as
> desired. Examples include:
>
> * In order to bypass the mnt_too_revealing() check, Kubernetes creates
> a procfs mount from an empty pidns so that user namespaced containers
> can be nested (without this, the nested containers would fail to
> mount procfs). But this requires forking off a helper process because
> you cannot just one-shot this using mount(2).
>
> * Container runtimes in general need to fork into a container before
> configuring its mounts, which can lead to security issues in the case
> of shared-pidns containers (a privileged process in the pidns can
> interact with your container runtime process). While
> SUID_DUMP_DISABLE and user namespaces make this less of an issue, the
> strict need for this due to a minor uAPI wart is kind of unfortunate.
>
> Things would be much easier if there was a way for userspace to just
> specify the pidns they want. Patch 1 implements a new "pidns" argument
> which can be set using fsconfig(2):
>
> fsconfig(procfd, FSCONFIG_SET_FD, "pidns", NULL, nsfd);
> fsconfig(procfd, FSCONFIG_SET_STRING, "pidns", "/proc/self/ns/pid", 0);
>
> or classic mount(2) / mount(8):
>
> // mount -t proc -o pidns=/proc/self/ns/pid proc /tmp/proc
> mount("proc", "/tmp/proc", "proc", MS_..., "pidns=/proc/self/ns/pid");
>
> The initial security model I have in this RFC is to be as conservative
> as possible and just mirror the security model for setns(2) -- which
> means that you can only set pidns=... to pid namespaces that your
> current pid namespace is a direct ancestor of. This fulfils the
> requirements of container runtimes, but I suspect that this may be too
> strict for some usecases.
>
> The pidns argument is not displayed in mountinfo -- it's not clear to me
> what value it would make sense to show (maybe we could just use ns_dname
> to provide an identifier for the namespace, but this number would be
> fairly useless to userspace). I'm open to suggestions.
>
> In addition, being able to figure out what pid namespace is being used
> by a procfs mount is quite useful when you have an administrative
> process (such as a container runtime) which wants to figure out the
> correct way of mapping PIDs between its own namespace and the namespace
> for procfs (using NS_GET_{PID,TGID}_{IN,FROM}_PIDNS). There are
> alternative ways to do this, but they all rely on ancillary information
> that third-party libraries and tools do not necessarily have access to.
>
> To make this easier, add a new ioctl (PROCFS_GET_PID_NAMESPACE) which
> can be used to get a reference to the pidns that a procfs is using.
>
> It's not quite clear what is the correct security model for this API,
> but the current approach I've taken is to:
>
> * Make the ioctl only valid on the root (meaning that a process without
> access to the procfs root -- such as only having an fd to a procfs
> file or some open_tree(2)-like subset -- cannot use this API).
>
> * Require that the process requesting either has access to
> /proc/1/ns/pid anyway (i.e. has ptrace-read access to the pidns
> pid1), has CAP_SYS_ADMIN access to the pidns (i.e. has administrative
> access to it and can join it if they had a handle), or is in a pidns
> that is a direct ancestor of the target pidns (i.e. all of the pids
> are already visible in the procfs for the current process's pidns).
What's the motivation for the ptrace-read option? While I don't see
an attack off the top of my head, it seems like creating a procfs
mount may give write-ish access to things in the pidns (because the
creator is likely to have CAP_DAC_OVERRIDE, etc) and possibly even
access to namespace-wide things that aren't inherently visible to
PID1.
Even the ancestor check seems dicey. Imagine that uid 1000 makes an
unprivileged container complete with a userns. Then uid 1001 (outside
the container) makes its own userns and mountns but stays in the init
pidns and then mounts (and owns, with all filesystem-related
capabilities) that mount. Is this really safe?
CAP_SYS_ADMIN seems about right.
--Andy
^ permalink raw reply
* Re: [PATCH RFC 0/4] procfs: make reference pidns more user-visible
From: Aleksa Sarai @ 2025-07-21 15:19 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Alexander Viro, Christian Brauner, Jan Kara, Jonathan Corbet,
Shuah Khan, linux-kernel, linux-fsdevel, linux-api, linux-doc,
linux-kselftest
In-Reply-To: <CALCETrVo+Mdj7as2R0R+FqTBbjqwTkXu5Zkj=dg8EVM9xRhBPw@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 6114 bytes --]
On 2025-07-21, Andy Lutomirski <luto@amacapital.net> wrote:
> On Mon, Jul 21, 2025 at 1:44 AM Aleksa Sarai <cyphar@cyphar.com> wrote:
> >
> > Ever since the introduction of pid namespaces, procfs has had very
> > implicit behaviour surrounding them (the pidns used by a procfs mount is
> > auto-selected based on the mounting process's active pidns, and the
> > pidns itself is basically hidden once the mount has been constructed).
> > This has historically meant that userspace was required to do some
> > special dances in order to configure the pidns of a procfs mount as
> > desired. Examples include:
> >
> > * In order to bypass the mnt_too_revealing() check, Kubernetes creates
> > a procfs mount from an empty pidns so that user namespaced containers
> > can be nested (without this, the nested containers would fail to
> > mount procfs). But this requires forking off a helper process because
> > you cannot just one-shot this using mount(2).
> >
> > * Container runtimes in general need to fork into a container before
> > configuring its mounts, which can lead to security issues in the case
> > of shared-pidns containers (a privileged process in the pidns can
> > interact with your container runtime process). While
> > SUID_DUMP_DISABLE and user namespaces make this less of an issue, the
> > strict need for this due to a minor uAPI wart is kind of unfortunate.
> >
> > Things would be much easier if there was a way for userspace to just
> > specify the pidns they want. Patch 1 implements a new "pidns" argument
> > which can be set using fsconfig(2):
> >
> > fsconfig(procfd, FSCONFIG_SET_FD, "pidns", NULL, nsfd);
> > fsconfig(procfd, FSCONFIG_SET_STRING, "pidns", "/proc/self/ns/pid", 0);
> >
> > or classic mount(2) / mount(8):
> >
> > // mount -t proc -o pidns=/proc/self/ns/pid proc /tmp/proc
> > mount("proc", "/tmp/proc", "proc", MS_..., "pidns=/proc/self/ns/pid");
> >
> > The initial security model I have in this RFC is to be as conservative
> > as possible and just mirror the security model for setns(2) -- which
> > means that you can only set pidns=... to pid namespaces that your
> > current pid namespace is a direct ancestor of. This fulfils the
> > requirements of container runtimes, but I suspect that this may be too
> > strict for some usecases.
> >
> > The pidns argument is not displayed in mountinfo -- it's not clear to me
> > what value it would make sense to show (maybe we could just use ns_dname
> > to provide an identifier for the namespace, but this number would be
> > fairly useless to userspace). I'm open to suggestions.
> >
> > In addition, being able to figure out what pid namespace is being used
> > by a procfs mount is quite useful when you have an administrative
> > process (such as a container runtime) which wants to figure out the
> > correct way of mapping PIDs between its own namespace and the namespace
> > for procfs (using NS_GET_{PID,TGID}_{IN,FROM}_PIDNS). There are
> > alternative ways to do this, but they all rely on ancillary information
> > that third-party libraries and tools do not necessarily have access to.
> >
> > To make this easier, add a new ioctl (PROCFS_GET_PID_NAMESPACE) which
> > can be used to get a reference to the pidns that a procfs is using.
> >
> > It's not quite clear what is the correct security model for this API,
> > but the current approach I've taken is to:
> >
> > * Make the ioctl only valid on the root (meaning that a process without
> > access to the procfs root -- such as only having an fd to a procfs
> > file or some open_tree(2)-like subset -- cannot use this API).
> >
> > * Require that the process requesting either has access to
> > /proc/1/ns/pid anyway (i.e. has ptrace-read access to the pidns
> > pid1), has CAP_SYS_ADMIN access to the pidns (i.e. has administrative
> > access to it and can join it if they had a handle), or is in a pidns
> > that is a direct ancestor of the target pidns (i.e. all of the pids
> > are already visible in the procfs for the current process's pidns).
>
> What's the motivation for the ptrace-read option? While I don't see
> an attack off the top of my head, it seems like creating a procfs
> mount may give write-ish access to things in the pidns (because the
> creator is likely to have CAP_DAC_OVERRIDE, etc) and possibly even
> access to namespace-wide things that aren't inherently visible to
> PID1.
This latter section is about the privilege model for
ioctl(PROCFS_GET_PID_NAMESPACE), not the pidns= mount flag. pidns=
requires CAP_SYS_ADMIN for pidns->user_ns, in addition to the same
restrictions as pidns_install() (must be a direct ancestor). Maybe I
should add some headers in this cover letter for v2...
For the ioctl -- if the user can ptrace-read pid1 in the pidns, they can
open a handle to /proc/1/ns/pid which is exactly the same thing they'd
get from PROCFS_GET_PID_NAMESPACE.
> Even the ancestor check seems dicey. Imagine that uid 1000 makes an
> unprivileged container complete with a userns. Then uid 1001 (outside
> the container) makes its own userns and mountns but stays in the init
> pidns and then mounts (and owns, with all filesystem-related
> capabilities) that mount. Is this really safe?
As for the ancestor check (for the ioctl), the logic I had was that
being in an ancestor pidns means that you already can see all of the
subprocesses in your own pidns, so it seems strange to not be able to
get a handle to their pidns. Maybe this isn't quite right, idk.
Ultimately there isn't too much you can do with a pidns fd if you don't
have privileges to join it (the only thing I can think of is that you
could bind-mount it, which could maybe be used to trick an
administrative process if they trusted your mountns for some reason).
> CAP_SYS_ADMIN seems about right.
For pidns=, sure. For the ioctl, I think this is overkill.
--
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
https://www.cyphar.com/
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* [PATCH RFC v2 0/4] procfs: make reference pidns more user-visible
From: Aleksa Sarai @ 2025-07-22 23:18 UTC (permalink / raw)
To: Alexander Viro, Christian Brauner, Jan Kara, Jonathan Corbet,
Shuah Khan
Cc: linux-kernel, linux-fsdevel, linux-api, linux-doc,
linux-kselftest, Aleksa Sarai
Ever since the introduction of pid namespaces, procfs has had very
implicit behaviour surrounding them (the pidns used by a procfs mount is
auto-selected based on the mounting process's active pidns, and the
pidns itself is basically hidden once the mount has been constructed).
/* pidns mount option for procfs */
This implicit behaviour has historically meant that userspace was
required to do some special dances in order to configure the pidns of a
procfs mount as desired. Examples include:
* In order to bypass the mnt_too_revealing() check, Kubernetes creates
a procfs mount from an empty pidns so that user namespaced containers
can be nested (without this, the nested containers would fail to
mount procfs). But this requires forking off a helper process because
you cannot just one-shot this using mount(2).
* Container runtimes in general need to fork into a container before
configuring its mounts, which can lead to security issues in the case
of shared-pidns containers (a privileged process in the pidns can
interact with your container runtime process). While
SUID_DUMP_DISABLE and user namespaces make this less of an issue, the
strict need for this due to a minor uAPI wart is kind of unfortunate.
Things would be much easier if there was a way for userspace to just
specify the pidns they want. Patch 1 implements a new "pidns" argument
which can be set using fsconfig(2):
fsconfig(procfd, FSCONFIG_SET_FD, "pidns", NULL, nsfd);
fsconfig(procfd, FSCONFIG_SET_STRING, "pidns", "/proc/self/ns/pid", 0);
or classic mount(2) / mount(8):
// mount -t proc -o pidns=/proc/self/ns/pid proc /tmp/proc
mount("proc", "/tmp/proc", "proc", MS_..., "pidns=/proc/self/ns/pid");
The initial security model I have in this RFC is to be as conservative
as possible and just mirror the security model for setns(2) -- which
means that you can only set pidns=... to pid namespaces that your
current pid namespace is a direct ancestor of and you have CAP_SYS_ADMIN
privileges over the pid namespace. This fulfils the requirements of
container runtimes, but I suspect that this may be too strict for some
usecases.
The pidns argument is not displayed in mountinfo -- it's not clear to me
what value it would make sense to show (maybe we could just use ns_dname
to provide an identifier for the namespace, but this number would be
fairly useless to userspace). I'm open to suggestions. Note that
PROCFS_GET_PID_NAMESPACE (see below) does at least let userspace get
information about this outside of mountinfo.
/* ioctl(PROCFS_GET_PID_NAMESPACE) */
In addition, being able to figure out what pid namespace is being used
by a procfs mount is quite useful when you have an administrative
process (such as a container runtime) which wants to figure out the
correct way of mapping PIDs between its own namespace and the namespace
for procfs (using NS_GET_{PID,TGID}_{IN,FROM}_PIDNS). There are
alternative ways to do this, but they all rely on ancillary information
that third-party libraries and tools do not necessarily have access to.
To make this easier, add a new ioctl (PROCFS_GET_PID_NAMESPACE) which
can be used to get a reference to the pidns that a procfs is using.
It's not quite clear what is the correct security model for this API,
but the current approach I've taken is to:
* Make the ioctl only valid on the root (meaning that a process without
access to the procfs root -- such as only having an fd to a procfs
file or some open_tree(2)-like subset -- cannot use this API).
* Require that the process requesting either has access to
/proc/1/ns/pid anyway (i.e. has ptrace-read access to the pidns
pid1), has CAP_SYS_ADMIN access to the pidns (i.e. has administrative
access to it and can join it if they had a handle), or is in a pidns
that is a direct ancestor of the target pidns (i.e. all of the pids
are already visible in the procfs for the current process's pidns).
The security model for this is a little loose, as it seems to me that
all of the cases mentioned are valid cases to allow access, but I'm open
to suggestions for whether we need to make this stricter or looser.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
---
Changes in v2:
- #ifdef CONFIG_PID_NS
- Improve cover letter wording to make it clear we're talking about two
separate features with different permission models. [Andy Lutomirski]
- Fix build warnings in pidns_is_ancestor() patch. [kernel test robot]
- v1: <https://lore.kernel.org/r/20250721-procfs-pidns-api-v1-0-5cd9007e512d@cyphar.com>
---
Aleksa Sarai (4):
pidns: move is-ancestor logic to helper
procfs: add "pidns" mount option
procfs: add PROCFS_GET_PID_NAMESPACE ioctl
selftests/proc: add tests for new pidns APIs
Documentation/filesystems/proc.rst | 10 ++
fs/proc/root.c | 144 ++++++++++++++-
include/linux/pid_namespace.h | 9 +
include/uapi/linux/fs.h | 3 +
kernel/pid_namespace.c | 23 ++-
tools/testing/selftests/proc/.gitignore | 1 +
tools/testing/selftests/proc/Makefile | 1 +
tools/testing/selftests/proc/proc-pidns.c | 286 ++++++++++++++++++++++++++++++
8 files changed, 461 insertions(+), 16 deletions(-)
---
base-commit: 4c838c7672c39ec6ec48456c6ce22d14a68f4cda
change-id: 20250717-procfs-pidns-api-8ed1583431f0
Best regards,
--
Aleksa Sarai <cyphar@cyphar.com>
^ permalink raw reply
* [PATCH RFC v2 1/4] pidns: move is-ancestor logic to helper
From: Aleksa Sarai @ 2025-07-22 23:18 UTC (permalink / raw)
To: Alexander Viro, Christian Brauner, Jan Kara, Jonathan Corbet,
Shuah Khan
Cc: linux-kernel, linux-fsdevel, linux-api, linux-doc,
linux-kselftest, Aleksa Sarai
In-Reply-To: <20250723-procfs-pidns-api-v2-0-621e7edd8e40@cyphar.com>
This check will be needed in later patches, and there's no point
open-coding it each time.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
---
include/linux/pid_namespace.h | 9 +++++++++
kernel/pid_namespace.c | 23 +++++++++++++++--------
2 files changed, 24 insertions(+), 8 deletions(-)
diff --git a/include/linux/pid_namespace.h b/include/linux/pid_namespace.h
index 7c67a5811199..17fdc059f8da 100644
--- a/include/linux/pid_namespace.h
+++ b/include/linux/pid_namespace.h
@@ -84,6 +84,9 @@ extern void zap_pid_ns_processes(struct pid_namespace *pid_ns);
extern int reboot_pid_ns(struct pid_namespace *pid_ns, int cmd);
extern void put_pid_ns(struct pid_namespace *ns);
+extern bool pidns_is_ancestor(struct pid_namespace *child,
+ struct pid_namespace *ancestor);
+
#else /* !CONFIG_PID_NS */
#include <linux/err.h>
@@ -118,6 +121,12 @@ static inline int reboot_pid_ns(struct pid_namespace *pid_ns, int cmd)
{
return 0;
}
+
+static inline bool pidns_is_ancestor(struct pid_namespace *child,
+ struct pid_namespace *ancestor)
+{
+ return false;
+}
#endif /* CONFIG_PID_NS */
extern struct pid_namespace *task_active_pid_ns(struct task_struct *tsk);
diff --git a/kernel/pid_namespace.c b/kernel/pid_namespace.c
index 7098ed44e717..c2783c5fa90b 100644
--- a/kernel/pid_namespace.c
+++ b/kernel/pid_namespace.c
@@ -390,11 +390,24 @@ static void pidns_put(struct ns_common *ns)
put_pid_ns(to_pid_ns(ns));
}
+bool pidns_is_ancestor(struct pid_namespace *child,
+ struct pid_namespace *ancestor)
+{
+ struct pid_namespace *ns;
+
+ if (child->level < ancestor->level)
+ return false;
+ for (ns = child; ns->level > ancestor->level; ns = ns->parent)
+ ;
+ return ns == ancestor;
+}
+EXPORT_SYMBOL_GPL(pidns_is_ancestor);
+
static int pidns_install(struct nsset *nsset, struct ns_common *ns)
{
struct nsproxy *nsproxy = nsset->nsproxy;
struct pid_namespace *active = task_active_pid_ns(current);
- struct pid_namespace *ancestor, *new = to_pid_ns(ns);
+ struct pid_namespace *new = to_pid_ns(ns);
if (!ns_capable(new->user_ns, CAP_SYS_ADMIN) ||
!ns_capable(nsset->cred->user_ns, CAP_SYS_ADMIN))
@@ -408,13 +421,7 @@ static int pidns_install(struct nsset *nsset, struct ns_common *ns)
* this maintains the property that processes and their
* children can not escape their current pid namespace.
*/
- if (new->level < active->level)
- return -EINVAL;
-
- ancestor = new;
- while (ancestor->level > active->level)
- ancestor = ancestor->parent;
- if (ancestor != active)
+ if (!pidns_is_ancestor(new, active))
return -EINVAL;
put_pid_ns(nsproxy->pid_ns_for_children);
--
2.50.0
^ permalink raw reply related
* [PATCH RFC v2 2/4] procfs: add "pidns" mount option
From: Aleksa Sarai @ 2025-07-22 23:18 UTC (permalink / raw)
To: Alexander Viro, Christian Brauner, Jan Kara, Jonathan Corbet,
Shuah Khan
Cc: linux-kernel, linux-fsdevel, linux-api, linux-doc,
linux-kselftest, Aleksa Sarai
In-Reply-To: <20250723-procfs-pidns-api-v2-0-621e7edd8e40@cyphar.com>
Since the introduction of pid namespaces, their interaction with procfs
has been entirely implicit in ways that require a lot of dancing around
by programs that need to construct sandboxes with different PID
namespaces.
Being able to explicitly specify the pid namespace to use when
constructing a procfs super block will allow programs to no longer need
to fork off a process which does then does unshare(2) / setns(2) and
forks again in order to construct a procfs in a pidns.
So, provide a "pidns" mount option which allows such users to just
explicitly state which pid namespace they want that procfs instance to
use. This interface can be used with fsconfig(2) either with a file
descriptor or a path:
fsconfig(procfd, FSCONFIG_SET_FD, "pidns", NULL, nsfd);
fsconfig(procfd, FSCONFIG_SET_STRING, "pidns", "/proc/self/ns/pid", 0);
or with classic mount(2) / mount(8):
// mount -t proc -o pidns=/proc/self/ns/pid proc /tmp/proc
mount("proc", "/tmp/proc", "proc", MS_..., "pidns=/proc/self/ns/pid");
As this new API is effectively shorthand for setns(2) followed by
mount(2), the permission model for this mirrors pidns_install() to avoid
opening up new attack surfaces by loosening the existing permission
model.
Note that the mount infrastructure also allows userspace to reconfigure
the pidns of an existing procfs mount, which may or may not be useful to
some users.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
---
Documentation/filesystems/proc.rst | 6 +++
fs/proc/root.c | 90 +++++++++++++++++++++++++++++++++++---
2 files changed, 90 insertions(+), 6 deletions(-)
diff --git a/Documentation/filesystems/proc.rst b/Documentation/filesystems/proc.rst
index 5236cb52e357..c520b9f8a3fd 100644
--- a/Documentation/filesystems/proc.rst
+++ b/Documentation/filesystems/proc.rst
@@ -2360,6 +2360,7 @@ The following mount options are supported:
hidepid= Set /proc/<pid>/ access mode.
gid= Set the group authorized to learn processes information.
subset= Show only the specified subset of procfs.
+ pidns= Specify a the namespace used by this procfs.
========= ========================================================
hidepid=off or hidepid=0 means classic mode - everybody may access all
@@ -2392,6 +2393,11 @@ information about processes information, just add identd to this group.
subset=pid hides all top level files and directories in the procfs that
are not related to tasks.
+pidns= specifies a pid namespace (either as a string path to something like
+`/proc/$pid/ns/pid`, or a file descriptor when using `FSCONFIG_SET_FD`) that
+will be used by the procfs instance when translating pids. By default, procfs
+will use the calling process's active pid namespace.
+
Chapter 5: Filesystem behavior
==============================
diff --git a/fs/proc/root.c b/fs/proc/root.c
index ed86ac710384..057c8a125c6e 100644
--- a/fs/proc/root.c
+++ b/fs/proc/root.c
@@ -38,12 +38,18 @@ enum proc_param {
Opt_gid,
Opt_hidepid,
Opt_subset,
+#ifdef CONFIG_PID_NS
+ Opt_pidns,
+#endif
};
static const struct fs_parameter_spec proc_fs_parameters[] = {
- fsparam_u32("gid", Opt_gid),
+ fsparam_u32("gid", Opt_gid),
fsparam_string("hidepid", Opt_hidepid),
fsparam_string("subset", Opt_subset),
+#ifdef CONFIG_PID_NS
+ fsparam_file_or_string("pidns", Opt_pidns),
+#endif
{}
};
@@ -109,11 +115,67 @@ static int proc_parse_subset_param(struct fs_context *fc, char *value)
return 0;
}
+#ifdef CONFIG_PID_NS
+static int proc_parse_pidns_param(struct fs_context *fc,
+ struct fs_parameter *param,
+ struct fs_parse_result *result)
+{
+ struct proc_fs_context *ctx = fc->fs_private;
+ struct pid_namespace *target, *active = task_active_pid_ns(current);
+ struct ns_common *ns;
+ struct file *ns_filp __free(fput) = NULL;
+
+ switch (param->type) {
+ case fs_value_is_file:
+ /* came throug fsconfig, steal the file reference */
+ ns_filp = param->file;
+ param->file = NULL;
+ break;
+ case fs_value_is_string:
+ ns_filp = filp_open(param->string, O_RDONLY, 0);
+ break;
+ default:
+ WARN_ON_ONCE(true);
+ break;
+ }
+ if (!ns_filp)
+ ns_filp = ERR_PTR(-EBADF);
+ if (IS_ERR(ns_filp)) {
+ errorfc(fc, "could not get file from pidns argument");
+ return PTR_ERR(ns_filp);
+ }
+
+ if (!proc_ns_file(ns_filp))
+ return invalfc(fc, "pidns argument is not an nsfs file");
+ ns = get_proc_ns(file_inode(ns_filp));
+ if (ns->ops->type != CLONE_NEWPID)
+ return invalfc(fc, "pidns argument is not a pidns file");
+ target = container_of(ns, struct pid_namespace, ns);
+
+ /*
+ * pidns= is shorthand for joining the pidns to get a fsopen fd, so the
+ * permission model should be the same as pidns_install().
+ */
+ if (!ns_capable(target->user_ns, CAP_SYS_ADMIN)) {
+ errorfc(fc, "insufficient permissions to set pidns");
+ return -EPERM;
+ }
+ if (!pidns_is_ancestor(target, active))
+ return invalfc(fc, "cannot set pidns to non-descendant pidns");
+
+ put_pid_ns(ctx->pid_ns);
+ ctx->pid_ns = get_pid_ns(target);
+ put_user_ns(fc->user_ns);
+ fc->user_ns = get_user_ns(ctx->pid_ns->user_ns);
+ return 0;
+}
+#endif /* CONFIG_PID_NS */
+
static int proc_parse_param(struct fs_context *fc, struct fs_parameter *param)
{
struct proc_fs_context *ctx = fc->fs_private;
struct fs_parse_result result;
- int opt;
+ int opt, err;
opt = fs_parse(fc, proc_fs_parameters, param, &result);
if (opt < 0)
@@ -125,14 +187,24 @@ static int proc_parse_param(struct fs_context *fc, struct fs_parameter *param)
break;
case Opt_hidepid:
- if (proc_parse_hidepid_param(fc, param))
- return -EINVAL;
+ err = proc_parse_hidepid_param(fc, param);
+ if (err)
+ return err;
break;
case Opt_subset:
- if (proc_parse_subset_param(fc, param->string) < 0)
- return -EINVAL;
+ err = proc_parse_subset_param(fc, param->string);
+ if (err)
+ return err;
+ break;
+
+#ifdef CONFIG_PID_NS
+ case Opt_pidns:
+ err = proc_parse_pidns_param(fc, param, &result);
+ if (err)
+ return err;
break;
+#endif
default:
return -EINVAL;
@@ -154,6 +226,12 @@ static void proc_apply_options(struct proc_fs_info *fs_info,
fs_info->hide_pid = ctx->hidepid;
if (ctx->mask & (1 << Opt_subset))
fs_info->pidonly = ctx->pidonly;
+#ifdef CONFIG_PID_NS
+ if (ctx->mask & (1 << Opt_pidns)) {
+ put_pid_ns(fs_info->pid_ns);
+ fs_info->pid_ns = get_pid_ns(ctx->pid_ns);
+ }
+#endif
}
static int proc_fill_super(struct super_block *s, struct fs_context *fc)
--
2.50.0
^ permalink raw reply related
* [PATCH RFC v2 3/4] procfs: add PROCFS_GET_PID_NAMESPACE ioctl
From: Aleksa Sarai @ 2025-07-22 23:18 UTC (permalink / raw)
To: Alexander Viro, Christian Brauner, Jan Kara, Jonathan Corbet,
Shuah Khan
Cc: linux-kernel, linux-fsdevel, linux-api, linux-doc,
linux-kselftest, Aleksa Sarai
In-Reply-To: <20250723-procfs-pidns-api-v2-0-621e7edd8e40@cyphar.com>
/proc has historically had very opaque semantics about PID namespaces,
which is a little unfortunate for container runtimes and other programs
that deal with switching namespaces very often. One common issue is that
of converting between PIDs in the process's namespace and PIDs in the
namespace of /proc.
In principle, it is possible to do this today by opening a pidfd with
pidfd_open(2) and then looking at /proc/self/fdinfo/$n (which will
contain a PID value translated to the pid namespace associated with that
procfs superblock). However, allocating a new file for each PID to be
converted is less than ideal for programs that may need to scan procfs,
and it is generally useful for userspace to be able to finally get this
information from procfs.
So, add a new API for this in the form of an ioctl(2) you can call on
the root directory of procfs. The returned file descriptor will have
O_CLOEXEC set. This acts as a sister feature to the new "pidns" mount
option, finally allowing userspace full control of the pid namespaces
associated with procfs instances.
The permission model for this is a bit looser than that of the "pidns"
mount option, but this is mainly because /proc/1/ns/pid provides the
same information, so as long as you have access to that magic-link (or
something equivalently reasonable such as privileges with CAP_SYS_ADMIN
or being in an ancestor pid namespace) it makes sense to allow userspace
to grab a handle. setns(2) will still have their own permission checks,
so being able to open a pidns handle doesn't really provide too many
other capabilities.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
---
Documentation/filesystems/proc.rst | 4 +++
fs/proc/root.c | 54 ++++++++++++++++++++++++++++++++++++--
include/uapi/linux/fs.h | 3 +++
3 files changed, 59 insertions(+), 2 deletions(-)
diff --git a/Documentation/filesystems/proc.rst b/Documentation/filesystems/proc.rst
index c520b9f8a3fd..506383273c9d 100644
--- a/Documentation/filesystems/proc.rst
+++ b/Documentation/filesystems/proc.rst
@@ -2398,6 +2398,10 @@ pidns= specifies a pid namespace (either as a string path to something like
will be used by the procfs instance when translating pids. By default, procfs
will use the calling process's active pid namespace.
+Processes can check which pid namespace is used by a procfs instance by using
+the `PROCFS_GET_PID_NAMESPACE` ioctl() on the root directory of the procfs
+instance.
+
Chapter 5: Filesystem behavior
==============================
diff --git a/fs/proc/root.c b/fs/proc/root.c
index 057c8a125c6e..548a57ec2152 100644
--- a/fs/proc/root.c
+++ b/fs/proc/root.c
@@ -23,8 +23,10 @@
#include <linux/cred.h>
#include <linux/magic.h>
#include <linux/slab.h>
+#include <linux/ptrace.h>
#include "internal.h"
+#include "../internal.h"
struct proc_fs_context {
struct pid_namespace *pid_ns;
@@ -418,15 +420,63 @@ static int proc_root_readdir(struct file *file, struct dir_context *ctx)
return proc_pid_readdir(file, ctx);
}
+static long int proc_root_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
+{
+ switch (cmd) {
+#ifdef CONFIG_PID_NS
+ case PROCFS_GET_PID_NAMESPACE: {
+ struct pid_namespace *active = task_active_pid_ns(current);
+ struct pid_namespace *ns = proc_pid_ns(file_inode(filp)->i_sb);
+ bool can_access_pidns = false;
+
+ /*
+ * If we are in an ancestors of the pidns, or have join
+ * privileges (CAP_SYS_ADMIN), then it makes sense that we
+ * would be able to grab a handle to the pidns.
+ *
+ * Otherwise, if there is a root process, then being able to
+ * access /proc/$pid/ns/pid is equivalent to this ioctl and so
+ * we should probably match the permission model. For empty
+ * namespaces it seems unlikely for there to be a downside to
+ * allowing unprivileged users to open a handle to it (setns
+ * will fail for unprivileged users anyway).
+ */
+ can_access_pidns = pidns_is_ancestor(ns, active) ||
+ ns_capable(ns->user_ns, CAP_SYS_ADMIN);
+ if (!can_access_pidns) {
+ bool cannot_ptrace_pid1 = false;
+
+ read_lock(&tasklist_lock);
+ if (ns->child_reaper)
+ cannot_ptrace_pid1 = ptrace_may_access(ns->child_reaper,
+ PTRACE_MODE_READ_FSCREDS);
+ read_unlock(&tasklist_lock);
+ can_access_pidns = !cannot_ptrace_pid1;
+ }
+ if (!can_access_pidns)
+ return -EPERM;
+
+ /* open_namespace() unconditionally consumes the reference. */
+ get_pid_ns(ns);
+ return open_namespace(to_ns_common(ns));
+ }
+#endif /* CONFIG_PID_NS */
+ default:
+ return -ENOIOCTLCMD;
+ }
+}
+
/*
* The root /proc directory is special, as it has the
* <pid> directories. Thus we don't use the generic
* directory handling functions for that..
*/
static const struct file_operations proc_root_operations = {
- .read = generic_read_dir,
- .iterate_shared = proc_root_readdir,
+ .read = generic_read_dir,
+ .iterate_shared = proc_root_readdir,
.llseek = generic_file_llseek,
+ .unlocked_ioctl = proc_root_ioctl,
+ .compat_ioctl = compat_ptr_ioctl,
};
/*
diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
index 0bd678a4a10e..aa642cb48feb 100644
--- a/include/uapi/linux/fs.h
+++ b/include/uapi/linux/fs.h
@@ -437,6 +437,9 @@ typedef int __bitwise __kernel_rwf_t;
#define PROCFS_IOCTL_MAGIC 'f'
+/* procfs root ioctls */
+#define PROCFS_GET_PID_NAMESPACE _IO(PROCFS_IOCTL_MAGIC, 1)
+
/* Pagemap ioctl */
#define PAGEMAP_SCAN _IOWR(PROCFS_IOCTL_MAGIC, 16, struct pm_scan_arg)
--
2.50.0
^ permalink raw reply related
* [PATCH RFC v2 4/4] selftests/proc: add tests for new pidns APIs
From: Aleksa Sarai @ 2025-07-22 23:18 UTC (permalink / raw)
To: Alexander Viro, Christian Brauner, Jan Kara, Jonathan Corbet,
Shuah Khan
Cc: linux-kernel, linux-fsdevel, linux-api, linux-doc,
linux-kselftest, Aleksa Sarai
In-Reply-To: <20250723-procfs-pidns-api-v2-0-621e7edd8e40@cyphar.com>
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
---
tools/testing/selftests/proc/.gitignore | 1 +
tools/testing/selftests/proc/Makefile | 1 +
tools/testing/selftests/proc/proc-pidns.c | 286 ++++++++++++++++++++++++++++++
3 files changed, 288 insertions(+)
diff --git a/tools/testing/selftests/proc/.gitignore b/tools/testing/selftests/proc/.gitignore
index 973968f45bba..2dced03e9e0e 100644
--- a/tools/testing/selftests/proc/.gitignore
+++ b/tools/testing/selftests/proc/.gitignore
@@ -17,6 +17,7 @@
/proc-tid0
/proc-uptime-001
/proc-uptime-002
+/proc-pidns
/read
/self
/setns-dcache
diff --git a/tools/testing/selftests/proc/Makefile b/tools/testing/selftests/proc/Makefile
index b12921b9794b..c6f7046b9860 100644
--- a/tools/testing/selftests/proc/Makefile
+++ b/tools/testing/selftests/proc/Makefile
@@ -27,5 +27,6 @@ TEST_GEN_PROGS += setns-sysvipc
TEST_GEN_PROGS += thread-self
TEST_GEN_PROGS += proc-multiple-procfs
TEST_GEN_PROGS += proc-fsconfig-hidepid
+TEST_GEN_PROGS += proc-pidns
include ../lib.mk
diff --git a/tools/testing/selftests/proc/proc-pidns.c b/tools/testing/selftests/proc/proc-pidns.c
new file mode 100644
index 000000000000..e7e34c78d383
--- /dev/null
+++ b/tools/testing/selftests/proc/proc-pidns.c
@@ -0,0 +1,286 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Author: Aleksa Sarai <cyphar@cyphar.com>
+ * Copyright (C) 2025 SUSE LLC.
+ */
+
+#include <assert.h>
+#include <errno.h>
+#include <sched.h>
+#include <stdbool.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <stdio.h>
+#include <sys/mount.h>
+#include <sys/stat.h>
+#include <sys/ioctl.h>
+#include <sys/prctl.h>
+
+#include "../kselftest_harness.h"
+
+#define bail(fmt, ...) \
+ do { \
+ fprintf(stderr, fmt ": %m", __VA_ARGS__); \
+ exit(1); \
+ } while (0)
+
+#define ASSERT_SUCCESS ASSERT_FALSE
+#define ASSERT_FAIL ASSERT_TRUE
+
+int touch(char *path)
+{
+ int fd = open(path, O_WRONLY|O_CREAT|O_CLOEXEC, 0644);
+ if (fd < 0 || close(fd) < 0)
+ return -errno;
+ return 0;
+}
+
+FIXTURE(ns)
+{
+ int host_mntns, host_pidns;
+ int dummy_pidns;
+};
+
+FIXTURE_SETUP(ns)
+{
+ /* Stash the old mntns. */
+ self->host_mntns = open("/proc/self/ns/mnt", O_RDONLY|O_CLOEXEC);
+ ASSERT_GE(self->host_mntns, 0);
+
+ /* Create a new mount namespace and make it private. */
+ ASSERT_SUCCESS(unshare(CLONE_NEWNS));
+ ASSERT_SUCCESS(mount(NULL, "/", NULL, MS_PRIVATE|MS_REC, NULL));
+
+ /*
+ * Create a proper tmpfs that we can use and will disappear once we
+ * leave this mntns.
+ */
+ ASSERT_SUCCESS(mount("tmpfs", "/tmp", "tmpfs", 0, NULL));
+
+ /*
+ * Create a pidns we can use for later tests. We need to fork off a
+ * child so that we get a usable nsfd that we can bind-mount and open.
+ */
+ ASSERT_SUCCESS(touch("/tmp/dummy-pidns"));
+
+ self->host_pidns = open("/proc/self/ns/pid", O_RDONLY|O_CLOEXEC);
+ ASSERT_GE(self->host_pidns, 0);
+ ASSERT_SUCCESS(unshare(CLONE_NEWPID));
+
+ pid_t pid = fork();
+ ASSERT_GE(pid, 0);
+ if (!pid) {
+ prctl(PR_SET_PDEATHSIG, SIGKILL);
+ ASSERT_SUCCESS(mount("/proc/self/ns/pid", "/tmp/dummy-pidns", NULL, MS_BIND, 0));
+ exit(0);
+ }
+
+ int wstatus;
+ ASSERT_EQ(waitpid(pid, &wstatus, 0), pid);
+ ASSERT_TRUE(WIFEXITED(wstatus));
+ ASSERT_EQ(WEXITSTATUS(wstatus), 0);
+
+ ASSERT_SUCCESS(setns(self->host_pidns, CLONE_NEWPID));
+
+ self->dummy_pidns = open("/tmp/dummy-pidns", O_RDONLY|O_CLOEXEC);
+ ASSERT_GE(self->dummy_pidns, 0);
+}
+
+FIXTURE_TEARDOWN(ns)
+{
+ ASSERT_SUCCESS(setns(self->host_mntns, CLONE_NEWNS));
+ ASSERT_SUCCESS(close(self->host_mntns));
+
+ ASSERT_SUCCESS(close(self->host_pidns));
+ ASSERT_SUCCESS(close(self->dummy_pidns));
+}
+
+TEST_F(ns, pidns_mount_string_path)
+{
+ ASSERT_SUCCESS(mkdir("/tmp/proc-host", 0755));
+ ASSERT_SUCCESS(mount("proc", "/tmp/proc-host", "proc", 0, "pidns=/proc/self/ns/pid"));
+ ASSERT_SUCCESS(access("/tmp/proc-host/self/", X_OK));
+
+ ASSERT_SUCCESS(mkdir("/tmp/proc-dummy", 0755));
+ ASSERT_SUCCESS(mount("proc", "/tmp/proc-dummy", "proc", 0, "pidns=/tmp/dummy-pidns"));
+ ASSERT_FAIL(access("/tmp/proc-dummy/1/", X_OK));
+ ASSERT_FAIL(access("/tmp/proc-dummy/self/", X_OK));
+}
+
+TEST_F(ns, pidns_fsconfig_string_path)
+{
+ int fsfd = fsopen("proc", FSOPEN_CLOEXEC);
+ ASSERT_GE(fsfd, 0);
+
+ ASSERT_SUCCESS(fsconfig(fsfd, FSCONFIG_SET_STRING, "pidns", "/tmp/dummy-pidns", 0));
+ ASSERT_SUCCESS(fsconfig(fsfd, FSCONFIG_CMD_CREATE, NULL, NULL, 0));
+
+ int mountfd = fsmount(fsfd, FSMOUNT_CLOEXEC, 0);
+ ASSERT_GE(mountfd, 0);
+
+ ASSERT_FAIL(faccessat(mountfd, "1/", X_OK, 0));
+ ASSERT_FAIL(faccessat(mountfd, "self/", X_OK, 0));
+
+ ASSERT_SUCCESS(close(fsfd));
+ ASSERT_SUCCESS(close(mountfd));
+}
+
+TEST_F(ns, pidns_fsconfig_fd)
+{
+ int fsfd = fsopen("proc", FSOPEN_CLOEXEC);
+ ASSERT_GE(fsfd, 0);
+
+ ASSERT_SUCCESS(fsconfig(fsfd, FSCONFIG_SET_FD, "pidns", NULL, self->dummy_pidns));
+ ASSERT_SUCCESS(fsconfig(fsfd, FSCONFIG_CMD_CREATE, NULL, NULL, 0));
+
+ int mountfd = fsmount(fsfd, FSMOUNT_CLOEXEC, 0);
+ ASSERT_GE(mountfd, 0);
+
+ ASSERT_FAIL(faccessat(mountfd, "1/", X_OK, 0));
+ ASSERT_FAIL(faccessat(mountfd, "self/", X_OK, 0));
+
+ ASSERT_SUCCESS(close(fsfd));
+ ASSERT_SUCCESS(close(mountfd));
+}
+
+TEST_F(ns, pidns_reconfigure_remount)
+{
+ ASSERT_SUCCESS(mkdir("/tmp/proc", 0755));
+ ASSERT_SUCCESS(mount("proc", "/tmp/proc", "proc", 0, ""));
+ ASSERT_SUCCESS(access("/tmp/proc/self/", X_OK));
+
+ ASSERT_SUCCESS(mount(NULL, "/tmp/proc", NULL, MS_REMOUNT, "pidns=/tmp/dummy-pidns"));
+ ASSERT_FAIL(access("/tmp/proc/self/", X_OK));
+}
+
+TEST_F(ns, pidns_reconfigure_fsconfig_string_path)
+{
+ int fsfd = fsopen("proc", FSOPEN_CLOEXEC);
+ ASSERT_GE(fsfd, 0);
+
+ ASSERT_SUCCESS(fsconfig(fsfd, FSCONFIG_CMD_CREATE, NULL, NULL, 0));
+
+ int mountfd = fsmount(fsfd, FSMOUNT_CLOEXEC, 0);
+ ASSERT_GE(mountfd, 0);
+
+ ASSERT_SUCCESS(faccessat(mountfd, "1/", X_OK, 0));
+ ASSERT_SUCCESS(faccessat(mountfd, "self/", X_OK, 0));
+
+ ASSERT_SUCCESS(fsconfig(fsfd, FSCONFIG_SET_STRING, "pidns", "/tmp/dummy-pidns", 0));
+ ASSERT_SUCCESS(fsconfig(fsfd, FSCONFIG_CMD_RECONFIGURE, NULL, NULL, 0));
+
+ ASSERT_FAIL(faccessat(mountfd, "1/", X_OK, 0));
+ ASSERT_FAIL(faccessat(mountfd, "self/", X_OK, 0));
+
+ ASSERT_SUCCESS(close(fsfd));
+ ASSERT_SUCCESS(close(mountfd));
+}
+
+TEST_F(ns, pidns_reconfigure_fsconfig_fd)
+{
+ int fsfd = fsopen("proc", FSOPEN_CLOEXEC);
+ ASSERT_GE(fsfd, 0);
+
+ ASSERT_SUCCESS(fsconfig(fsfd, FSCONFIG_CMD_CREATE, NULL, NULL, 0));
+
+ int mountfd = fsmount(fsfd, FSMOUNT_CLOEXEC, 0);
+ ASSERT_GE(mountfd, 0);
+
+ ASSERT_SUCCESS(faccessat(mountfd, "1/", X_OK, 0));
+ ASSERT_SUCCESS(faccessat(mountfd, "self/", X_OK, 0));
+
+ ASSERT_SUCCESS(fsconfig(fsfd, FSCONFIG_SET_FD, "pidns", NULL, self->dummy_pidns));
+ ASSERT_SUCCESS(fsconfig(fsfd, FSCONFIG_CMD_RECONFIGURE, NULL, NULL, 0));
+
+ ASSERT_FAIL(faccessat(mountfd, "1/", X_OK, 0));
+ ASSERT_FAIL(faccessat(mountfd, "self/", X_OK, 0));
+
+ ASSERT_SUCCESS(close(fsfd));
+ ASSERT_SUCCESS(close(mountfd));
+}
+
+int is_same_inode(int fd1, int fd2)
+{
+ struct stat stat1, stat2;
+
+ assert(fstat(fd1, &stat1) == 0);
+ assert(fstat(fd2, &stat2) == 0);
+
+ return stat1.st_ino == stat2.st_ino && stat1.st_dev == stat2.st_dev;
+}
+
+#define PROCFS_IOCTL_MAGIC 'f'
+#define PROCFS_GET_PID_NAMESPACE _IO(PROCFS_IOCTL_MAGIC, 1)
+
+TEST_F(ns, get_pidns_ioctl)
+{
+ int fsfd = fsopen("proc", FSOPEN_CLOEXEC);
+ ASSERT_GE(fsfd, 0);
+
+ ASSERT_SUCCESS(fsconfig(fsfd, FSCONFIG_SET_FD, "pidns", NULL, self->dummy_pidns));
+ ASSERT_SUCCESS(fsconfig(fsfd, FSCONFIG_CMD_CREATE, NULL, NULL, 0));
+
+ int mountfd = fsmount(fsfd, FSMOUNT_CLOEXEC, 0);
+ ASSERT_GE(mountfd, 0);
+
+ /* fsmount returns an O_PATH, which ioctl(2) doesn't accept. */
+ int new_mountfd = openat(mountfd, ".", O_RDONLY|O_DIRECTORY|O_CLOEXEC);
+ ASSERT_GE(new_mountfd, 0);
+
+ ASSERT_SUCCESS(close(mountfd));
+ mountfd = -EBADF;
+
+ int procfs_pidns = ioctl(new_mountfd, PROCFS_GET_PID_NAMESPACE);
+ ASSERT_GE(procfs_pidns, 0);
+
+ ASSERT_NE(self->dummy_pidns, procfs_pidns);
+ ASSERT_FALSE(is_same_inode(self->host_pidns, procfs_pidns));
+ ASSERT_TRUE(is_same_inode(self->dummy_pidns, procfs_pidns));
+
+ ASSERT_SUCCESS(close(fsfd));
+ ASSERT_SUCCESS(close(new_mountfd));
+ ASSERT_SUCCESS(close(procfs_pidns));
+}
+
+TEST_F(ns, reconfigure_get_pidns_ioctl)
+{
+ int fsfd = fsopen("proc", FSOPEN_CLOEXEC);
+ ASSERT_GE(fsfd, 0);
+
+ ASSERT_SUCCESS(fsconfig(fsfd, FSCONFIG_CMD_CREATE, NULL, NULL, 0));
+
+ int mountfd = fsmount(fsfd, FSMOUNT_CLOEXEC, 0);
+ ASSERT_GE(mountfd, 0);
+
+ /* fsmount returns an O_PATH, which ioctl(2) doesn't accept. */
+ int new_mountfd = openat(mountfd, ".", O_RDONLY|O_DIRECTORY|O_CLOEXEC);
+ ASSERT_GE(new_mountfd, 0);
+
+ ASSERT_SUCCESS(close(mountfd));
+ mountfd = -EBADF;
+
+ int procfs_pidns1 = ioctl(new_mountfd, PROCFS_GET_PID_NAMESPACE);
+ ASSERT_GE(procfs_pidns1, 0);
+
+ ASSERT_NE(self->dummy_pidns, procfs_pidns1);
+ ASSERT_TRUE(is_same_inode(self->host_pidns, procfs_pidns1));
+ ASSERT_FALSE(is_same_inode(self->dummy_pidns, procfs_pidns1));
+
+ ASSERT_SUCCESS(fsconfig(fsfd, FSCONFIG_SET_STRING, "pidns", "/tmp/dummy-pidns", 0));
+ ASSERT_SUCCESS(fsconfig(fsfd, FSCONFIG_CMD_RECONFIGURE, NULL, NULL, 0));
+
+ int procfs_pidns2 = ioctl(new_mountfd, PROCFS_GET_PID_NAMESPACE);
+ ASSERT_GE(procfs_pidns2, 0);
+
+ ASSERT_NE(self->dummy_pidns, procfs_pidns2);
+ ASSERT_FALSE(is_same_inode(self->host_pidns, procfs_pidns2));
+ ASSERT_TRUE(is_same_inode(self->dummy_pidns, procfs_pidns2));
+
+ ASSERT_SUCCESS(close(fsfd));
+ ASSERT_SUCCESS(close(new_mountfd));
+ ASSERT_SUCCESS(close(procfs_pidns1));
+ ASSERT_SUCCESS(close(procfs_pidns2));
+}
+
+TEST_HARNESS_MAIN
--
2.50.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox