Page MenuHomeFreeBSD

kern_cpuset: allow using the explicit form of own pid/tid in capability mode
ClosedPublic

Authored by greg_unrelenting.technology on Jul 2 2020, 8:12 PM.

Details

Summary

Firefox (Spidermonkey) does this to get the stack base (well, on FreeBSD and NetBSD — OpenBSD conveniently has a pthread_stackseg_np thing):

pthread_t thread = pthread_self();
pthread_attr_get_np(thread, &sattr);
rc = pthread_attr_getstack(&sattr, &stackBase, &stackSize);

And pthread_attr_get_np (_thr_attr_get_np) does:

	ret = cpuset_getaffinity(CPU_LEVEL_WHICH, CPU_WHICH_TID, TID(pthread),
		dst->cpusetsize, dst->cpuset);

Whoops, that's not allowed in capability mode, even though we are getting the current thread's attributes, which should be allowed!

(Firefox does work with failing pthread_attr_get_np anyway, but it's better not to have the error :D)

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

This looks ok to me. I'm happy to commit the patch as-is, but:

  • these checks should really be lifted into a subroutine,
  • cpuset_(get|set)domain() are not listed in capabilities.conf and so aren't available in capability mode at all, though clearly they should be.

Please let me know if you'd like to write patches for this, otherwise I will work on it.

sys/kern/kern_cpuset.c
1750 ↗(On Diff #74030)

It seems odd that CPU_WHICH_TID doesn't allow one to select a different thread from the same process. I'm not sure if it's intentional.

This revision is now accepted and ready to land.Jul 6 2020, 2:27 PM
  • these checks should really be lifted into a subroutine,

Yeah I thought about that..

Please let me know if you'd like to write patches for this, otherwise I will work on it.

Feel free to make the better version of the patch yourself, the patch review process would just be a slowdown :)

  • these checks should really be lifted into a subroutine,

Yeah I thought about that..

Please let me know if you'd like to write patches for this, otherwise I will work on it.

Feel free to make the better version of the patch yourself, the patch review process would just be a slowdown :)

Ok.

sys/kern/kern_cpuset.c
1750 ↗(On Diff #74030)

It's a bit tricky to handle this case without introducing TOCTTOU races. We'd have to modify cpuset_which() to restrict the tdfind() call to the current process, so the capsicum checking wouldn't be done in a single centralized place anymore.