Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Is this a bug in linux kernel concerning write to /proc/self/loginuid?

There is a possibility that i found a bug in linux kernel. Let's consider application that write to /proc/self/loginuid from main thread and one auxiliary thread. The code is below:

#include <stdio.h>
#include <pthread.h>
#include <sys/syscall.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>

void write_loginuid(char *str)
{
    int fd;

    printf("%s\n", str);

    fd = open("/proc/self/loginuid", O_RDWR);

    if (fd < 0) {
        perror(str);
        return;
    }

    if (write(fd, "0", 2) != 2) {
        printf("write\n");
        perror(str);
    }

    close(fd);
}

void *thread_function(void *arg)
{
    fprintf(stderr, "Hello from thread! my pid = %u, tid = %u, parent pid = %u\n", getpid(), syscall(SYS_gettid), getppid());

    write_loginuid("thread");

    return NULL;
}

int main()
{
    pthread_t thread;

    pthread_create(&thread, NULL, thread_function, NULL);

    write_loginuid("main process");

    fprintf(stderr, "test my pid = %u, tid = %u, parent pid = %u\n", getpid(), syscall(SYS_gettid), getppid());

    pthread_join(thread, NULL);
    return 0;
}

After executing this application we get:

main process
test my pid = 3487, tid = 3487, parent pid = 3283
Hello from thread! my pid = 3487, tid = 3488, parent pid = 3283
thread
write
thread: Operation not permitted

That tells us the thread write failed by -EPERM.

Looking at the kernel file fs/proc/base.c and function proc_loginuid_write() we see at the beginning check:

static ssize_t proc_loginuid_write(struct file * file, const char __user * buf,
                   size_t count, loff_t *ppos)
{
    struct inode * inode = file_inode(file);
    uid_t loginuid;
    kuid_t kloginuid;
    int rv;

    /* this is the probably buggy check */
    rcu_read_lock();
    if (current != pid_task(proc_pid(inode), PIDTYPE_PID)) {
        rcu_read_unlock();
        return -EPERM;
    }
    rcu_read_unlock();

So, looking at the code above we see that only for exact PID (checked by me with printks) we pass through.Thread doesn't satisfy the condition, because compared pids differs.

So my question is: is this a bug ? Why to not allow thread's of particular process to change the loginuid? I encountered this in login application that spawned another thread for PAM login.

like image 785
sibislaw Avatar asked Nov 08 '22 02:11

sibislaw


1 Answers

Whether this is bug or not i written a fix that extends writing permission to this file by threads:

rcu_read_lock();
/*
 * I changed the condition that it checks now the tgid as returned in sys_getpid()
 * rather than task_struct pointers
 */
if (task_tgid_vnr(current) != task_tgid_vnr(pid_task(proc_pid(inode), PIDTYPE_PID))) {
    rcu_read_unlock();
    return -EPERM;
}
rcu_read_unlock();

What do you think about it? Does it affects security?

like image 162
sibislaw Avatar answered Nov 15 '22 07:11

sibislaw