Re: [uml-devel] [REVIEW][PATCH 19/22] signal/um: Use force_sig_fault in relay_signal.

2018-04-24 Thread Eric W. Biederman 
Martin Pärtel  writes:

> And once more in plain text..
>
> On 25 April 2018 at 01:00, Martin Pärtel  wrote:
>>
>> Hi all,
>>
>> This was ages ago, but from what I remember...
>>
>>>
>>> Having a second look I really don't understand what relay_signal is
>>> trying to do.
>>>
>>> The function relay_signal does not pass siginfo through unchanged.
>>
>>
>> Just copying the entire struct would do the wrong thing. It was discussed 
>> here:
>> https://marc.info/?l=user-mode-linux-devel=133910707911999=2

So you are regnerating siginfo to ensure you don't copy unintended
things such as the host pid and host uid.

Then my analysis is correct that you simply missed filtering out the
si codes that are not signal specific and do not use the fault layout
in struct siginfo.

Is si_addr safe to copy across?  I presume so since the kernel just
ptraces an ordinary process, but I figure I should ask and double
check.

I am going to respin my patch.  I would say that you really need a
white-list of si_codes that whose use of struct siginfo that you know.
Otherwise you could get into the same problem of under or over copying
data.

Eric

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Re: [uml-devel] [REVIEW][PATCH 19/22] signal/um: Use force_sig_fault in relay_signal.

2018-04-24 Thread Eric W. Biederman 
Sigh I should have Cc'd Martin Partel as well as this bit is his
original code.

Anton Ivanov  writes:

> Hi Richard,
>
> There was a post to uml-devel during the days when the sourceforge mailing 
> list
> was working in random drop mode which claimed that "this fixes the arm build".
>
> I have not kept it locally and I do not see it the archive (I do not see a few
> other posts there either - including some of mine).
>
> The joys of having a broken list :(
>
> Whoever posted it, if you are reading it, please re-post again so we can have 
> a
> look.
>
> In the meantime we are as you said - x86 only.

The only case I can see my changed relay_signal affecting on arm is the
nasty hach where errno is set in conjunction with trap_trace.

Having a second look I really don't understand what relay_signal is
trying to do.

The function relay_signal does not pass siginfo through unchanged.
The function relay_signal does not handle cases where si_code is
not SI_USER or SI_KERNEL, or any of the other signal independent
si_codes.

In my change I believe I have preserved the character of relay_signal of
just passing through the fault.

Still even after reading the commit that upgraded relay_signal to
preserve si_code and si_addr I really don't understand the intended
logic.

Am I missing something subtle or have the subtle details of siginfo just
always been ignored?

commit d3c1cfcdb43e023ab1b1c7a555cd9e929026500a
Author: Martin Pärtel 
Date:   Thu Aug 2 00:49:17 2012 +0200

um: pass siginfo to guest process

UML guest processes now get correct siginfo_t for SIGTRAP, SIGFPE,
SIGILL and SIGBUS. Specifically, si_addr and si_code are now correct
where previously they were si_addr = NULL and si_code = 128.

Signed-off-by: Martin Pärtel 
Signed-off-by: Richard Weinberger 

Eric


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Re: [uml-devel] [REVIEW][PATCH 19/22] signal/um: Use force_sig_fault in relay_signal.

2018-04-24 Thread Anton Ivanov 

Hi Richard,

There was a post to uml-devel during the days when the sourceforge 
mailing list was working in random drop mode which claimed that "this 
fixes the arm build".


I have not kept it locally and I do not see it the archive (I do not see 
a few other posts there either - including some of mine).


The joys of having a broken list :(

Whoever posted it, if you are reading it, please re-post again so we can 
have a look.


In the meantime we are as you said - x86 only.

A.

On 04/24/18 09:32, Richard Weinberger wrote:

On Fri, Apr 20, 2018 at 6:06 PM, Anton Ivanov
 wrote:

On 04/20/18 15:38, Eric W. Biederman wrote:

Today user mode linux only works on x86 and x86_64 and this allows
simplifications of relay_signal.


I believe someone recently fixed the ARM port. I have not had the time to
try the fixes though.

Huh? UML is for ages x86 only.




--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Re: [uml-devel] [REVIEW][PATCH 19/22] signal/um: Use force_sig_fault in relay_signal.

2018-04-24 Thread Richard Weinberger 
On Fri, Apr 20, 2018 at 6:06 PM, Anton Ivanov
 wrote:
>
> On 04/20/18 15:38, Eric W. Biederman wrote:
>>
>> Today user mode linux only works on x86 and x86_64 and this allows
>> simplifications of relay_signal.
>
>
> I believe someone recently fixed the ARM port. I have not had the time to
> try the fixes though.

Huh? UML is for ages x86 only.

-- 
Thanks,
//richard

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Re: [uml-devel] [REVIEW][PATCH 19/22] signal/um: Use force_sig_fault in relay_signal.

2018-04-20 Thread Anton Ivanov 


On 04/20/18 15:38, Eric W. Biederman wrote:

Today user mode linux only works on x86 and x86_64 and this allows
simplifications of relay_signal.


I believe someone recently fixed the ARM port. I have not had the time 
to try the fixes though.


I have added the new list we are migrating to the cc list.

A.





- x86 always set si_errno to 0 in fault handlers.
- x86 does not implement si_trapno.
- Only si_codes between SI_USER and SI_KERNEL have a fault address.

Therefore warn if si_errno is set (it should never be).
Use force_sig_info in the case where we know we have a good fault.

For signals whose content it is not clear how to relay use plain
force_sig and let the signal sending code come up with an
appropriate generic siginfo.

Cc: Jeff Dike 
Cc: Richard Weinberger 
Cc: user-mode-linux-devel@lists.sourceforge.net
Signed-off-by: "Eric W. Biederman" 
---
  arch/um/kernel/trap.c | 28 +---
  1 file changed, 13 insertions(+), 15 deletions(-)

diff --git a/arch/um/kernel/trap.c b/arch/um/kernel/trap.c
index d4d38520c4c6..5f0ff17cd790 100644
--- a/arch/um/kernel/trap.c
+++ b/arch/um/kernel/trap.c
@@ -296,9 +296,6 @@ unsigned long segv(struct faultinfo fi, unsigned long ip, 
int is_user,
  
  void relay_signal(int sig, struct siginfo *si, struct uml_pt_regs *regs)

  {
-   struct faultinfo *fi;
-   struct siginfo clean_si;
-
if (!UPT_IS_USER(regs)) {
if (sig == SIGBUS)
printk(KERN_ERR "Bus error - the host /dev/shm or /tmp "
@@ -308,29 +305,30 @@ void relay_signal(int sig, struct siginfo *si, struct 
uml_pt_regs *regs)
  
  	arch_examine_signal(sig, regs);
  
-	clear_siginfo(_si);

-   clean_si.si_signo = si->si_signo;
-   clean_si.si_errno = si->si_errno;
-   clean_si.si_code = si->si_code;
+   if (unlikely(si->si_errno)) {
+   printk(KERN_ERR "Attempted to relay signal %d (si_code = %d) with 
errno %d\n",
+  sig, si->si_code, si->si_errno);
+   }
switch (sig) {
case SIGILL:
case SIGFPE:
case SIGSEGV:
case SIGBUS:
case SIGTRAP:
-   fi = UPT_FAULTINFO(regs);
-   clean_si.si_addr = (void __user *) FAULT_ADDRESS(*fi);
-   current->thread.arch.faultinfo = *fi;
-#ifdef __ARCH_SI_TRAPNO
-   clean_si.si_trapno = si->si_trapno;
-#endif
-   break;
+   if ((si->si_code > SI_USER) && (si->si_code < SI_KERNEL)) {
+   struct faultinfo *fi = UPT_FAULTINFO(regs);
+   current->thread.arch.faultinfo = *fi;
+   force_sig_fault(sig, si->si_code,
+   (void __user *)FAULT_ADDRESS(*fi),
+   current);
+   break;
+   }
default:
printk(KERN_ERR "Attempted to relay unknown signal %d (si_code = 
%d)\n",
sig, si->si_code);
}
  
-	force_sig_info(sig, _si, current);

+   force_sig(sig, current);
  }
  
  void bus_handler(int sig, struct siginfo *si, struct uml_pt_regs *regs)



--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

[uml-devel] [REVIEW][PATCH 19/22] signal/um: Use force_sig_fault in relay_signal.

2018-04-20 Thread Eric W. Biederman 
Today user mode linux only works on x86 and x86_64 and this allows
simplifications of relay_signal.

- x86 always set si_errno to 0 in fault handlers.
- x86 does not implement si_trapno.
- Only si_codes between SI_USER and SI_KERNEL have a fault address.

Therefore warn if si_errno is set (it should never be).
Use force_sig_info in the case where we know we have a good fault.

For signals whose content it is not clear how to relay use plain
force_sig and let the signal sending code come up with an
appropriate generic siginfo.

Cc: Jeff Dike 
Cc: Richard Weinberger 
Cc: user-mode-linux-devel@lists.sourceforge.net
Signed-off-by: "Eric W. Biederman" 
---
 arch/um/kernel/trap.c | 28 +---
 1 file changed, 13 insertions(+), 15 deletions(-)

diff --git a/arch/um/kernel/trap.c b/arch/um/kernel/trap.c
index d4d38520c4c6..5f0ff17cd790 100644
--- a/arch/um/kernel/trap.c
+++ b/arch/um/kernel/trap.c
@@ -296,9 +296,6 @@ unsigned long segv(struct faultinfo fi, unsigned long ip, 
int is_user,
 
 void relay_signal(int sig, struct siginfo *si, struct uml_pt_regs *regs)
 {
-   struct faultinfo *fi;
-   struct siginfo clean_si;
-
if (!UPT_IS_USER(regs)) {
if (sig == SIGBUS)
printk(KERN_ERR "Bus error - the host /dev/shm or /tmp "
@@ -308,29 +305,30 @@ void relay_signal(int sig, struct siginfo *si, struct 
uml_pt_regs *regs)
 
arch_examine_signal(sig, regs);
 
-   clear_siginfo(_si);
-   clean_si.si_signo = si->si_signo;
-   clean_si.si_errno = si->si_errno;
-   clean_si.si_code = si->si_code;
+   if (unlikely(si->si_errno)) {
+   printk(KERN_ERR "Attempted to relay signal %d (si_code = %d) 
with errno %d\n",
+  sig, si->si_code, si->si_errno);
+   }
switch (sig) {
case SIGILL:
case SIGFPE:
case SIGSEGV:
case SIGBUS:
case SIGTRAP:
-   fi = UPT_FAULTINFO(regs);
-   clean_si.si_addr = (void __user *) FAULT_ADDRESS(*fi);
-   current->thread.arch.faultinfo = *fi;
-#ifdef __ARCH_SI_TRAPNO
-   clean_si.si_trapno = si->si_trapno;
-#endif
-   break;
+   if ((si->si_code > SI_USER) && (si->si_code < SI_KERNEL)) {
+   struct faultinfo *fi = UPT_FAULTINFO(regs);
+   current->thread.arch.faultinfo = *fi;
+   force_sig_fault(sig, si->si_code,
+   (void __user *)FAULT_ADDRESS(*fi),
+   current);
+   break;
+   }
default:
printk(KERN_ERR "Attempted to relay unknown signal %d (si_code 
= %d)\n",
sig, si->si_code);
}
 
-   force_sig_info(sig, _si, current);
+   force_sig(sig, current);
 }
 
 void bus_handler(int sig, struct siginfo *si, struct uml_pt_regs *regs)
-- 
2.14.1


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel