user-mode-linux-devel

Re: [uml-devel] IRQ handler reentrancy

[Anton I. <ant...@ko...>]

Patch in your queue, current code looks OK.

I will try to assemble the full ubd acceleration sequence of patches 
this afternoon to submit that as well.

A.

On 11/12/15 18:38, Richard Weinberger wrote:
> Am 11.12.2015 um 12:24 schrieb Anton Ivanov:
>> On 11/12/15 08:16, Richard Weinberger wrote:
>>> Am 11.12.2015 um 07:58 schrieb Anton Ivanov:
>>>>>> 2. I cannot catch what is wrong with the current code in signal.c. When
>>>>>> I read it, it should not produce re-entrancy. But it does.
>>>>> Sorry for the delay. Until now I did not find the time to dig into that.
>>>>> Did you find the offending code in signal.c?
>>>> Yes.
>>>>
>>>> Unblock signals is logically incorrect - it will re-trigger an
>>>> interrupts even if there is an interrupt in flight whose processing has
>>>> not been finished.
>>>>
>>>> I tried several approaches both with the original poll() controller and
>>>> with my epoll() based version, some show promise.
>>>>
>>>> I had to put it aside until next Friday as I have some stuff due at work
>>>> so I cannot spare time to work on it until then. Once I get that out of
>>>> the way I should be able to spare it a day or two which should be enough
>>>> to finish it.
>>>>
>>>> Ditto for the UBD improvements.
>>> One thing we have to consider is that's legit to have SIGIO nested.
>> Correct. That is considered :)
>>
>> Both when looking at poll() and epoll()
>>
>> However, it is not legit to have sigio on a specific fd nested. That is mostly safe for the poll() version, but will need to be accounted for in any surgery on the irq controller.
> So, the current code is fine unless you switch to epoll()?
> Is it because you use epoll() in edge-triggered mode?
>
> Thanks,
> //richard
>

[uml-devel] [PATCH 2/2] Optimization - do not change hard IRQ flags in soft IRQ processing

[Anton I. <ai...@br...>]

Software IRQ processing in generic architectures assumes that the
exit out of hard IRQ may have re-enabled interrupts (some
architectures may have an implicit EOI). It presumes them enabled
and toggles the flags once more just in case unless this is turned
off in the architecture specific hardirq.h by setting
__ARCH_IRQ_EXIT_IRQS_DISABLED

This patch adds this to UML where due to the way IRQs are handled
it is an optimization (it works fine without it too).

Signed-off-by: Anton Ivanov <ai...@br...>
---
 arch/um/include/asm/hardirq.h | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)
 create mode 100644 arch/um/include/asm/hardirq.h

diff --git a/arch/um/include/asm/hardirq.h b/arch/um/include/asm/hardirq.h
new file mode 100644
index 0000000..756f077
--- /dev/null
+++ b/arch/um/include/asm/hardirq.h
@@ -0,0 +1,23 @@
+#ifndef __ASM_UM_HARDIRQ_H
+#define __ASM_UM_HARDIRQ_H
+
+#include <linux/cache.h>
+#include <linux/threads.h>
+
+typedef struct {
+	unsigned int __softirq_pending;
+} ____cacheline_aligned irq_cpustat_t;
+
+#include <linux/irq_cpustat.h>	/* Standard mappings for irq_cpustat_t above */
+#include <linux/irq.h>
+
+#ifndef ack_bad_irq
+static inline void ack_bad_irq(unsigned int irq)
+{
+	printk(KERN_CRIT "unexpected IRQ trap at vector %02x\n", irq);
+}
+#endif
+
+#define __ARCH_IRQ_EXIT_IRQS_DISABLED 1
+
+#endif /* __ASM_UM_HARDIRQ_H */
-- 
2.1.4

[uml-devel] [PATCH 1/2] Prevent IRQ handler reentrancy

[Anton I. <ai...@br...>]

The existing IRQ handler design in UML does not prevent reentrancy

This is mitigated by fd-enable/fd-disable semantics for the IO
portion of the UML subsystem. The timer, however, can and is
re-entered resulting in very deep stack usage and occasional
stack exhaustion.

This patch prevents this by checking if there is a timer
interrupt in-flight before processing any pending timer interrupts.

Signed-off-by: Anton Ivanov <ai...@br...>
---
 arch/um/os-Linux/signal.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/arch/um/os-Linux/signal.c b/arch/um/os-Linux/signal.c
index c211153..7801666 100644
--- a/arch/um/os-Linux/signal.c
+++ b/arch/um/os-Linux/signal.c
@@ -62,6 +62,7 @@ static void sig_handler_common(int sig, struct siginfo *si, mcontext_t *mc)
 
 static int signals_enabled;
 static unsigned int signals_pending;
+static unsigned int signals_active = 0;
 
 void sig_handler(int sig, struct siginfo *si, mcontext_t *mc)
 {
@@ -101,7 +102,12 @@ void timer_alarm_handler(int sig, struct siginfo *unused_si, mcontext_t *mc)
 
 	block_signals();
 
+	signals_active |= SIGALRM_MASK;
+
 	timer_real_alarm_handler(mc);
+
+	signals_active &= ~SIGALRM_MASK;
+
 	set_signals(enabled);
 }
 
@@ -286,8 +292,16 @@ void unblock_signals(void)
 		if (save_pending & SIGIO_MASK)
 			sig_handler_common(SIGIO, NULL, NULL);
 
-		if (save_pending & SIGALRM_MASK)
+		/* Do not reenter the handler */
+
+		if ((save_pending & SIGALRM_MASK) && (!(signals_active & SIGALRM_MASK)))
 			timer_real_alarm_handler(NULL);
+
+		/* Rerun the loop only if there is still pending SIGIO and not in TIMER handler */
+
+		if (!(signals_pending & SIGIO_MASK) && (signals_active & SIGALRM_MASK))
+			return;
+
 	}
 }
 
-- 
2.1.4

[uml-devel] [PATCH] uml: flush stdout before forking

[Vegard N. <veg...@or...>]

I was seeing some really weird behaviour where piping UML's output
somewhere would cause output to get duplicated:

  $ ./vmlinux | head -n 40
  Checking that ptrace can change system call numbers...Core dump limits :
          soft - 0
          hard - NONE
  OK
  Checking syscall emulation patch for ptrace...Core dump limits :
          soft - 0
          hard - NONE
  OK
  Checking advanced syscall emulation patch for ptrace...Core dump limits :
          soft - 0
          hard - NONE
  OK
  Core dump limits :
          soft - 0
          hard - NONE

This is because these tests do a fork() which duplicates the non-empty
stdout buffer, then glibc flushes the duplicated buffer as each child
exits.

A simple workaround is to flush before forking.

Signed-off-by: Vegard Nossum <veg...@or...>
---
 arch/um/os-Linux/start_up.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/um/os-Linux/start_up.c b/arch/um/os-Linux/start_up.c
index 47f1ff0..22a358e 100644
--- a/arch/um/os-Linux/start_up.c
+++ b/arch/um/os-Linux/start_up.c
@@ -94,6 +94,8 @@ static int start_ptraced_child(void)
 {
 	int pid, n, status;
 
+	fflush(stdout);
+
 	pid = fork();
 	if (pid == 0)
 		ptrace_child();
-- 
1.9.1

Re: [uml-devel] [PATCH] uml: fix hostfs mknod()

[Vegard N. <veg...@or...>]

On 12/16/2015 11:17 PM, Richard Weinberger wrote:
> Am 16.12.2015 um 21:59 schrieb Vegard Nossum:
>> An inverted return value check in hostfs_mknod() caused the function
>> to return success after handling it as an error (and cleaning up).
>>
[...]
>
> Applied! :-)
>
> BTW: How did you create this patch? I had to apply it by hand using -p0...
> git am didn't like it.

Ah, sorry, I have diff.noprefix=true since I like to double click on
filenames in a diff (there's probably a way to configure the selection
to skip '[ab]/' prefixes when you do that but this was easier). git am
-p0 should work too.

Thanks!


Vegard

Re: [uml-devel] [PATCH] uml: fix hostfs mknod()

[Richard W. <ri...@no...>]

Am 16.12.2015 um 21:59 schrieb Vegard Nossum:
> An inverted return value check in hostfs_mknod() caused the function
> to return success after handling it as an error (and cleaning up).
> 
> It resulted in the following segfault when trying to bind() a named
> unix socket:
> 
>   Pid: 198, comm: a.out Not tainted 4.4.0-rc4
>   RIP: 0033:[<0000000061077df6>]
>   RSP: 00000000daae5d60  EFLAGS: 00010202
>   RAX: 0000000000000000 RBX: 000000006092a460 RCX: 00000000dfc54208
>   RDX: 0000000061073ef1 RSI: 0000000000000070 RDI: 00000000e027d600
>   RBP: 00000000daae5de0 R08: 00000000da980ac0 R09: 0000000000000000
>   R10: 0000000000000003 R11: 00007fb1ae08f72a R12: 0000000000000000
>   R13: 000000006092a460 R14: 00000000daaa97c0 R15: 00000000daaa9a88
>   Kernel panic - not syncing: Kernel mode fault at addr 0x40, ip 0x61077df6
>   CPU: 0 PID: 198 Comm: a.out Not tainted 4.4.0-rc4 #1
>   Stack:
>    e027d620 dfc54208 0000006f da981398
>    61bee000 0000c1ed daae5de0 0000006e
>    e027d620 dfcd4208 00000005 6092a460
>   Call Trace:
>    [<60dedc67>] SyS_bind+0xf7/0x110
>    [<600587be>] handle_syscall+0x7e/0x80
>    [<60066ad7>] userspace+0x3e7/0x4e0
>    [<6006321f>] ? save_registers+0x1f/0x40
>    [<6006c88e>] ? arch_prctl+0x1be/0x1f0
>    [<60054985>] fork_handler+0x85/0x90
> 
> Let's also get rid of the "cosmic ray protection" while we're at it.
> 
> Fixes: e9193059b1b3 "hostfs: fix races in dentry_name() and inode_name()"
> Signed-off-by: Vegard Nossum <veg...@or...>
> Cc: Jeff Dike <jd...@ad...>
> Cc: Al Viro <vi...@ze...>
> Cc: st...@vg...

Applied! :-)

BTW: How did you create this patch? I had to apply it by hand using -p0...
git am didn't like it.

Thanks,
//richard

[uml-devel] [PATCH] uml: fix hostfs mknod()

[Vegard N. <veg...@or...>]

An inverted return value check in hostfs_mknod() caused the function
to return success after handling it as an error (and cleaning up).

It resulted in the following segfault when trying to bind() a named
unix socket:

  Pid: 198, comm: a.out Not tainted 4.4.0-rc4
  RIP: 0033:[<0000000061077df6>]
  RSP: 00000000daae5d60  EFLAGS: 00010202
  RAX: 0000000000000000 RBX: 000000006092a460 RCX: 00000000dfc54208
  RDX: 0000000061073ef1 RSI: 0000000000000070 RDI: 00000000e027d600
  RBP: 00000000daae5de0 R08: 00000000da980ac0 R09: 0000000000000000
  R10: 0000000000000003 R11: 00007fb1ae08f72a R12: 0000000000000000
  R13: 000000006092a460 R14: 00000000daaa97c0 R15: 00000000daaa9a88
  Kernel panic - not syncing: Kernel mode fault at addr 0x40, ip 0x61077df6
  CPU: 0 PID: 198 Comm: a.out Not tainted 4.4.0-rc4 #1
  Stack:
   e027d620 dfc54208 0000006f da981398
   61bee000 0000c1ed daae5de0 0000006e
   e027d620 dfcd4208 00000005 6092a460
  Call Trace:
   [<60dedc67>] SyS_bind+0xf7/0x110
   [<600587be>] handle_syscall+0x7e/0x80
   [<60066ad7>] userspace+0x3e7/0x4e0
   [<6006321f>] ? save_registers+0x1f/0x40
   [<6006c88e>] ? arch_prctl+0x1be/0x1f0
   [<60054985>] fork_handler+0x85/0x90

Let's also get rid of the "cosmic ray protection" while we're at it.

Fixes: e9193059b1b3 "hostfs: fix races in dentry_name() and inode_name()"
Signed-off-by: Vegard Nossum <veg...@or...>
Cc: Jeff Dike <jd...@ad...>
Cc: Al Viro <vi...@ze...>
Cc: st...@vg...
---
 fs/hostfs/hostfs_kern.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git fs/hostfs/hostfs_kern.c fs/hostfs/hostfs_kern.c
index 2ac99db..5a7b322 100644
--- fs/hostfs/hostfs_kern.c
+++ fs/hostfs/hostfs_kern.c
@@ -730,15 +730,13 @@ static int hostfs_mknod(struct inode *dir, struct dentry *dentry, umode_t mode,
 
 	init_special_inode(inode, mode, dev);
 	err = do_mknod(name, mode, MAJOR(dev), MINOR(dev));
-	if (!err)
+	if (err)
 		goto out_free;
 
 	err = read_name(inode, name);
 	__putname(name);
 	if (err)
 		goto out_put;
-	if (err)
-		goto out_put;
 
 	d_instantiate(dentry, inode);
 	return 0;
-- 
1.9.1

Re: [uml-devel] [PATCH v3 1/2] um: Do not set unsecure permission for temporary file

[Tristan S. <tsc...@go...>]

Acked-by: Tristan Schmelcher <tsc...@go...>

Re: [uml-devel] IRQ handler reentrancy

[Anton I. <ant...@ko...>]

On 11/12/15 18:38, Richard Weinberger wrote:
> Am 11.12.2015 um 12:24 schrieb Anton Ivanov:
>> On 11/12/15 08:16, Richard Weinberger wrote:
>>> Am 11.12.2015 um 07:58 schrieb Anton Ivanov:
>>>>>> 2. I cannot catch what is wrong with the current code in signal.c. When
>>>>>> I read it, it should not produce re-entrancy. But it does.
>>>>> Sorry for the delay. Until now I did not find the time to dig into that.
>>>>> Did you find the offending code in signal.c?
>>>> Yes.
>>>>
>>>> Unblock signals is logically incorrect - it will re-trigger an
>>>> interrupts even if there is an interrupt in flight whose processing has
>>>> not been finished.
>>>>
>>>> I tried several approaches both with the original poll() controller and
>>>> with my epoll() based version, some show promise.
>>>>
>>>> I had to put it aside until next Friday as I have some stuff due at work
>>>> so I cannot spare time to work on it until then. Once I get that out of
>>>> the way I should be able to spare it a day or two which should be enough
>>>> to finish it.
>>>>
>>>> Ditto for the UBD improvements.
>>> One thing we have to consider is that's legit to have SIGIO nested.
>> Correct. That is considered :)
>>
>> Both when looking at poll() and epoll()
>>
>> However, it is not legit to have sigio on a specific fd nested. That is mostly safe for the poll() version, but will need to be accounted for in any surgery on the irq controller.
> So, the current code is fine unless you switch to epoll()?

Correct. It looks OK, though I have not looked at it in depth to make 
sure there is no race somewhere.

> Is it because you use epoll() in edge-triggered mode?

For now it is level which means it should be identical to poll().

I want to be able (long term) to use either - have the user set what 
they want for a particular IRQ. There are a couple of use cases like 
packet mmap and can-write IRQs where edge allows for better code.

I have a working POC which sets per - fd flag to avoid reentrancy on a 
per-fd basis so it behaves identically to the poll one - similar to the 
"reactivate fd" semantics.

What I see with it however is some weird stalls on ubd and I am not 100% 
sure that they are epoll specific, it just tends to make them easily 
reproducible as it is faster.

I will pick it Monday-Tuesday the week before Xmas after I get some work 
stuff out of the way.

A.

>
> Thanks,
> //richard
>

Re: [uml-devel] IRQ handler reentrancy

[Richard W. <ri...@no...>]

Am 11.12.2015 um 12:24 schrieb Anton Ivanov:
> On 11/12/15 08:16, Richard Weinberger wrote:
>> Am 11.12.2015 um 07:58 schrieb Anton Ivanov:
>>>>> 2. I cannot catch what is wrong with the current code in signal.c. When
>>>>> I read it, it should not produce re-entrancy. But it does.
>>>> Sorry for the delay. Until now I did not find the time to dig into that.
>>>> Did you find the offending code in signal.c?
>>> Yes.
>>>
>>> Unblock signals is logically incorrect - it will re-trigger an
>>> interrupts even if there is an interrupt in flight whose processing has
>>> not been finished.
>>>
>>> I tried several approaches both with the original poll() controller and
>>> with my epoll() based version, some show promise.
>>>
>>> I had to put it aside until next Friday as I have some stuff due at work
>>> so I cannot spare time to work on it until then. Once I get that out of
>>> the way I should be able to spare it a day or two which should be enough
>>> to finish it.
>>>
>>> Ditto for the UBD improvements.
>> One thing we have to consider is that's legit to have SIGIO nested.
> 
> Correct. That is considered :)
> 
> Both when looking at poll() and epoll()
> 
> However, it is not legit to have sigio on a specific fd nested. That is mostly safe for the poll() version, but will need to be accounted for in any surgery on the irq controller.

So, the current code is fine unless you switch to epoll()?
Is it because you use epoll() in edge-triggered mode?

Thanks,
//richard

Re: [uml-devel] IRQ handler reentrancy

[Anton I. <ant...@ko...>]

On 11/12/15 08:16, Richard Weinberger wrote:
> Am 11.12.2015 um 07:58 schrieb Anton Ivanov:
>>>> 2. I cannot catch what is wrong with the current code in signal.c. When
>>>> I read it, it should not produce re-entrancy. But it does.
>>> Sorry for the delay. Until now I did not find the time to dig into that.
>>> Did you find the offending code in signal.c?
>> Yes.
>>
>> Unblock signals is logically incorrect - it will re-trigger an
>> interrupts even if there is an interrupt in flight whose processing has
>> not been finished.
>>
>> I tried several approaches both with the original poll() controller and
>> with my epoll() based version, some show promise.
>>
>> I had to put it aside until next Friday as I have some stuff due at work
>> so I cannot spare time to work on it until then. Once I get that out of
>> the way I should be able to spare it a day or two which should be enough
>> to finish it.
>>
>> Ditto for the UBD improvements.
> One thing we have to consider is that's legit to have SIGIO nested.

Correct. That is considered :)

Both when looking at poll() and epoll()

However, it is not legit to have sigio on a specific fd nested. That is 
mostly safe for the poll() version, but will need to be accounted for in 
any surgery on the irq controller.

A.

> I'm currently investigating whether we use do_IRQ() correctly.
>
> Thanks,
> //richard
>

Re: [uml-devel] IRQ handler reentrancy

[Richard W. <ri...@no...>]

Am 11.12.2015 um 07:58 schrieb Anton Ivanov:
>>> 2. I cannot catch what is wrong with the current code in signal.c. When
>>> I read it, it should not produce re-entrancy. But it does.
>> Sorry for the delay. Until now I did not find the time to dig into that.
>> Did you find the offending code in signal.c?
> 
> Yes.
> 
> Unblock signals is logically incorrect - it will re-trigger an
> interrupts even if there is an interrupt in flight whose processing has
> not been finished.
> 
> I tried several approaches both with the original poll() controller and
> with my epoll() based version, some show promise.
> 
> I had to put it aside until next Friday as I have some stuff due at work
> so I cannot spare time to work on it until then. Once I get that out of
> the way I should be able to spare it a day or two which should be enough
> to finish it.
> 
> Ditto for the UBD improvements.

One thing we have to consider is that's legit to have SIGIO nested.
I'm currently investigating whether we use do_IRQ() correctly.

Thanks,
//richard

Re: [uml-devel] IRQ handler reentrancy

[Anton I. <ant...@ko...>]

On 10/12/15 22:40, Richard Weinberger wrote:
> On Fri, Nov 20, 2015 at 1:05 PM, Anton Ivanov
> <ant...@ko...> wrote:
>> I have gotten to the bottom of this.
>>
>> 1. The IRQ handler re-entrancy issue predates the timer patch. Adding a
>> simple guard with a WARN_ON_ONCE around the device loop in the
>> sig_io_handler catches it in plain 4.3
>>
>> diff --git a/arch/um/kernel/irq.c b/arch/um/kernel/irq.c
>> index 23cb935..ac0bbce 100644
>> --- a/arch/um/kernel/irq.c
>> +++ b/arch/um/kernel/irq.c
>> @@ -30,12 +30,17 @@ static struct irq_fd **last_irq_ptr = &active_fds;
>>
>>   extern void free_irqs(void);
>>
>> +static int in_poll_handler = 0;
>> +
>>   void sigio_handler(int sig, struct siginfo *unused_si, struct
>> uml_pt_regs *regs)
>>   {
>>          struct irq_fd *irq_fd;
>>          int n;
>>
>> +    WARN_ON_ONCE(in_poll_handler == 1);
>> +
>>          while (1) {
>> +        in_poll_handler = 1;
>>                  n = os_waiting_for_events(active_fds);
>>                  if (n <= 0) {
>>                          if (n == -EINTR)
>> @@ -51,6 +56,7 @@ void sigio_handler(int sig, struct siginfo *unused_si,
>> struct uml_pt_regs *regs)
>>                          }
>>                  }
>>          }
>> +    in_poll_handler = 0;
>>
>>          free_irqs();
>>   }
>>
>> This is dangerously broken - you can under heavy IO exhaust the stack,
>> you can get packets out of order, etc. Most IO is reasonably atomic so
>> corruption is not likely, but not impossible (especially if one or more
>> drivers are optimized to use multi-read/multi-write).
>>
>> 2. I cannot catch what is wrong with the current code in signal.c. When
>> I read it, it should not produce re-entrancy. But it does.
> Sorry for the delay. Until now I did not find the time to dig into that.
> Did you find the offending code in signal.c?

Yes.

Unblock signals is logically incorrect - it will re-trigger an
interrupts even if there is an interrupt in flight whose processing has
not been finished.

I tried several approaches both with the original poll() controller and
with my epoll() based version, some show promise.

I had to put it aside until next Friday as I have some stuff due at work
so I cannot spare time to work on it until then. Once I get that out of
the way I should be able to spare it a day or two which should be enough
to finish it.

Ditto for the UBD improvements.

A.

> I'm also winding my head how to fix this properly (and to verify
> whether your patches are correct).
> This UML code is very very old and a dark corner.
>

Re: [uml-devel] IRQ handler reentrancy

[Richard W. <ric...@gm...>]

On Fri, Nov 20, 2015 at 1:05 PM, Anton Ivanov
<ant...@ko...> wrote:
> I have gotten to the bottom of this.
>
> 1. The IRQ handler re-entrancy issue predates the timer patch. Adding a
> simple guard with a WARN_ON_ONCE around the device loop in the
> sig_io_handler catches it in plain 4.3
>
> diff --git a/arch/um/kernel/irq.c b/arch/um/kernel/irq.c
> index 23cb935..ac0bbce 100644
> --- a/arch/um/kernel/irq.c
> +++ b/arch/um/kernel/irq.c
> @@ -30,12 +30,17 @@ static struct irq_fd **last_irq_ptr = &active_fds;
>
>   extern void free_irqs(void);
>
> +static int in_poll_handler = 0;
> +
>   void sigio_handler(int sig, struct siginfo *unused_si, struct
> uml_pt_regs *regs)
>   {
>          struct irq_fd *irq_fd;
>          int n;
>
> +    WARN_ON_ONCE(in_poll_handler == 1);
> +
>          while (1) {
> +        in_poll_handler = 1;
>                  n = os_waiting_for_events(active_fds);
>                  if (n <= 0) {
>                          if (n == -EINTR)
> @@ -51,6 +56,7 @@ void sigio_handler(int sig, struct siginfo *unused_si,
> struct uml_pt_regs *regs)
>                          }
>                  }
>          }
> +    in_poll_handler = 0;
>
>          free_irqs();
>   }
>
> This is dangerously broken - you can under heavy IO exhaust the stack,
> you can get packets out of order, etc. Most IO is reasonably atomic so
> corruption is not likely, but not impossible (especially if one or more
> drivers are optimized to use multi-read/multi-write).
>
> 2. I cannot catch what is wrong with the current code in signal.c. When
> I read it, it should not produce re-entrancy. But it does.

Sorry for the delay. Until now I did not find the time to dig into that.
Did you find the offending code in signal.c?
I'm also winding my head how to fix this properly (and to verify
whether your patches are correct).
This UML code is very very old and a dark corner.

-- 
Thanks,
//richard

Re: [uml-devel] uml instance crashes when started from script

[<st...@ni...>]

Den 2015-12-06 16:34, skrev Vegard Nossum:
> Hi,
>
> I've been running into some odd crashes when starting my UML instance
> from Python. This is my script:
>
> import subprocess
> subprocess.check_call(['path/to/vmlinux', 'mem=2048M',
> 'rootfstype=hostfs', 'rw', 'init=/bin/bash'])

Can you please share your kernel config file?

Stian

Re: [uml-devel] [GIT PULL] UML fixes for 4.4-rc5

[Richard W. <ri...@no...>]

Linus,

Am 09.12.2015 um 02:35 schrieb Linus Torvalds:
> On Tue, Dec 8, 2015 at 1:39 PM, Richard Weinberger <ri...@no...> wrote:
>>
>> This pull request contains various bug fixes, most of them are
>> fall out from the merge window.
>>
>> Richard Weinberger (2):
>>       um: Fix fpstate handling
> 
> Ugh. This is very ugly. It's apparently the result of commit
> 530e5c827182 ("x86/headers: Make sigcontext pointers bit independent")
> and apparently nobody noticed the uml fallout.
> 
> I've pulled, but I wanted the x86 people involved to be aware of this
> ugly corner. I wonder if there might be some way for uml to continue
> to use that fpstate entry as a pointer, at least when the wordsize
> matches (which it will)?

I agree. One way to get rid of the ugliness would be rewriting UML's
FP code. The code is very old and doing it like x86 does would be a
good thing anyway. But that's nothing I'll do at this stage of development.
That's material for the next merge window.

Thanks,
//richard

Re: [uml-devel] [GIT PULL] UML fixes for 4.4-rc5

[Linus T. <tor...@li...>]

On Tue, Dec 8, 2015 at 1:39 PM, Richard Weinberger <ri...@no...> wrote:
>
> This pull request contains various bug fixes, most of them are
> fall out from the merge window.
>
> Richard Weinberger (2):
>       um: Fix fpstate handling

Ugh. This is very ugly. It's apparently the result of commit
530e5c827182 ("x86/headers: Make sigcontext pointers bit independent")
and apparently nobody noticed the uml fallout.

I've pulled, but I wanted the x86 people involved to be aware of this
ugly corner. I wonder if there might be some way for uml to continue
to use that fpstate entry as a pointer, at least when the wordsize
matches (which it will)?

                   Linus

Re: [uml-devel] uml instance crashes when started from script

[Richard W. <ri...@no...>]

Hi!

Am 06.12.2015 um 16:34 schrieb Vegard Nossum:
> Hi,
> 
> I've been running into some odd crashes when starting my UML instance from Python. This is my script:
> 
> import subprocess
> subprocess.check_call(['path/to/vmlinux', 'mem=2048M', 'rootfstype=hostfs', 'rw', 'init=/bin/bash'])
> 
> This will crash 9 out of 10 times with various strange messages on the console:

Hmm, I cannot reproduce here on my laptop.
Stian, can you?

I wonder what could cause this.
A completely different terminal should not cause a crash.

*confused*,
//richard

Re: [uml-devel] [PATCH v2 1/2] um: Set secure access mode for temporary file

[Richard W. <ri...@no...>]

Am 08.12.2015 um 21:37 schrieb Tristan Schmelcher:
> On 6 December 2015 at 09:43, Mickaël Salaün <mi...@di...> wrote:
>> Well, I'm concerned to use umask because it is not thread-safe and drivers may use create_mem_file() in a multi-theaded context.
> 
> You are right. We should perhaps set the umask to 0700 permanently
> during process start. But I am not sure if this will interfere with
> other UML code.

It *should* not hurt. Let's see what explodes. :)

>> I prefer to stick to fchmod and handle the race-condition with O_TMPFILE unsell someone is sure that this will not create bugs :)
> 
> The fchmod call is basically useless and should probably be removed.

I agree.

Thanks,
//richard

[uml-devel] [GIT PULL] UML fixes for 4.4-rc5

[Richard W. <ri...@no...>]

Linus,

The following changes since commit 527e9316f8ec44bd53d90fb9f611fa7ffff52bb9:

  Linux 4.4-rc4 (2015-12-06 15:43:12 -0800)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/rw/uml.git for-linus-4.4-rc5

for you to fetch changes up to 887a9853092c09e20598f4a7f91ac1cfb762be50:

  um: fix returns without va_end (2015-12-08 22:26:00 +0100)

----------------------------------------------------------------
This pull request contains various bug fixes, most of them are
fall out from the merge window.

----------------------------------------------------------------
Geyslan G. Bem (1):
      um: fix returns without va_end

Lorenzo Colitti (1):
      arch: um: fix error when linking vmlinux.

Richard Weinberger (2):
      um: Fix get_signal() usage
      um: Fix fpstate handling

 arch/um/Makefile           |  2 +-
 arch/um/drivers/net_user.c | 10 ++++++----
 arch/um/kernel/signal.c    |  2 +-
 arch/x86/um/signal.c       | 18 ++++++++++--------
 scripts/link-vmlinux.sh    |  2 +-
 5 files changed, 19 insertions(+), 15 deletions(-)

Re: [uml-devel] [PATCH] arch: um: fix error when linking vmlinux.

[Richard W. <ric...@gm...>]

On Wed, Nov 18, 2015 at 3:12 PM, Lorenzo Colitti <lo...@go...> wrote:
> On gcc Ubuntu 4.8.4-2ubuntu1~14.04, linking vmlinux fails with:
>
> arch/um/os-Linux/built-in.o: In function `os_timer_create':
> /android/kernel/android/arch/um/os-Linux/time.c:51: undefined reference to `timer_create'
> arch/um/os-Linux/built-in.o: In function `os_timer_set_interval':
> /android/kernel/android/arch/um/os-Linux/time.c:84: undefined reference to `timer_settime'
> arch/um/os-Linux/built-in.o: In function `os_timer_remain':
> /android/kernel/android/arch/um/os-Linux/time.c:109: undefined reference to `timer_gettime'
> arch/um/os-Linux/built-in.o: In function `os_timer_one_shot':
> /android/kernel/android/arch/um/os-Linux/time.c:132: undefined reference to `timer_settime'
> arch/um/os-Linux/built-in.o: In function `os_timer_disable':
> /android/kernel/android/arch/um/os-Linux/time.c:145: undefined reference to `timer_settime'
>
> This is because -lrt appears in the generated link commandline
> after arch/um/os-Linux/built-in.o. Fix this by removing -lrt from
> arch/um/Makefile and adding it to the UM-specific section of
> scripts/link-vmlinux.sh.
>
> Signed-off-by: Lorenzo Colitti <lo...@go...>
> ---
>  arch/um/Makefile        | 2 +-
>  scripts/link-vmlinux.sh | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/um/Makefile b/arch/um/Makefile
> index 25ed409..e3abe6f 100644
> --- a/arch/um/Makefile
> +++ b/arch/um/Makefile
> @@ -131,7 +131,7 @@ export LDS_ELF_FORMAT := $(ELF_FORMAT)
>  # The wrappers will select whether using "malloc" or the kernel allocator.
>  LINK_WRAPS = -Wl,--wrap,malloc -Wl,--wrap,free -Wl,--wrap,calloc
>
> -LD_FLAGS_CMDLINE = $(foreach opt,$(LDFLAGS),-Wl,$(opt)) -lrt
> +LD_FLAGS_CMDLINE = $(foreach opt,$(LDFLAGS),-Wl,$(opt))
>
>  # Used by link-vmlinux.sh which has special support for um link
>  export CFLAGS_vmlinux := $(LINK-y) $(LINK_WRAPS) $(LD_FLAGS_CMDLINE)
> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> index 1a10d8a..dacf71a 100755
> --- a/scripts/link-vmlinux.sh
> +++ b/scripts/link-vmlinux.sh
> @@ -62,7 +62,7 @@ vmlinux_link()
>                         -Wl,--start-group                                    \
>                                  ${KBUILD_VMLINUX_MAIN}                      \
>                         -Wl,--end-group                                      \
> -                       -lutil ${1}
> +                       -lutil -lrt ${1}
>                 rm -f linux
>         fi
>  }
> --

Applied! :)

-- 
Thanks,
//richard

Re: [uml-devel] [PATCH] um: fix returns without va_end

[Richard W. <ric...@gm...>]

On Tue, Dec 1, 2015 at 9:18 PM, Geyslan G. Bem <ge...@gm...> wrote:
> When using va_list ensure that va_start will be followed by va_end.
>
> Signed-off-by: Geyslan G. Bem <ge...@gm...>
> ---
>  arch/um/drivers/net_user.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/arch/um/drivers/net_user.c b/arch/um/drivers/net_user.c
> index e697a41..e9f8445 100644
> --- a/arch/um/drivers/net_user.c
> +++ b/arch/um/drivers/net_user.c
> @@ -249,21 +249,23 @@ void close_addr(unsigned char *addr, unsigned char *netmask, void *arg)
>
>  char *split_if_spec(char *str, ...)
>  {
> -       char **arg, *end;
> +       char **arg, *end, *ret = NULL;
>         va_list ap;
>
>         va_start(ap, str);
>         while ((arg = va_arg(ap, char **)) != NULL) {
>                 if (*str == '\0')
> -                       return NULL;
> +                       goto out;
>                 end = strchr(str, ',');
>                 if (end != str)
>                         *arg = str;
>                 if (end == NULL)
> -                       return NULL;
> +                       goto out;
>                 *end++ = '\0';
>                 str = end;
>         }
> +       ret = str;
> +out:
>         va_end(ap);
> -       return str;
> +       return ret;
>  }

Applied! :)

-- 
Thanks,
//richard

Re: [uml-devel] [PATCH v2 1/2] um: Set secure access mode for temporary file

[Tristan S. <tsc...@go...>]

On 6 December 2015 at 09:43, Mickaël Salaün <mi...@di...> wrote:
> Well, I'm concerned to use umask because it is not thread-safe and drivers may use create_mem_file() in a multi-theaded context.

You are right. We should perhaps set the umask to 0700 permanently
during process start. But I am not sure if this will interfere with
other UML code.

> I prefer to stick to fchmod and handle the race-condition with O_TMPFILE unsell someone is sure that this will not create bugs :)

The fchmod call is basically useless and should probably be removed.
Even mmap only checks the file descriptor, not the file permissions. I
have pasted a test program below if you wish to confirm. AFAICT
changing the permissions after file deletion accomplishes nothing
unless the attacker bizarrely chooses to hard-link the file during the
race instead of opening it.

#include <assert.h>
#include <fcntl.h>
#include <sys/mman.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <unistd.h>

int main(int argc, char **argv) {
  int fd = open("./foo", O_RDWR|O_CREAT|O_EXCL, 0700);
  assert(fd >= 0);
  int ret = write(fd, "bar\n", 4);
  assert(ret == 4);
  ret = fchmod(fd, 0400);
  assert(ret >= 0);
  char *buf = mmap(0, 4, PROT_READ|PROT_WRITE|PROT_EXEC, MAP_SHARED, fd, 0);
  assert(buf);
  buf[2] = 'z';
  ret = munmap(buf, 4);
  assert(ret >= 0);
  return 0;
}

Re: [uml-devel] uml instance crashes when started from script

[Anton I. <ant...@ko...>]

On 07/12/15 07:24, st...@ni... wrote:
> Den 2015-12-06 16:34, skrev Vegard Nossum:
>> Hi,
>>
>> I've been running into some odd crashes when starting my UML instance
>> from Python. This is my script:
>>
>> import subprocess
>> subprocess.check_call(['path/to/vmlinux', 'mem=2048M',
>> 'rootfstype=hostfs', 'rw', 'init=/bin/bash'])
>>
>> [    1.880000] winch_thread : TIOCSCTTY failed on fd 1 err = 1
>>
> I suspect subprocess.check_call() uses pipes and not pty/tty when 
> creating this process.

It does.

>  Same problem probably happens if do you something 
> like
>
> cat /dev/null | path/to/vmlinux mem=2048M rootfstype=hostfs rw 
> init=/bin/bash 2>&1 >log

Brgds,

A.
>
>
>
> Stian
>
> ------------------------------------------------------------------------------
> Go from Idea to Many App Stores Faster with Intel(R) XDK
> Give your users amazing mobile app experiences with Intel(R) XDK.
> Use one codebase in this all-in-one HTML5 development environment.
> Design, debug & build mobile apps & 2D/3D high-impact games for multiple OSs.
> http://pubads.g.doubleclick.net/gampad/clk?id=254741911&iu=/4140
> _______________________________________________
> User-mode-linux-devel mailing list
> Use...@li...
> https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel
>

Re: [uml-devel] uml instance crashes when started from script

[<st...@ni...>]

Den 2015-12-06 16:34, skrev Vegard Nossum:
> Hi,
>
> I've been running into some odd crashes when starting my UML instance
> from Python. This is my script:
>
> import subprocess
> subprocess.check_call(['path/to/vmlinux', 'mem=2048M',
> 'rootfstype=hostfs', 'rw', 'init=/bin/bash'])
>
> [    1.880000] winch_thread : TIOCSCTTY failed on fd 1 err = 1
>

I suspect subprocess.check_call() uses pipes and not pty/tty when 
creating this process. Same problem probably happens if do you something 
like

cat /dev/null | path/to/vmlinux mem=2048M rootfstype=hostfs rw 
init=/bin/bash 2>&1 >log



Stian