diff mbox series

nptl: Don't madvise user provided stack

Message ID 20200624104124.27695-1-szabolcs.nagy@arm.com
State Committed
Commit 087942251f26d5fd5802b8d14e47d460263a0c4d
Headers
Series nptl: Don't madvise user provided stack |

Commit Message

Szabolcs Nagy June 24, 2020, 10:41 a.m. UTC 
  User provided stack should not be released nor madvised at
thread exit because it's owned by the user.

If the memory is shared or file based then MADV_DONTNEED
can have unwanted effects. With memory tagging on aarch64
linux the tags are dropped and thus it may invalidate
pointers.

Tested on aarch64-linux-gnu with MTE, it fixes

FAIL: nptl/tst-stack3
FAIL: nptl/tst-stack3-mem

---

Note: the arm64 MTE linux ABI is still under discussion, but
for MADV_DONTNEED the current behaviour seemed most useful.
this behaviour is mainly problematic when madivse is used on
memory that's allocated and owned by somebody else which is
not expected to be common. (MADV_FREE and MADV_WIPEONFORK
has similar issues.)
---
 nptl/pthread_create.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Florian Weimer June 24, 2020, 11:05 a.m. UTC | #1 
* Szabolcs Nagy:

> +  if (!pd->user_stack)
> +    advise_stack_range (pd->stackblock, pd->stackblock_size, (uintptr_t) pd,
> +			pd->guardsize);

Could we still use MADV_FREE?

But the patch as-is looks fine to me.

Thanks,
Florian
Szabolcs Nagy June 24, 2020, 11:32 a.m. UTC | #2 
The 06/24/2020 13:05, Florian Weimer wrote:
> * Szabolcs Nagy:
> 
> > +  if (!pd->user_stack)
> > +    advise_stack_range (pd->stackblock, pd->stackblock_size, (uintptr_t) pd,
> > +			pd->guardsize);
> 
> Could we still use MADV_FREE?
> 
> But the patch as-is looks fine to me.

MADV_FREE has the same issue (it does
not zero the tags immediately, but may
do so later)

i hope users don't commonly use madvise
(or other mmap/munmap/.. operations) on
malloced memory, that is unlikely to be
supportable with memory tagging.
Szabolcs Nagy June 25, 2020, 1:21 p.m. UTC | #3 
The 06/24/2020 12:32, Szabolcs Nagy wrote:
> The 06/24/2020 13:05, Florian Weimer wrote:
> > * Szabolcs Nagy:
> > 
> > > +  if (!pd->user_stack)
> > > +    advise_stack_range (pd->stackblock, pd->stackblock_size, (uintptr_t) pd,
> > > +			pd->guardsize);
> > 
> > Could we still use MADV_FREE?
> > 
> > But the patch as-is looks fine to me.

now committed.

> 
> MADV_FREE has the same issue (it does
> not zero the tags immediately, but may
> do so later)
> 
> i hope users don't commonly use madvise
> (or other mmap/munmap/.. operations) on
> malloced memory, that is unlikely to be
> supportable with memory tagging.
diff mbox series

Patch

diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c
index 35a9927cf2..6d6ab88960 100644
--- a/nptl/pthread_create.c
+++ b/nptl/pthread_create.c
@@ -549,8 +549,9 @@  START_THREAD_DEFN
     }
 #endif
 
-  advise_stack_range (pd->stackblock, pd->stackblock_size, (uintptr_t) pd,
-		      pd->guardsize);
+  if (!pd->user_stack)
+    advise_stack_range (pd->stackblock, pd->stackblock_size, (uintptr_t) pd,
+			pd->guardsize);
 
   if (__glibc_unlikely (pd->cancelhandling & SETXID_BITMASK))
     {