2007-09-22 04:41:03

by lepton

[permalink] [raw]
Subject: [PATCH] 2.6.22.6 user-mode linux: use address instead of value as argument in os_free_irq_by_cb

Hi,
There is a bug in os_free_irq_by_cb, when the first element
of active_fds list is free, the value of active_fds is not
updated, just value in stack is updated.

The intresting thing is that without this patch, a poweroff
in user mode linux guest will halt the host linux system.It
seems that after the tracing thread is dead, the syscall to
sys_reboot of the traced thread is executed by host. I don't
know if it is another bug.

Signed-off-by: Lepton Wu <[email protected]>

diff -X linux-2.6.22.6/Documentation/dontdiff -pru linux-2.6.22.6/arch/um/include/os.h linux-2.6.22.6-lepton/arch/um/include/os.h
--- linux-2.6.22.6/arch/um/include/os.h 2007-09-14 17:41:10.000000000 +0800
+++ linux-2.6.22.6-lepton/arch/um/include/os.h 2007-09-22 12:15:28.000000000 +0800
@@ -325,7 +325,7 @@ extern void reboot_skas(void);
extern int os_waiting_for_events(struct irq_fd *active_fds);
extern int os_create_pollfd(int fd, int events, void *tmp_pfd, int size_tmpfds);
extern void os_free_irq_by_cb(int (*test)(struct irq_fd *, void *), void *arg,
- struct irq_fd *active_fds, struct irq_fd ***last_irq_ptr2);
+ struct irq_fd **active_fds_ptr, struct irq_fd ***last_irq_ptr2);
extern void os_free_irq_later(struct irq_fd *active_fds,
int irq, void *dev_id);
extern int os_get_pollfd(int i);
diff -X linux-2.6.22.6/Documentation/dontdiff -pru linux-2.6.22.6/arch/um/kernel/irq.c linux-2.6.22.6-lepton/arch/um/kernel/irq.c
--- linux-2.6.22.6/arch/um/kernel/irq.c 2007-09-14 17:41:10.000000000 +0800
+++ linux-2.6.22.6-lepton/arch/um/kernel/irq.c 2007-09-22 12:15:05.000000000 +0800
@@ -218,7 +218,7 @@ static void free_irq_by_cb(int (*test)(s
unsigned long flags;

spin_lock_irqsave(&irq_lock, flags);
- os_free_irq_by_cb(test, arg, active_fds, &last_irq_ptr);
+ os_free_irq_by_cb(test, arg, &active_fds, &last_irq_ptr);
spin_unlock_irqrestore(&irq_lock, flags);
}

diff -X linux-2.6.22.6/Documentation/dontdiff -pru linux-2.6.22.6/arch/um/os-Linux/irq.c linux-2.6.22.6-lepton/arch/um/os-Linux/irq.c
--- linux-2.6.22.6/arch/um/os-Linux/irq.c 2007-09-14 17:41:10.000000000 +0800
+++ linux-2.6.22.6-lepton/arch/um/os-Linux/irq.c 2007-09-22 12:15:42.000000000 +0800
@@ -84,12 +84,12 @@ int os_create_pollfd(int fd, int events,
}

void os_free_irq_by_cb(int (*test)(struct irq_fd *, void *), void *arg,
- struct irq_fd *active_fds, struct irq_fd ***last_irq_ptr2)
+ struct irq_fd **active_fds_ptr, struct irq_fd ***last_irq_ptr2)
{
struct irq_fd **prev;
int i = 0;

- prev = &active_fds;
+ prev = active_fds_ptr;
while (*prev != NULL) {
if ((*test)(*prev, arg)) {
struct irq_fd *old_fd = *prev;


2007-09-24 17:02:40

by Jeff Dike

[permalink] [raw]
Subject: Re: [PATCH] 2.6.22.6 user-mode linux: use address instead of value as argument in os_free_irq_by_cb

On Sat, Sep 22, 2007 at 12:39:55PM +0800, lepton wrote:
> There is a bug in os_free_irq_by_cb, when the first element
> of active_fds list is free, the value of active_fds is not
> updated, just value in stack is updated.

Man, that sucks.

Nice spotting.

>
> The intresting thing is that without this patch, a poweroff
> in user mode linux guest will halt the host linux system.It
> seems that after the tracing thread is dead, the syscall to
> sys_reboot of the traced thread is executed by host. I don't
> know if it is another bug.

I would say that you shouldn't be running UMLs as root. However, a
sys_reboot shouldn't escape being ptraced either. This is a bug, but
I don't see any connection between it and ptrace missing a final
system call.

Jeff

--
Work email - jdike at linux dot intel dot com

2007-09-25 01:13:34

by lepton

[permalink] [raw]
Subject: Re: [PATCH] 2.6.22.6 user-mode linux: use address instead of value as argument in os_free_irq_by_cb

I added dump_stack and some printk in host kernel. The following is what
I got when sys_reboot in host kernel is called, the first line is
printing the process state and ptrace state and pid of the calling
process. the following is the call path.

Sep 22 14:25:49 pc kernel: linux Rptrace: 00000000 pid:3698
Sep 22 14:25:49 pc kernel: [<c01234a7>] kernel_halt+0x8/0x24
Sep 22 14:25:49 pc kernel: [<c01239b2>] sys_reboot+0x14c/0x1c7
Sep 22 14:25:49 pc kernel: [<c0120712>] recalc_sigpending+0xb/0x1d
Sep 22 14:25:49 pc kernel: [<c0121ed2>] dequeue_signal+0x9d/0x11b
Sep 22 14:25:49 pc kernel: [<c012222b>]
get_signal_to_deliver+0xe1/0x389
Sep 22 14:25:49 pc kernel: [<c0101bd5>] do_notify_resume+0x84/0x5f1
Sep 22 14:25:49 pc kernel: [<c012212c>] ptrace_notify+0x6f/0x8d
Sep 22 14:25:49 pc kernel: [<c010249e>] syscall_call+0x7/0xb
Sep 22 14:25:49 pc kernel: =======================
Sep 22 14:25:50 pc kernel: sd 1:0:0:0: [sda] Synchronizing SCSI cache
Sep 22 14:25:50 pc kernel: sd 1:0:0:0: [sda] Stopping disk
Sep 22 14:25:51 pc kernel: System halted.

On Mon, Sep 24, 2007 at 01:02:20PM -0400, Jeff Dike wrote:
> I would say that you shouldn't be running UMLs as root. However, a
> sys_reboot shouldn't escape being ptraced either. This is a bug, but
> I don't see any connection between it and ptrace missing a final
> system call.
>
> Jeff
>
> --
> Work email - jdike at linux dot intel dot com