• Colin Ian King's avatar
    UBUNTU:SAUCE: exec: ensure file system accounting in check_unsafe_exec is correct · d6572202
    Colin Ian King authored
    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: default avatarColin Ian King <colin.king@canonical.com>
    Acked-by: default avatarSeth Forshee <seth.forshee@canonical.com>
    Acked-by: default avatarStefan Bader <stefan.bader@canonical.com>
    Signed-off-by: default avatarThadeu Lima de Souza Cascardo <cascardo@canonical.com>
    d6572202
exec.c 42 KB