Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Beware user space buffers in newer kernels #13

Open
rockobonaparte opened this issue Sep 15, 2024 · 0 comments
Open

Beware user space buffers in newer kernels #13

rockobonaparte opened this issue Sep 15, 2024 · 0 comments

Comments

@rockobonaparte
Copy link

I was using your examples to figure out how to bang some bits on a Raspberry Pi 5 (thank you for this project, by the way). It's running the 6.6.50 kernel. I think it was somewhere in the 6.x kernels that touching user space buffers started causing oopses. I suffered this in my completely-unrelated day job. So let's say this file that I was actively poking at:

https://github.com/Johannes4Linux/Linux_Driver_Tutorial/blob/main/21_dt_gpio/dt_gpio.c

static ssize_t my_write(struct file *File, const char *user_buffer, size_t count, loff_t *offs) {
	switch (user_buffer[0]) {
		case '0':
		case '1':
			gpiod_set_value(my_led, user_buffer[0] - '0');
		default:
			break;
	}
	return count;
}

Touching user_buffer[0] is going to make the kernel oops, throw a fit, and possibly even end up hanging. I had a few reboots puke.

I think the idea is the user could monkey with the memory while the kernel code is reacting to it, and an attacker could use that to make the kernel code do crazy, naughty stuff. I'm not sure of all possible remedies--like if there's a way to declare that you don't care about the vulnerability and you want to touch the buffer anyways) because a typical strategy is to just copy everything to a new buffer like:

static ssize_t my_write(struct file *File, const char *user_buffer, size_t count, loff_t *offs) {
        if(count > 0) {
                char *copied = (char*) kmalloc(sizeof(char) * count, GFP_KERNEL);
                if(copy_from_user(copied, user_buffer, count) == 0) {
                        switch (copied[0]) {
                                case '0':
                                case '1':
                                        gpiod_set_value(out_gpio_desc, copied[0] - '0');
                                default:
                                        break;
                        }
                } else {
                        printk("dt_probe - Could not copy user_buffer to a save internal buffer. Abort GPIO change\n");
                }
        }
        return count;
}

The idea is that the user could still be monkeying with that buffer while copy_from_user is going, but the module isn't actively trying to use the buffer and it'll just get the screwed up one after the copy is done.

This is not killing me or anything, but GitHub doesn't really give alternatives for highlighting this stuff other than to file an issue. I'll leave it to you what you want to do and I won't get weird if you close the issue without doing anything. Again, thank you for this project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant