Hacker News new | comments | show | ask | jobs | submit login
Kernel module for advanced rickrolling replaces open() call (github.com)
177 points by wink 1919 days ago | hide | past | web | 63 comments | favorite



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 need is in this discussion thread.

(edit: I can already see some pull requests on this)


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.


> 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.


Did you notice the smiley at the end of that line? I intended that to mean that I'm not really serious.


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


This is great, for the fun of course, but also and mainly because it is a simple example of a working kernel module, which is not something one can see very often.


What are you talking about? The linux sources are full of simple examples of working kernel modules.


But those are all things like parallel port drivers. This is for an Internet meme!!!!


For wildly varying values of "simple"...


Do read http://news.ycombinator.com/item?id=2973012 (various security issues), though.


Yep yep I saw that too. But this kind of security issue is not specific to kernel module, it's something every C programmer should be aware of already. (I don't think it's that important here tho, because if the kernel crashes because of this module it's "lol" too :-p). What's interesting here is the C file structure (the #includes and the MODULE_* and module_* macros and functions) and the Makefile.


Manipulating the cr0 register is _very_ kernel specific.


Oh right I didn't see that before posting my previous comment, I thought it was just the `p = (char *)(path + strlen(path) - 4);` thing, which I had also spotted on my own. Thanks for the warning!


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...


Out of curiosity, I checked out how the open syscall in linux works (see fs/open.c). It uses getname (from fs/namei.h), which effectively copies the filename from userland memory to a chunk of kernel memory. It uses strncpy_from_user to do this and uses PATH_MAX as the maximum length. So even the linux kernel does rely on the fact that there is a null terminator in the string.

So your assumption on omitting the null terminator is wrong, BUT there's another flaw in this. The length of the string can be less than four bytes. By allocating the string in a nice page boundary this can be used to cause a memory protection fault.


> It uses strncpy_from_user to do this and uses PATH_MAX as the maximum length. So even the linux kernel does rely on the fact that there is a null terminator in the string.

The PATH_MAX guarantees that the string copy will terminate even if no null character is present.

Now if any routines that call strncpy_from_user assume that it leaves them with a null terminated string there is still a potential for trouble but that does not stem from the use of strncpy_from_user.

Check the implementation for do_getname:

http://lxr.free-electrons.com/source/fs/namei.c

And you'll see that it returns -ENAMETOOLONG when the name is >= the buffer allocated, only < len is allowed. (line 133 in the link above).


> It uses strncpy_from_user to do this and uses PATH_MAX as the maximum length. So even the linux kernel does rely on the fact that there is a null terminator in the string.

But the kernel does not call strlen() on the user-supplied string...


You're inserting a dodgy bit of a code into a live running kernel. You are an attacker! I mean, valid technicality, but really? If you've got the ability to insert any module you want, you have the ability (ok, not always, but usually) to rm -rf /


Bullshit.

Anyone who's using open() can kill the kernel after loading this module.


I guess you meant, ``Anyone who's using open() can kill the kernel after somebody else loads the module.''. Which is far worse: any unprivileged user, say the victim of rickroll, can bite back hard.


This seems like a feature not a bug.


Probably best that any user can kill the kernel in this situation, if you stop to think about it.


Anyone who can load a kernel module can do _whatever they want_. That was my point, which you've obviously missed.


And you are obviously missing that modern operating systems have more than just one user. :-)


I'm genuinely interested - what do you mean? It's my understanding (and please, correct me if I'm wrong) that if you're able to insmod a non-standard kernel module into a linux system, you are root already. I don't understand what this has to do with unix being multi-user, only root has the ability to insmod /tmp/haha.ko Please note I'm not talking about the ability of the kernel to automatically load modules as required. That's only from the /lib/modules path and you need to be root have to inserted a module in there anyway.

And if you have that ability, worrying about the security of a module such as this seems, to me, totally and utterly pointless, because you could be inserting a rootkit, or a module that randomly deletes a file every time you press the letter A etc.

So could you explain what you mean please? Why be concerned about security when security is already compromised? How does it being multi-user make a difference?


Sure.

Suppose your admin is a funny guy and loads this kernel module at April 1st. I (Jonny non-root user) can no longer listen to my favorite mp3, because the kernel module changes the path all the time. The admin loughs and has fun.

But then later I call open() with some fancy arguments and take over the whole machine (or at least crash it). This is the part where I lough.


Ahhh right, yes, Ok, I see what you mean. If you loaded this on a serious system, you're insane. But right, I see your point.


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 other parts of the kernel enable or disable this at context switches or at other times.

What this also means is that this kernel module will only work on x86 CPU's. So no rickrolling you ARM boxes or smartphones.


> 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.


If we could take CR0 et al manipulations away from module authors, we'd do it long ago. Sigh...


This is why I avoid out-of-tree modules. At least someone had to glance over the in-tree module before I run it on my machine. When you download random kernel modules from the Internet, they're probably from people too lazy to have their code properly reviewed. And if they're too lazy to have their code reviewed, they probably don't feel too bad about disabling all page write protection, either.


Can you name such a module?


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


[deleted]


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


I'll be saving this for April 1, 2012 :)


See, I'd do this sort of thing for any image by opening up my wifi and waiting for people to connect. Then just ensure I serve rickroll pics/videos/flash videos/sound whenever appropriate content is requested.


Why not just write a FUSE module?


1. A kernel module is more 1337 ;)

2. Using FUSE you would have to guaranty that your fs overlaps all other mounted filesystems.


Maybe because the author prefers to play with kernel. Other option is create a wrapper library and enforce it's usage with LD_PRELOAD.


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?


I know apps can invoke kernel syscalls directly, and I'm sure you could name a handful of apps that do this all time. But are you really doing that in your mp3 player?


Since static binaries don't use shared libraries, LD_PRELOAD has no effect - it is the same as with assembly code you mentioned.


With LD_PRELOAD you can hook any _function_ call. A syscall is something different. It depends how the syscall wrapper within the libc is coded. Some can be hooked...


But open(3) is a function call, so it can be hooked.


It's open(2). Yes, here on my x86_64 it can be hooked. But I've seen lots of setups where this was not doable. As I said it depends how the libc's wrapper is written.


On Linux, there's typically both an open(3) and an open(2). The former is implemented by glibc and calls the latter which is implemented by the kernel.

So an app calls the open() function. This results in an unresolved symbol which the run-time linker matches up with the open() exported by glibc. glibc then does the open() syscall.


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?


libc implements a function called open. Its documentation appears in section 3 of the UNIX manual. Because of that, we call it open(3). Linux implements a system call called open. Its documentation appears in section 2 of the UNIX manual. Because of that, we call it open(2). From assembly, you manually call open(2). From a C program linked against libc, you typically open(3). open(3) can be hooked, because it's just a regular function. open(2) is not a function, and so cannot be hooked like a function.


syscall number 5 you mean?


How does strace hook it?


With a system call, ptrace(2).


ptrace.


I'm kind of suspecting this started off as "I want to make a simple kernel module", not "I wonder how I can make my computer rickroll me more often".


may I suggest the use of a random rick-roll probability?


Brilliant.




Guidelines | FAQ | Support | API | Security | Lists | Bookmarklet | DMCA | Apply to YC | Contact

Search: