All of lore.kernel.org
 help / color / mirror / Atom feed
* [uml-devel] Clearing kmalloc_ok during shutdown is broken - malloc will clear our data.
@ 2005-09-28 11:46 Blaisorblade
  2005-09-28 20:12 ` Jeff Dike
  2005-09-28 21:31 ` Jeff Dike
  0 siblings, 2 replies; 10+ messages in thread
From: Blaisorblade @ 2005-09-28 11:46 UTC (permalink / raw)
  To: Jeff Dike, user-mode-linux-devel

I've recently realized how potentially misguided that thing is.

And I am wondering about whether the recent "eactivate_all_fds failed, errno = 
9" (no, the initial "d" never comes out) may be due to this. Even if, now I 
see that was done even before... but first only on TT mode, then also in SKAS 
mode with commit 6770cb61ff6d557613a8382b28f9b0a919fb112f, which says "when 
getting a fatal signal".

But still, Jeff, how can we expect that malloc won't stomp over all our data 
which we preallocated with kmalloc and such?

There's no single mention of that in your original changelog, and this is 
untrivial, so I can assume you didn't realize this issue.

The git commit is 026549d28469f7d4ca7e5a4707f0d2dc4f2c164c.

On the other side, could you explain why you don't like kmalloc in first 
place? It surely works.

Also, there are some calls to kmalloc in the shutdown path - and they work. 
I know this because I saw a problem with one of them: it gave "might_sleep 
while atomic", and it was kmalloc in the shutdown, or rather, in panic() - 
for the broken sysrq t (where's the fix you promised?).

While looking for the "change" function, I thought even if it's static, that's 
still not enough to deserve such an insignificant name. Ah, it's in 
arch/um/drivers/net_user.c.

 <3>Debug: sleeping function called from invalid context 
at /home/paolo/Admin/kernel/6/VCS/linux-2.6.13/mm/slab.c:2096
in_atomic():1, irqs_disabled():0

a0327088:  [<a00164f2>] dump_stack+0x22/0x30
a03270a8:  [<a0049e1c>] __might_sleep+0xac/0xd0
a03270c8:  [<a0094fa0>] __kmalloc+0xc0/0x110
a03270f8:  [<a00129ba>] um_kmalloc+0x1a/0x20
a0327108:  [<a0038b6b>] change+0xcb/0x220
a03271d8:  [<a0038d10>] close_addr+0x20/0x30
a03271e8:  [<a00385bb>] iter_addresses+0x7b/0x90
a0327218:  [<a0041b0a>] tuntap_close+0x4a/0x70
a0327238:  [<a003827d>] close_devices+0x4d/0x90
a0327258:  [<a0012a92>] do_uml_exitcalls+0x22/0x40
a0327268:  [<a0013763>] uml_cleanup+0x13/0x20
.... panic...

The real solution for this warning is to replace um_kmalloc with malloc(), and 
set, during shutdown, kmalloc_only_atomic - which would switch 
__wrap_malloc() from um_kmalloc to um_kmalloc_atomic.

Or better yet, simply test in_atomic() and irqs_disabled() to choose between 
the atomic and normal versions.
-- 
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! Mail: gratis 1GB per i messaggi e allegati da 10MB 
http://mail.yahoo.it



-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [uml-devel] Clearing kmalloc_ok during shutdown is broken - malloc will clear our data.
  2005-09-28 11:46 [uml-devel] Clearing kmalloc_ok during shutdown is broken - malloc will clear our data Blaisorblade
@ 2005-09-28 20:12 ` Jeff Dike
  2005-09-29 12:07   ` Blaisorblade
  2005-09-28 21:31 ` Jeff Dike
  1 sibling, 1 reply; 10+ messages in thread
From: Jeff Dike @ 2005-09-28 20:12 UTC (permalink / raw)
  To: Blaisorblade; +Cc: user-mode-linux-devel

On Wed, Sep 28, 2005 at 01:46:15PM +0200, Blaisorblade wrote:
> And I am wondering about whether the recent "eactivate_all_fds failed, errno = 
> 9" 

I found this one, although I didn't notice the missing 'd'.  It turns out that
close_chan closes file descriptors, but never freed the associated IRQs.

> But still, Jeff, how can we expect that malloc won't stomp over all our data 
> which we preallocated with kmalloc and such?

Because I leave a decent amount of room between the brk and the start of 
kmalloc-able memory (which is free_pages-d during boot).  It's a couple meg,
I think.  No guarantees, of course, but there isn't a lot of mallocing
happening.

> There's no single mention of that in your original changelog, and this is 
> untrivial, so I can assume you didn't realize this issue.
> 
> The git commit is 026549d28469f7d4ca7e5a4707f0d2dc4f2c164c.
> 
> On the other side, could you explain why you don't like kmalloc in first 
> place? It surely works.

I'm not sure - let me think about it.  But I fixed that for a reason - it
was causing a crash somehow, I just don't remember the details.

> The real solution for this warning is to replace um_kmalloc with malloc(), and 
> set, during shutdown, kmalloc_only_atomic - which would switch 
> __wrap_malloc() from um_kmalloc to um_kmalloc_atomic.

Yeah, that's userspace code, so it should probably just use malloc.

> Or better yet, simply test in_atomic() and irqs_disabled() to choose between 
> the atomic and normal versions.

I don't really like testing in_atomic() because you should generally know
what context you're running in.

				Jeff


-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [uml-devel] Clearing kmalloc_ok during shutdown is broken - malloc will clear our data.
  2005-09-28 11:46 [uml-devel] Clearing kmalloc_ok during shutdown is broken - malloc will clear our data Blaisorblade
  2005-09-28 20:12 ` Jeff Dike
@ 2005-09-28 21:31 ` Jeff Dike
  2005-09-29 14:14   ` Blaisorblade
  1 sibling, 1 reply; 10+ messages in thread
From: Jeff Dike @ 2005-09-28 21:31 UTC (permalink / raw)
  To: Blaisorblade; +Cc: user-mode-linux-devel

[-- Attachment #1: Type: text/plain, Size: 373 bytes --]

On Wed, Sep 28, 2005 at 01:46:15PM +0200, Blaisorblade wrote:
> Also, there are some calls to kmalloc in the shutdown path - and they work. 
> I know this because I saw a problem with one of them: it gave "might_sleep 
> while atomic", and it was kmalloc in the shutdown, or rather, in panic() - 
> for the broken sysrq t (where's the fix you promised?).

Attached.

	Jeff

[-- Attachment #2: sysrq-t --]
[-- Type: text/plain, Size: 9502 bytes --]

# From Allan Graves:
#
# Fix sysrq-t support for skas mode.  The old code had the IP and SP coming
# from the registers in the thread struct, which are completely wrong since
# those are the userspace registers.  This fixes that by pulling the correct
# values from the jmp_buf in which the kernel state of each thread is stored.
#
# Signed-off-by: Allan Graves <allan.graves@oracle.com>
Index: test/arch/um/include/registers.h
===================================================================
--- test.orig/arch/um/include/registers.h	2005-09-14 15:52:06.000000000 -0400
+++ test/arch/um/include/registers.h	2005-09-27 19:00:35.000000000 -0400
@@ -15,16 +15,6 @@
 extern void restore_registers(int pid, union uml_pt_regs *regs);
 extern void init_registers(int pid);
 extern void get_safe_registers(unsigned long * regs);
+extern void get_thread_regs(union uml_pt_regs *uml_regs, void *buffer);
 
 #endif
-
-/*
- * Overrides for Emacs so that we follow Linus's tabbing style.
- * Emacs will notice this stuff at the end of the file and automatically
- * adjust the settings for this buffer only.  This must remain at the end
- * of the file.
- * ---------------------------------------------------------------------------
- * Local variables:
- * c-file-style: "linux"
- * End:
- */
Index: test/arch/um/include/sysdep-x86_64/ptrace.h
===================================================================
--- test.orig/arch/um/include/sysdep-x86_64/ptrace.h	2005-09-27 11:33:43.000000000 -0400
+++ test/arch/um/include/sysdep-x86_64/ptrace.h	2005-09-27 19:55:07.000000000 -0400
@@ -218,10 +218,6 @@
                 case RBP: UPT_RBP(regs) = __upt_val; break; \
                 case ORIG_RAX: UPT_ORIG_RAX(regs) = __upt_val; break; \
                 case CS: UPT_CS(regs) = __upt_val; break; \
-                case DS: UPT_DS(regs) = __upt_val; break; \
-                case ES: UPT_ES(regs) = __upt_val; break; \
-                case FS: UPT_FS(regs) = __upt_val; break; \
-                case GS: UPT_GS(regs) = __upt_val; break; \
                 case EFLAGS: UPT_EFLAGS(regs) = __upt_val; break; \
                 default :  \
                         panic("Bad register in UPT_SET : %d\n", reg);  \
Index: test/arch/um/kernel/sysrq.c
===================================================================
--- test.orig/arch/um/kernel/sysrq.c	2005-06-17 15:48:29.000000000 -0400
+++ test/arch/um/kernel/sysrq.c	2005-09-27 19:00:35.000000000 -0400
@@ -62,13 +62,7 @@
 
 	if (esp == NULL) {
 		if (task != current && task != NULL) {
-			/* XXX: Isn't this bogus? I.e. isn't this the
-			 * *userspace* stack of this task? If not so, use this
-			 * even when task == current (as in i386).
-			 */
 			esp = (unsigned long *) KSTK_ESP(task);
-			/* Which one? No actual difference - just coding style.*/
-			//esp = (unsigned long *) PT_REGS_IP(&task->thread.regs);
 		} else {
 			esp = (unsigned long *) &esp;
 		}
@@ -84,5 +78,5 @@
 	}
 
 	printk("Call Trace: \n");
-	show_trace(current, esp);
+	show_trace(task, esp);
 }
Index: test/arch/um/os-Linux/sys-i386/registers.c
===================================================================
--- test.orig/arch/um/os-Linux/sys-i386/registers.c	2005-09-14 15:52:06.000000000 -0400
+++ test/arch/um/os-Linux/sys-i386/registers.c	2005-09-27 19:28:43.000000000 -0400
@@ -5,6 +5,7 @@
 
 #include <errno.h>
 #include <string.h>
+#include <setjmp.h>
 #include "sysdep/ptrace_user.h"
 #include "sysdep/ptrace.h"
 #include "uml-config.h"
@@ -126,13 +127,11 @@
 	memcpy(regs, exec_regs, HOST_FRAME_SIZE * sizeof(unsigned long));
 }
 
-/*
- * Overrides for Emacs so that we follow Linus's tabbing style.
- * Emacs will notice this stuff at the end of the file and automatically
- * adjust the settings for this buffer only.  This must remain at the end
- * of the file.
- * ---------------------------------------------------------------------------
- * Local variables:
- * c-file-style: "linux"
- * End:
- */
+void get_thread_regs(union uml_pt_regs *uml_regs, void *buffer)
+{
+	struct __jmp_buf_tag *jmpbuf = buffer;
+
+	UPT_SET(uml_regs, EIP, jmpbuf->__jmpbuf[JB_PC]);
+	UPT_SET(uml_regs, UESP, jmpbuf->__jmpbuf[JB_SP]);
+	UPT_SET(uml_regs, EBP, jmpbuf->__jmpbuf[JB_BP]);
+}
Index: test/arch/um/os-Linux/sys-x86_64/registers.c
===================================================================
--- test.orig/arch/um/os-Linux/sys-x86_64/registers.c	2005-09-14 15:52:06.000000000 -0400
+++ test/arch/um/os-Linux/sys-x86_64/registers.c	2005-09-27 19:31:44.000000000 -0400
@@ -5,6 +5,7 @@
 
 #include <errno.h>
 #include <string.h>
+#include <setjmp.h>
 #include "ptrace_user.h"
 #include "uml-config.h"
 #include "skas_ptregs.h"
@@ -74,13 +75,11 @@
 	memcpy(regs, exec_regs, HOST_FRAME_SIZE * sizeof(unsigned long));
 }
 
-/*
- * Overrides for Emacs so that we follow Linus's tabbing style.
- * Emacs will notice this stuff at the end of the file and automatically
- * adjust the settings for this buffer only.  This must remain at the end
- * of the file.
- * ---------------------------------------------------------------------------
- * Local variables:
- * c-file-style: "linux"
- * End:
- */
+void get_thread_regs(union uml_pt_regs *uml_regs, void *buffer)
+{
+	struct __jmp_buf_tag *jmpbuf = buffer;
+
+	UPT_SET(uml_regs, RIP, jmpbuf->__jmpbuf[JB_PC]);
+	UPT_SET(uml_regs, RSP, jmpbuf->__jmpbuf[JB_RSP]);
+	UPT_SET(uml_regs, RBP, jmpbuf->__jmpbuf[JB_RBP]);
+}
Index: test/arch/um/sys-i386/sysrq.c
===================================================================
--- test.orig/arch/um/sys-i386/sysrq.c	2005-06-17 15:48:29.000000000 -0400
+++ test/arch/um/sys-i386/sysrq.c	2005-09-27 19:00:35.000000000 -0400
@@ -88,9 +88,7 @@
 		task = current;
 
 	if (task != current) {
-		//ebp = (unsigned long) KSTK_EBP(task);
-		/* Which one? No actual difference - just coding style.*/
-		ebp = (unsigned long) PT_REGS_EBP(&task->thread.regs);
+		ebp = (unsigned long) KSTK_EBP(task);
 	} else {
 		asm ("movl %%ebp, %0" : "=r" (ebp) : );
 	}
@@ -99,15 +97,6 @@
 		((unsigned long)stack & (~(THREAD_SIZE - 1)));
 	print_context_stack(context, stack, ebp);
 
-	/*while (((long) stack & (THREAD_SIZE-1)) != 0) {
-		addr = *stack;
-		if (__kernel_text_address(addr)) {
-			printk("%08lx:	[<%08lx>]", (unsigned long) stack, addr);
-			print_symbol(" %s", addr);
-			printk("\n");
-		}
-		stack++;
-	}*/
 	printk("\n");
 }
 
Index: test/include/asm-um/processor-generic.h
===================================================================
--- test.orig/include/asm-um/processor-generic.h	2005-09-27 11:34:18.000000000 -0400
+++ test/include/asm-um/processor-generic.h	2005-09-27 19:17:47.000000000 -0400
@@ -13,6 +13,7 @@
 #include "linux/config.h"
 #include "asm/ptrace.h"
 #include "choose-mode.h"
+#include "registers.h"
 
 struct mm_struct;
 
@@ -136,19 +137,15 @@
 #define current_cpu_data boot_cpu_data
 #endif
 
-#define KSTK_EIP(tsk) (PT_REGS_IP(&tsk->thread.regs))
-#define KSTK_ESP(tsk) (PT_REGS_SP(&tsk->thread.regs))
-#define get_wchan(p) (0)
 
+#ifdef CONFIG_MODE_SKAS
+#define KSTK_REG(tsk, reg) \
+	({ union uml_pt_regs regs; \
+	   get_thread_regs(&regs, tsk->thread.mode.skas.switch_buf); \
+	   UPT_REG(&regs, reg); })
+#else
+#define KSTK_REG(tsk, reg) (0xbadbabe)
 #endif
+#define get_wchan(p) (0)
 
-/*
- * Overrides for Emacs so that we follow Linus's tabbing style.
- * Emacs will notice this stuff at the end of the file and automatically
- * adjust the settings for this buffer only.  This must remain at the end
- * of the file.
- * ---------------------------------------------------------------------------
- * Local variables:
- * c-file-style: "linux"
- * End:
- */
+#endif
Index: test/include/asm-um/processor-i386.h
===================================================================
--- test.orig/include/asm-um/processor-i386.h	2005-06-17 15:48:29.000000000 -0400
+++ test/include/asm-um/processor-i386.h	2005-09-27 19:17:05.000000000 -0400
@@ -43,17 +43,10 @@
 #define ARCH_IS_STACKGROW(address) \
        (address + 32 >= UPT_SP(&current->thread.regs.regs))
 
+#define KSTK_EIP(tsk) KSTK_REG(tsk, EIP)
+#define KSTK_ESP(tsk) KSTK_REG(tsk, UESP)
+#define KSTK_EBP(tsk) KSTK_REG(tsk, EBP)
+
 #include "asm/processor-generic.h"
 
 #endif
-
-/*
- * Overrides for Emacs so that we follow Linus's tabbing style.
- * Emacs will notice this stuff at the end of the file and automatically
- * adjust the settings for this buffer only.  This must remain at the end
- * of the file.
- * ---------------------------------------------------------------------------
- * Local variables:
- * c-file-style: "linux"
- * End:
- */
Index: test/include/asm-um/processor-x86_64.h
===================================================================
--- test.orig/include/asm-um/processor-x86_64.h	2005-06-17 15:48:29.000000000 -0400
+++ test/include/asm-um/processor-x86_64.h	2005-09-27 19:17:39.000000000 -0400
@@ -36,17 +36,9 @@
 #define ARCH_IS_STACKGROW(address) \
         (address + 128 >= UPT_SP(&current->thread.regs.regs))
 
+#define KSTK_EIP(tsk) KSTK_REG(tsk, RIP)
+#define KSTK_ESP(tsk) KSTK_REG(tsk, RSP)
+
 #include "asm/processor-generic.h"
 
 #endif
-
-/*
- * Overrides for Emacs so that we follow Linus's tabbing style.
- * Emacs will notice this stuff at the end of the file and automatically
- * adjust the settings for this buffer only.  This must remain at the end
- * of the file.
- * ---------------------------------------------------------------------------
- * Local variables:
- * c-file-style: "linux"
- * End:
- */

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [uml-devel] Clearing kmalloc_ok during shutdown is broken - malloc will clear our data.
  2005-09-28 20:12 ` Jeff Dike
@ 2005-09-29 12:07   ` Blaisorblade
  0 siblings, 0 replies; 10+ messages in thread
From: Blaisorblade @ 2005-09-29 12:07 UTC (permalink / raw)
  To: Jeff Dike; +Cc: user-mode-linux-devel

On Wednesday 28 September 2005 22:12, Jeff Dike wrote:
> On Wed, Sep 28, 2005 at 01:46:15PM +0200, Blaisorblade wrote:
> > And I am wondering about whether the recent "eactivate_all_fds failed,
> > errno = 9"

> I found this one, although I didn't notice the missing 'd'.
Hmm, after saying that I saw that 'd' again.
In my experience with 2.6.13, it normally works, except when panicking (I 
triggered with sysrq t, and got the failure).

If it isn't a memory corruption, which I suspected, then we're ok.
> It turns out 
> that close_chan closes file descriptors, but never freed the associated
> IRQs.
So deactivate_all_fds() tries to close them again...
> > But still, Jeff, how can we expect that malloc won't stomp over all our
> > data which we preallocated with kmalloc and such?

> Because I leave a decent amount of room between the brk and the start of
> kmalloc-able memory (which is free_pages-d during boot).  It's a couple
> meg, I think.

> No guarantees, of course, but there isn't a lot of mallocing 
> happening.
Ok, this is enough, but could be documented. But we could record the end of 
malloc'able data, and check it, since we're using a wrapper anyway, and 
return NULL + my_printf()* a message if the space is exhausted.

* printf() can overflow kernel stack according to Al Viro, see 
not_configged_chan messages.


> > On the other side, could you explain why you don't like kmalloc in first
> > place? It surely works.

> I'm not sure - let me think about it.  But I fixed that for a reason - it
> was causing a crash somehow, I just don't remember the details.

> > The real solution for this warning is to replace um_kmalloc with
> > malloc(), and set, during shutdown, kmalloc_only_atomic - which would
> > switch
> > __wrap_malloc() from um_kmalloc to um_kmalloc_atomic.

> Yeah, that's userspace code, so it should probably just use malloc.

> > Or better yet, simply test in_atomic() and irqs_disabled() to choose
> > between the atomic and normal versions.

> I don't really like testing in_atomic() because you should generally know
> what context you're running in.
Beyond that, in_atomic() isn't set by spinlocks (at least not normally - it is 
set when preemption is enabled, not for us).

Userspace code calling malloc could miss this knowledge (as in the example 
above). Also userspace is supposed to use malloc, and with malloc there's no 
choice. And finally, surely libc isn't aware of anything such.
-- 
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! Mail: gratis 1GB per i messaggi e allegati da 10MB 
http://mail.yahoo.it



-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [uml-devel] Clearing kmalloc_ok during shutdown is broken - malloc will clear our data.
  2005-09-28 21:31 ` Jeff Dike
@ 2005-09-29 14:14   ` Blaisorblade
  2005-10-02  1:08     ` Jeff Dike
  0 siblings, 1 reply; 10+ messages in thread
From: Blaisorblade @ 2005-09-29 14:14 UTC (permalink / raw)
  To: user-mode-linux-devel, Allan Graves; +Cc: Jeff Dike

On Wednesday 28 September 2005 23:31, Jeff Dike wrote:
> On Wed, Sep 28, 2005 at 01:46:15PM +0200, Blaisorblade wrote:
> > Also, there are some calls to kmalloc in the shutdown path - and they
> > work. I know this because I saw a problem with one of them: it gave
> > "might_sleep while atomic", and it was kmalloc in the shutdown, or
> > rather, in panic() - for the broken sysrq t (where's the fix you
> > promised?).

> Attached.

I now even found (by chance) the original mail from Allan Graves - and the 
changes in arch/um/include/sysdep-x86_64/ptrace.h weren't in his patch and 
are unrelated.

Plus, I think they're also bogus (those registers exist), but I may be wrong, 
I'm not looking at these unrelated changes since they pertain to something 
else and I've no changelog.

On the patch: it makes sense, and the "XXX" removed comment, which explained 
the bug, was added by myself.

On the bogus value: I'm more accustomed to 0xdeadbeef, since 0xbadbabe could 
be valid while 0xdeadbeef not (it's in the last giga).

The only problem I see is that we need to test it on a wide glibc range - 
you're using an internal header detail, so glibc will break it at will.

Sadly, we haven't a better fix (except reimplementing setjmp(), which is even 
worse than the original problem - plus I don't like glibc sources enough to 
go picking the 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! Mail: gratis 1GB per i messaggi e allegati da 10MB 
http://mail.yahoo.it



-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [uml-devel] Clearing kmalloc_ok during shutdown is broken - malloc will clear our data.
  2005-09-29 14:14   ` Blaisorblade
@ 2005-10-02  1:08     ` Jeff Dike
  2005-10-02 10:31       ` Blaisorblade
  0 siblings, 1 reply; 10+ messages in thread
From: Jeff Dike @ 2005-10-02  1:08 UTC (permalink / raw)
  To: Blaisorblade; +Cc: user-mode-linux-devel, Allan Graves

On Thu, Sep 29, 2005 at 04:14:02PM +0200, Blaisorblade wrote:
> I now even found (by chance) the original mail from Allan Graves - and the 
> changes in arch/um/include/sysdep-x86_64/ptrace.h weren't in his patch and 
> are unrelated.
> Plus, I think they're also bogus (those registers exist), but I may be wrong, 
The patch uses UPT_REG apparently for the first time.  Those registers exist,
but there are no defines for them in the x86_64 ptrace.h.  UPT_REG is never
called with any of those as its argument, so it's easy to just remove those
cases.

> On the bogus value: I'm more accustomed to 0xdeadbeef, since 0xbadbabe could 
> be valid while 0xdeadbeef not (it's in the last giga).

Yeah, that makes sense.

> The only problem I see is that we need to test it on a wide glibc range - 
> you're using an internal header detail, so glibc will break it at will.

Yeah, it's bad.  The other way to do it is to explictly save the registers
in the thread struct, which is effectively the reimplementing setjmp option
which you mentioned.

				Jeff



-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [uml-devel] Clearing kmalloc_ok during shutdown is broken - malloc will clear our data.
  2005-10-02  1:08     ` Jeff Dike
@ 2005-10-02 10:31       ` Blaisorblade
  2005-10-02 18:27         ` Jeff Dike
  2005-10-03 13:40         ` Allan Graves
  0 siblings, 2 replies; 10+ messages in thread
From: Blaisorblade @ 2005-10-02 10:31 UTC (permalink / raw)
  To: Jeff Dike; +Cc: user-mode-linux-devel, Allan Graves

On Sunday 02 October 2005 03:08, Jeff Dike wrote:
> On Thu, Sep 29, 2005 at 04:14:02PM +0200, Blaisorblade wrote:
> > I now even found (by chance) the original mail from Allan Graves - and
> > the changes in arch/um/include/sysdep-x86_64/ptrace.h weren't in his
> > patch and are unrelated.
> > Plus, I think they're also bogus (those registers exist), but I may be
> > wrong,

> The patch uses UPT_REG apparently for the first time.  Those registers
> exist, but there are no defines for them in the x86_64 ptrace.h.  UPT_REG
> is never called with any of those as its argument, so it's easy to just
> remove those cases.
Ah, ok.

> > The only problem I see is that we need to test it on a wide glibc range -
> > you're using an internal header detail, so glibc will break it at will.

> Yeah, it's bad.  The other way to do it is to explictly save the registers
> in the thread struct, which is effectively the reimplementing setjmp option
> which you mentioned.
At least, if we save them separately from the jmpbuf_t, we can use them for 
sysrq t, without reimplementing setjmp() and longjmp(). Not nice, wastes 24 
bytes, but would work.

I have the doubt that the location of those registers is part of the ABI, 
(pending: find an example where I can be statically linked to glibc and 
dynamically linked to a library dynamically linked to glibc, and must pass 
jmpbuf_t between the two implementations)
even if the C names aren't part of the API, so we could copy the structure.

Probably, however, it's just better to test on, say, a Slackware 8.1, and hope 
for the best and go doing a fix when things change.
-- 
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! Mail: gratis 1GB per i messaggi e allegati da 10MB 
http://mail.yahoo.it



-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [uml-devel] Clearing kmalloc_ok during shutdown is broken - malloc will clear our data.
  2005-10-02 10:31       ` Blaisorblade
@ 2005-10-02 18:27         ` Jeff Dike
  2005-10-03 13:40         ` Allan Graves
  1 sibling, 0 replies; 10+ messages in thread
From: Jeff Dike @ 2005-10-02 18:27 UTC (permalink / raw)
  To: Blaisorblade; +Cc: user-mode-linux-devel, Allan Graves

On Sun, Oct 02, 2005 at 12:31:04PM +0200, Blaisorblade wrote:
> > Yeah, it's bad.  The other way to do it is to explictly save the registers
> > in the thread struct, which is effectively the reimplementing setjmp option
> > which you mentioned.
> At least, if we save them separately from the jmpbuf_t, we can use them for 
> sysrq t, without reimplementing setjmp() and longjmp(). Not nice, wastes 24 
> bytes, but would work.

Yup.

> I have the doubt that the location of those registers is part of the ABI, 

I think they are probably not.

> Probably, however, it's just better to test on, say, a Slackware 8.1, and hope 
> for the best and go doing a fix when things change.

Yeah, I agree.  On the positive side, I think that jmp_buf has probably been
unchanged for a long time, and will remain so.

				Jeff


-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [uml-devel] Clearing kmalloc_ok during shutdown is broken - malloc will clear our data.
  2005-10-02 10:31       ` Blaisorblade
  2005-10-02 18:27         ` Jeff Dike
@ 2005-10-03 13:40         ` Allan Graves
  2005-10-03 18:48           ` Blaisorblade
  1 sibling, 1 reply; 10+ messages in thread
From: Allan Graves @ 2005-10-03 13:40 UTC (permalink / raw)
  To: Blaisorblade; +Cc: Jeff Dike, user-mode-linux-devel

I'm not understanding why glibc would break the registers at will.
 From setjmp:
    /* NOTE: The machine-dependent definitions of `__sigsetjmp'
       assume that a `jmp_buf' begins with a `__jmp_buf' and that
       `__mask_was_saved' follows it.  Do not move these members
       or add others before it.  */

Seems to indicate to me that this isn't gonna change, and I'm using the 
bits/setjmp.h defines, so if they do change, the code should just follow 
along with the change.  Am I missing something?
Allan

Blaisorblade wrote:

>On Sunday 02 October 2005 03:08, Jeff Dike wrote:
>  
>
>>On Thu, Sep 29, 2005 at 04:14:02PM +0200, Blaisorblade wrote:
>>    
>>
>>>I now even found (by chance) the original mail from Allan Graves - and
>>>the changes in arch/um/include/sysdep-x86_64/ptrace.h weren't in his
>>>patch and are unrelated.
>>>Plus, I think they're also bogus (those registers exist), but I may be
>>>wrong,
>>>      
>>>
>
>  
>
>>The patch uses UPT_REG apparently for the first time.  Those registers
>>exist, but there are no defines for them in the x86_64 ptrace.h.  UPT_REG
>>is never called with any of those as its argument, so it's easy to just
>>remove those cases.
>>    
>>
>Ah, ok.
>
>  
>
>>>The only problem I see is that we need to test it on a wide glibc range -
>>>you're using an internal header detail, so glibc will break it at will.
>>>      
>>>
>
>  
>
>>Yeah, it's bad.  The other way to do it is to explictly save the registers
>>in the thread struct, which is effectively the reimplementing setjmp option
>>which you mentioned.
>>    
>>
>At least, if we save them separately from the jmpbuf_t, we can use them for 
>sysrq t, without reimplementing setjmp() and longjmp(). Not nice, wastes 24 
>bytes, but would work.
>
>I have the doubt that the location of those registers is part of the ABI, 
>(pending: find an example where I can be statically linked to glibc and 
>dynamically linked to a library dynamically linked to glibc, and must pass 
>jmpbuf_t between the two implementations)
>even if the C names aren't part of the API, so we could copy the structure.
>
>Probably, however, it's just better to test on, say, a Slackware 8.1, and hope 
>for the best and go doing a fix when things change.
>  
>


-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [uml-devel] Clearing kmalloc_ok during shutdown is broken - malloc will clear our data.
  2005-10-03 13:40         ` Allan Graves
@ 2005-10-03 18:48           ` Blaisorblade
  0 siblings, 0 replies; 10+ messages in thread
From: Blaisorblade @ 2005-10-03 18:48 UTC (permalink / raw)
  To: Allan Graves; +Cc: Jeff Dike, user-mode-linux-devel

On Monday 03 October 2005 15:40, Allan Graves wrote:
> I'm not understanding why glibc would break the registers at will.
>  From setjmp:
>     /* NOTE: The machine-dependent definitions of `__sigsetjmp'
>        assume that a `jmp_buf' begins with a `__jmp_buf' and that
>        `__mask_was_saved' follows it.  Do not move these members
>        or add others before it.  */

> Seems to indicate to me that this isn't gonna change,
the comments above are mostly relative to the definitions of __sigsetjmp, i.e. 
to glibc internal code. Also, it's related to the layout of the structure, 
while our main problem is not the structure layout.
> and I'm using the 
> bits/setjmp.h defines, so if they do change, the code should just follow
> along with the change.  Am I missing something?
I didn't even see those constants - I stopped earlier, at __jmp_buf_tag. Given 
that it feels like a Glibc private thing, I worried.

Since those constants are explicitly exported, I guess that's for userspace 
programs as well, so Glibc provides that API as a public one.
-- 
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! Mail: gratis 1GB per i messaggi e allegati da 10MB 
http://mail.yahoo.it



-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2005-10-03 18:48 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-09-28 11:46 [uml-devel] Clearing kmalloc_ok during shutdown is broken - malloc will clear our data Blaisorblade
2005-09-28 20:12 ` Jeff Dike
2005-09-29 12:07   ` Blaisorblade
2005-09-28 21:31 ` Jeff Dike
2005-09-29 14:14   ` Blaisorblade
2005-10-02  1:08     ` Jeff Dike
2005-10-02 10:31       ` Blaisorblade
2005-10-02 18:27         ` Jeff Dike
2005-10-03 13:40         ` Allan Graves
2005-10-03 18:48           ` Blaisorblade

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.