UBUNTU:SAUCE: exec: ensure file system accounting in check_unsafe_exec is correct
BugLink: http://bugs.launchpad.net/bugs/1672819 There are two very race windows that need to be taken into consideration when check_unsafe_exec performs the file system accounting comparing the number of fs->users against the number threads that share the same fs. The first race can occur when a pthread creates a new pthread and the the fs->users count is incremented before the new pthread is associated with the pthread performing the exec. When this occurs, the pthread performing the exec has flags with bit PF_FORKNOEXEC set. The second race can occur when a pthread is terminating and the fs->users count has been decremented by the pthread is still associated with the pthread that is performing the exec. When this occurs, the pthread peforming the exec has flags with bit PF_EXITING set. This fix keeps track of any pthreads that may be in the race window (when PF_FORKNOEXEC or PF_EXITING) are set and if the fs count does not match the expected count we retry the count as we may have hit this small race windows. Tests on an 8 thread server with the reproducer (see below) show that this retry occurs rarely, so the overhead of the retry is very small. Below is a reproducer of the race condition. The bug manifests itself because the current check_unsafe_exec hits this race and indicates it is not a safe exec, and the exec'd suid program fails to setuid. $ cat Makefile ALL=a b all: $(ALL) a: LDFLAGS=-pthread b: b.c $(CC) b.c -o b sudo chown root:root b sudo chmod u+s b test: for I in $$(seq 1000); do echo $I; ./a ; done clean: rm -vf $(ALL) $ cat a.c #include <stdio.h> #include <unistd.h> #include <pthread.h> #include <sys/types.h> void *nothing(void *p) { return NULL; } void *target(void *p) { for (;;) { pthread_t t; if (pthread_create(&t, NULL, nothing, NULL) == 0) pthread_join(t, NULL); } return NULL; } int main(void) { struct timespec tv; int i; for (i = 0; i < 10; i++) { pthread_t t; pthread_create(&t, NULL, target, NULL); } tv.tv_sec = 0; tv.tv_nsec = 100000; nanosleep(&tv, NULL); if (execl("./b", "./b", NULL) < 0) perror("execl"); return 0; } $ cat b.c #include <stdio.h> #include <unistd.h> #include <sys/types.h> int main(void) { const uid_t euid = geteuid(); if (euid != 0) { printf("Failed, got euid %d (expecting 0)\n", euid); return 1; } return 0; } $ make make cc -pthread a.c -o a cc b.c -o b sudo chown root:root b sudo chmod u+s b $ for i in $(seq 1000); do ./a; done Without the fix, one will see 'Failed, got euid 1000 (expecting 0)' messages Signed-off-by:Colin Ian King <colin.king@canonical.com> Acked-by:
Seth Forshee <seth.forshee@canonical.com> Acked-by:
Stefan Bader <stefan.bader@canonical.com> Signed-off-by:
Thadeu Lima de Souza Cascardo <cascardo@canonical.com>
Showing
Please register or sign in to comment