Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Kernel module for advanced rickrolling replaces open() call

github.com

Post reply on HN
Print view View original

51–60 of 67 posts

Re: Kernel module for advanced rickrolling replaces open() call

#51

Post by KonradKlause »

waitwhat wrote: post #50
KonradKlause wrote: post #49

Earlier quoted context omitted.

open(2) is the wrapper within the libc. There is no open(3) on Linux.

Section 2 of the unix manual is for syscalls. Section 3 is for C library functions. http://en.wikipedia.org/wiki/Man_page#Manual_sections If libc implements it, then it is by definition in section 3. EDIT: (signing out of this sub-thread)

Sorry, you're just wrong.

Have you ever called a system call by hand using assembly?

Have you ever looked at glibc's source?

Habe you ever looked at the kernel's close() syscall?

Re: Kernel module for advanced rickrolling replaces open() call

#52

Post by bstx »

exDM69 wrote: post #23
KonradKlause wrote: post #12

Earlier quoted context omitted.

BTW: write_cr0(read_cr0() | 0x10000) is a absolute no-go. It's a nice and funny kernel module but it dooms the kernel's security.

For the uninitiated, cr0 is a control register in the x86 family of processors. Bit 16 of that register is (according to wikipedia) WP - Write protect: Determines whether the CPU can write to pages marked read-only. What this means is that the kernel can no longer see if a user is writing to a read-only location, which is a major part of memory protection. It has major security implications. I don't know whether othe…

> What this means is that the kernel can no longer see if a user is writing to a read-only location, which is a major part of memory protection.

No, disabling WP in CR0 is only relevant for privileged code. Writes to write protected pages in user mode will still trap. It might mess up some in-kernel COW stuff though.

Re: Kernel module for advanced rickrolling replaces open() call

#53

Post by bonzini »

KonradKlause wrote: post #12
KonradKlause wrote: post #3

p = (char *)(path + strlen(path) - 4); This is a very bad idea as path is user-supplied and has to be treated as malicious. An attacker can omit the string-terminator...

BTW: write_cr0(read_cr0() | 0x10000) is a absolute no-go. It's a nice and funny kernel module but it dooms the kernel's security.

Not sure why it's needed, too. It could also restrict the rw/ro toggle to the init_rickroll and exit_rickroll functions.

Re: Kernel module for advanced rickrolling replaces open() call

#54

Post by datenwolf »

forgotusername wrote: post #44
merijnv wrote: post #25

Earlier quoted context omitted.

Eh, the code does look for the string terminator. It calls strlen, which indexes the char* until it finds a NULL byte. If the string doesn't have a NULL byte then strlen will happily continue reading past the boundary until it either segfaults or finds a NULL byte.

Just some random nitpickery, but "NULL" has a specific meaning in C, which is 0 cast to a void pointer. So preferably talk about a "NUL" (as it appears in most ASCII tables) or perhaps "null character" (which is what recent C standards prefer instead, cant remember why they changed though).

And… you got your nitpickery wrong. A C null pointer is just 0. The exact definition in the statement is "a null pointer is a integral constant expression with value 0".

There are some stddef.h which cast it to a void*, but technically that's just a compatibility hack. See http://c-faq.com/null/macro.html http://c-faq.com/null/safermacs.html http://c-faq.com/null/nullor0.html http://c-faq.com/null/long0.html

Re: Kernel module for advanced rickrolling replaces open() call

#55

Post by fpletz »

exDM69 wrote: post #29

SECURITY WARNING: do not use this module! Scroll down for more information in the discussion. This module disables a major part of kernel memory protection and trusts user provided file names to be valid. This makes it possible for an UNPRIVILEGED USER to do bad things. These problems are, of course, fixable someone may fix it. Bonus points for whoever fixes the problems and submits a pull request. All the info you n…

I never had security in mind when developing this. I just did it for fun and to prove it was possible after I had the idea. It was never intended to go that popular.

I will incorporate all fixes and address all issues raised in the next few days.

Re: Kernel module for advanced rickrolling replaces open() call

#56

Post by gravitronic »

exDM69 wrote: post #29

SECURITY WARNING: do not use this module! Scroll down for more information in the discussion. This module disables a major part of kernel memory protection and trusts user provided file names to be valid. This makes it possible for an UNPRIVILEGED USER to do bad things. These problems are, of course, fixable someone may fix it. Bonus points for whoever fixes the problems and submits a pull request. All the info you n…

Ah shoot, we just rolled this out to production this morning and sales doesn't want to wait for the delay...

Re: Kernel module for advanced rickrolling replaces open() call

#57

Post by calloc »

KonradKlause wrote: post #51
waitwhat wrote: post #50

Earlier quoted context omitted.

Section 2 of the unix manual is for syscalls. Section 3 is for C library functions. http://en.wikipedia.org/wiki/Man_page#Manual_sections If libc implements it, then it is by definition in section 3. EDIT: (signing out of this sub-thread)

Sorry, you're just wrong. Have you ever called a system call by hand using assembly? Have you ever looked at glibc's source? Habe you ever looked at the kernel's close() syscall?

syscall number 5 you mean?

Re: Kernel module for advanced rickrolling replaces open() call

#58

Post by exDM69 »

fpletz wrote: post #55
exDM69 wrote: post #29

SECURITY WARNING: do not use this module! Scroll down for more information in the discussion. This module disables a major part of kernel memory protection and trusts user provided file names to be valid. This makes it possible for an UNPRIVILEGED USER to do bad things. These problems are, of course, fixable someone may fix it. Bonus points for whoever fixes the problems and submits a pull request. All the info you n…

I never had security in mind when developing this. I just did it for fun and to prove it was possible after I had the idea. It was never intended to go that popular. I will incorporate all fixes and address all issues raised in the next few days.

You should never ever admit that you didn't think of security when writing code, especially in the kernel space :)

I think that instead of the strlen pull request you accepted, you should use getname/strncpy_from_user like the open() syscall does instead of strlen+memcpy.

Also, while you're here, could you explain what is the purpose of disabling memory write protection? Is it to allow writing to the syscall table? You seem to enable writing in the page table for the syscall table before writing, but never disable it. Could it be that you forgot to flush the translation lookaside buffer (TLB) after updating the pagetable and the change you did does not take effect soon enough? I don't know how you flush TLB's in Linux but the Intel manuals tell you to do so after updating the page table.

It's fun to see that there were so many people interested in this, although it's not much more than a joke. I guess part of the interest stems from the fact that it's a small kernel module with so little code that it's easy to grok.

Re: Kernel module for advanced rickrolling replaces open() call

#59

Post by exDM69 »

waitwhat wrote: post #41
KonradKlause wrote: post #32

Earlier quoted context omitted.

Using LD_PRELOAD you cannot hook a syscall like open().

Doesn't pretty much everything call into libc to open() things rather than invoke the syscall directly?

No. Assembly code invokes kernel syscalls by invoking an interrupt or using a syscall opcode. No libc or function calls involved, so LD_PRELOAD can't hook in.

You might even have several libc's in your system. You might have a uClibc + Busybox based initrd and a full glibc based root system.

Btw. how does LD_PRELOAD act together static binaries?

Re: Kernel module for advanced rickrolling replaces open() call

#60

Post by Pewpewarrows »

exDM69 wrote: post #58
fpletz wrote: post #55

Earlier quoted context omitted.

I never had security in mind when developing this. I just did it for fun and to prove it was possible after I had the idea. It was never intended to go that popular. I will incorporate all fixes and address all issues raised in the next few days.

You should never ever admit that you didn't think of security when writing code, especially in the kernel space :) I think that instead of the strlen pull request you accepted, you should use getname/strncpy_from_user like the open() syscall does instead of strlen+memcpy. Also, while you're here, could you explain what is the purpose of disabling memory write protection? Is it to allow writing to the syscall table? Y…

> You should never ever admit that you didn't think of security when writing code

For a joke weekend project that you never intended anyone to see / use seriously? Sorry, no. I would never waste time making something secure that I did for funzies that I'll likely never touch again. If I happen to come across or realize a security issue as I'm writing it I might make a comment there in the source or in the README. Otherwise, that's time spent I could have been making my real projects better and more secure.

Post reply on HN