On Wednesday 14 December 2005 17:48, Jeff Dike wrote: > On Wed, Dec 14, 2005 at 01:15:55PM +0100, Blaisorblade wrote: > > As a fix, I suggest switching away from printk() for such early uses. Do > > you agree? I wonder if that's enough, but hey, the rule is "no spinlock > > without kernel stack". And then it seems correct the printk() avoidance.
> Yes, this occurred to me later - printk (and any other kernel stuff) should > not run on the initial stack. I've tried to be careful about this, but we > seem to have missed some things. printf should be used on the initial > stack, printk should be used on kernel stacks, and neither should be used > elsewhere. On the same theme (wrong stacks): [42949390.690000] Debug: sleeping function called from invalid context at /home/paolo/Admin/kernel/6/VCS/linux-2.6.14/mm/slab.c:2459 [42949390.690000] in_atomic():1, irqs_disabled():0 [42949390.690000] a1d84efc: [<a0014672>] dump_stack+0x22/0x30 [42949390.690000] a1d84f1c: [<a003558c>] __might_sleep+0xac/0xd0 [42949390.690000] a1d84f3c: [<a006aa21>] __kmalloc+0xc1/0x110 [42949390.690000] a1d84f6c: [<a0011a39>] um_kmalloc+0x19/0x20 [42949390.690000] a1d84f7c: [<a001017d>] __wrap_malloc+0x2d/0x80 [42949390.690000] a1d84f8c: [<a01f6c59>] execvp+0x79/0x2b0 [42949390.690000] a1d84fbc: [<a000ed3f>] helper_child+0x2f/0xc0 [42949390.690000] a1d84fdc: [<a01f914a>] clone+0x6a/0x80 [42949390.690000] [42949407.770000] line_ioctl: tty1: unknown ioctl: 0x4b50 Apart from the fact that we should really make __wrap_malloc() use GFP_ATOMIC when needed (I have a working patch, I've held it because I wasn't sure it was safe, plus in_atomic() || irq_disabled() is not exhaustive - see when a spinlock is held and preemption was disabled compile-time), here we have libc calling malloc() on a non-kernel stack (the newly allocated one for the child). So, even the in_atomic() value is invalid, being deducted from the one of the calling thread; and trying to access any member of current would dereference (struct task*) rounded_down($esp), which would likely crash. In this (simple) case, it seems it's not worth to implement some stack detection for __wrap_malloc() - it even seems very difficult to do. Possible ideas: *) lookup in a list the result of getpid() and consider a search miss as a "we're not a kernel thread" *) try adding a magic to thread_info and test for that (we have a different stack with clone, normally) *) using TLS variables. I currently don't endorse any of them - and even then we don't have any suitable allocator for this situation. I'm rather trying switching away from execvp(), to avoid any allocations (the callbacks supplied to run_helper() are also to audit, however - but dup2() and close() seem prevalent, and they're safe). Unluckily, most users require the PATH search. And while digging I've not found a glibc API providing the PATH lookup function separately from execvp(). Guess we'll have to incorporate a copy from glibc code. -- Inform me of my mistakes, so I can keep imitating Homer Simpson's "Doh!". Paolo Giarrusso, aka Blaisorblade (Skype ID "PaoloGiarrusso", ICQ 215621894) http://www.user-mode-linux.org/~blaisorblade ___________________________________ Yahoo! Messenger: chiamate gratuite in tutto il mondo http://it.messenger.yahoo.com ------------------------------------------------------- This SF.net email is sponsored by: Splunk Inc. Do you grep through log files for problems? Stop! Download the new AJAX search engine that makes searching your log files as easy as surfing the web. DOWNLOAD SPLUNK! http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click _______________________________________________ User-mode-linux-user mailing list User-mode-linux-user@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/user-mode-linux-user
