All of lore.kernel.org
 help / color / mirror / Atom feed
From: ebiederm@xmission.com (Eric W. Biederman)
To: "Martin Pärtel" <martin.partel@gmail.com>
Cc: Anton Ivanov <anton.ivanov@kot-begemot.co.uk>,
	Richard Weinberger <richard.weinberger@gmail.com>,
	Linux-Arch <linux-arch@vger.kernel.org>,
	Richard Weinberger <richard@nod.at>,
	Jeff Dike <jdike@addtoit.com>,
	linux-um@lists.infradead.org, LKML <linux-kernel@vger.kernel.org>,
	"user-mode-linux-devel\@lists.sourceforge.net"
	<user-mode-linux-devel@lists.sourceforge.net>
Subject: Re: [uml-devel] [REVIEW][PATCH 19/22] signal/um: Use force_sig_fault in relay_signal.
Date: Tue, 24 Apr 2018 17:24:24 -0500	[thread overview]
Message-ID: <87lgdc1bvr.fsf@xmission.com> (raw)
In-Reply-To: <CAMD8JhxHwcuXh68o1=fbZ_t6DH7x_dO_0nEMqL=cHXBfZXub=w@mail.gmail.com> ("Martin \=\?utf-8\?Q\?P\=C3\=A4rtel\=22's\?\= message of "Wed, 25 Apr 2018 01:03:39 +0300")

Martin Pärtel <martin.partel@gmail.com> writes:

> And once more in plain text..
>
> On 25 April 2018 at 01:00, Martin Pärtel <martin.partel@gmail.com> 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&m=133910707911999&w=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

WARNING: multiple messages have this Message-ID (diff)
From: ebiederm@xmission.com (Eric W. Biederman)
To: "Martin Pärtel" <martin.partel@gmail.com>
Cc: Anton Ivanov <anton.ivanov@kot-begemot.co.uk>,
	Richard Weinberger <richard.weinberger@gmail.com>,
	Linux-Arch <linux-arch@vger.kernel.org>,
	Richard Weinberger <richard@nod.at>,
	Jeff Dike <jdike@addtoit.com>,
	linux-um@lists.infradead.org, LKML <linux-kernel@vger.kernel.org>,
	"user-mode-linux-devel@lists.sourceforge.net"
	<user-mode-linux-devel@lists.sourceforge.net>
Subject: Re: [uml-devel] [REVIEW][PATCH 19/22] signal/um: Use force_sig_fault in relay_signal.
Date: Tue, 24 Apr 2018 17:24:24 -0500	[thread overview]
Message-ID: <87lgdc1bvr.fsf@xmission.com> (raw)
In-Reply-To: <CAMD8JhxHwcuXh68o1=fbZ_t6DH7x_dO_0nEMqL=cHXBfZXub=w@mail.gmail.com> ("Martin \=\?utf-8\?Q\?P\=C3\=A4rtel\=22's\?\= message of "Wed, 25 Apr 2018 01:03:39 +0300")

Martin Pärtel <martin.partel@gmail.com> writes:

> And once more in plain text..
>
> On 25 April 2018 at 01:00, Martin Pärtel <martin.partel@gmail.com> 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&m=133910707911999&w=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

WARNING: multiple messages have this Message-ID (diff)
From: ebiederm@xmission.com (Eric W. Biederman)
To: "Martin Pärtel" <martin.partel@gmail.com>
Cc: Linux-Arch <linux-arch@vger.kernel.org>,
	"user-mode-linux-devel@lists.sourceforge.net"
	<user-mode-linux-devel@lists.sourceforge.net>,
	Richard Weinberger <richard.weinberger@gmail.com>,
	Richard Weinberger <richard@nod.at>,
	Jeff Dike <jdike@addtoit.com>,
	linux-um@lists.infradead.org, LKML <linux-kernel@vger.kernel.org>
Subject: Re: [uml-devel] [REVIEW][PATCH 19/22] signal/um: Use force_sig_fault in relay_signal.
Date: Tue, 24 Apr 2018 17:24:24 -0500	[thread overview]
Message-ID: <87lgdc1bvr.fsf@xmission.com> (raw)
In-Reply-To: <CAMD8JhxHwcuXh68o1=fbZ_t6DH7x_dO_0nEMqL=cHXBfZXub=w@mail.gmail.com> ("Martin \=\?utf-8\?Q\?P\=C3\=A4rtel\=22's\?\= message of "Wed, 25 Apr 2018 01:03:39 +0300")

Martin Pärtel <martin.partel@gmail.com> writes:

> And once more in plain text..
>
> On 25 April 2018 at 01:00, Martin Pärtel <martin.partel@gmail.com> 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&m=133910707911999&w=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

  reply	other threads:[~2018-04-24 22:26 UTC|newest]

Thread overview: 113+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-20  1:01 [REVIEW][PATCH 00/17] siginfo bugfixes and cleanups Eric W. Biederman
2018-04-20  1:01 ` Eric W. Biederman
2018-04-20  1:03 ` [REVIEW][PATCH 01/17] signal/alpha: Document a conflict with SI_USER for SIGFPE Eric W. Biederman
2018-04-20  1:03 ` [REVIEW][PATCH 02/17] sparc: fix compat siginfo ABI regression Eric W. Biederman
2018-04-20  1:03   ` Eric W. Biederman
2018-04-20  1:03 ` [REVIEW][PATCH 03/17] signal/sh: Use force_sig_fault in hw_breakpoint_handler Eric W. Biederman
2018-04-20  1:03   ` Eric W. Biederman
2018-04-20  1:03 ` [REVIEW][PATCH 04/17] signal/nds32: Use force_sig in unhandled_interruption and unhandled_exceptions Eric W. Biederman
2018-04-25 12:14   ` Vincent Chen
2018-04-20  1:03 ` [REVIEW][PATCH 05/17] signal/nds32: Use force_sig(SIGILL) in do_revisn Eric W. Biederman
2018-04-25 12:10   ` Vincent Chen
2018-04-25 16:13     ` [PATCH] signal/nds32: More information in do_revinsn Eric W. Biederman
2018-04-26  3:02       ` Vincent Chen
2018-04-20  1:03 ` [REVIEW][PATCH 06/17] signal: Ensure every siginfo we send has all bits initialized Eric W. Biederman
2018-04-20  1:03 ` [REVIEW][PATCH 07/17] signal: Reduce copy_siginfo_to_user to just copy_to_user Eric W. Biederman
2018-04-20  1:03 ` [REVIEW][PATCH 08/17] signal: Stop special casing TRAP_FIXME and FPE_FIXME in siginfo_layout Eric W. Biederman
2018-04-20  1:04 ` [REVIEW][PATCH 09/17] signal: Remove SEGV_BNDERR ifdefs Eric W. Biederman
2018-04-20  1:04 ` [REVIEW][PATCH 10/17] signal: Remove ifdefs for BUS_MCEERR_AR and BUS_MCEERR_AO Eric W. Biederman
2018-04-20  1:04 ` [REVIEW][PATCH 11/17] signal/alpha: Replace FPE_FIXME with FPE_FLTUNK Eric W. Biederman
2018-04-20  1:04 ` [REVIEW][PATCH 12/17] signal/ia64: " Eric W. Biederman
2018-04-20  1:04   ` Eric W. Biederman
2018-04-20  1:04 ` [REVIEW][PATCH 13/17] signal/powerpc: " Eric W. Biederman
2018-04-20  1:04 ` [REVIEW][PATCH 14/17] signal/unicore32: Use FPE_FLTUNK instead of 0 in ucf64_raise_sigfpe Eric W. Biederman
2018-04-20  1:04 ` [REVIEW][PATCH 15/17] signal: Add TRAP_UNK si_code for undiagnosted trap exceptions Eric W. Biederman
2018-04-20  1:04 ` [REVIEW][PATCH 16/17] signal/alpha: Replace TRAP_FIXME with TRAP_UNK Eric W. Biederman
2018-04-20  1:04 ` [REVIEW][PATCH 17/17] signal/powerpc: " Eric W. Biederman
2018-04-20 14:35 ` [REVIEW][PATCH 00/22] Simplifying siginfo users Eric W. Biederman
2018-04-20 14:35   ` Eric W. Biederman
2018-04-20 14:35   ` [OpenRISC] " Eric W. Biederman
2018-04-20 14:35   ` Eric W. Biederman
2018-04-20 14:35   ` Eric W. Biederman
2018-04-20 14:35   ` Eric W. Biederman
2018-04-20 14:37   ` [REVIEW][PATCH 01/22] signal/alpha: Use send_sig_fault where appropriate Eric W. Biederman
2018-04-20 14:37   ` [REVIEW][PATCH 02/22] signal/alpha: Use force_sig_fault " Eric W. Biederman
2018-04-20 14:37   ` [REVIEW][PATCH 03/22] signal/c6x: " Eric W. Biederman
2018-04-20 14:37   ` [REVIEW][PATCH 04/22] signal/hexagon: Use force_sig_fault as appropriate Eric W. Biederman
2018-04-20 14:37   ` [REVIEW][PATCH 05/22] signal/m68k: Use force_sig_fault where appropriate Eric W. Biederman
2018-04-20 14:37     ` Eric W. Biederman
2018-04-20 14:37   ` [REVIEW][PATCH 06/22] signal/microblaze: Remove the commented out force_sig_info in do_page_fault Eric W. Biederman
2018-04-20 14:37   ` [REVIEW][PATCH 07/22] signal/microblaze: Use force_sig_fault where appropriate Eric W. Biederman
2018-04-20 14:37   ` [REVIEW][PATCH 08/22] signal/mips: " Eric W. Biederman
2018-05-09 15:14     ` Matt Redfearn
2018-05-09 15:14       ` Matt Redfearn
2018-05-10  2:39       ` Eric W. Biederman
2018-05-10  2:39         ` Eric W. Biederman
2018-05-10  7:59         ` Matt Redfearn
2018-05-10  7:59           ` Matt Redfearn
2018-05-11  2:31           ` Eric W. Biederman
2018-05-11  2:31             ` Eric W. Biederman
2018-04-20 14:37   ` [REVIEW][PATCH 09/22] signal/nds32: " Eric W. Biederman
2018-04-25 11:29     ` Vincent Chen
2018-04-25 15:57       ` Eric W. Biederman
2018-04-26  1:24         ` Greentime Hu
2018-04-20 14:37   ` [REVIEW][PATCH 10/22] signal/nios2: " Eric W. Biederman
2018-04-20 14:38   ` [REVIEW][PATCH 11/22] signal/openrisc: " Eric W. Biederman
2018-04-20 14:38     ` [OpenRISC] " Eric W. Biederman
2018-04-20 14:38   ` [REVIEW][PATCH 12/22] signal/parisc: Use force_sig_mceerr " Eric W. Biederman
2018-04-20 14:38   ` [REVIEW][PATCH 13/22] signal/parisc: Use force_sig_fault " Eric W. Biederman
2018-04-21 17:24     ` Helge Deller
2018-04-20 14:38   ` [REVIEW][PATCH 14/22] signal/riscv: " Eric W. Biederman
2018-04-20 14:38     ` Eric W. Biederman
2018-04-21  7:25     ` Christoph Hellwig
2018-04-21  7:25       ` Christoph Hellwig
2018-04-24 15:31       ` [REVIEW][PATCH 23/22] signal/riscv: Replace do_trap_siginfo with force_sig_fault Eric W. Biederman
2018-04-24 15:31         ` Eric W. Biederman
2018-04-23 19:11     ` [REVIEW][PATCH 14/22] signal/riscv: Use force_sig_fault where appropriate Palmer Dabbelt
2018-04-23 19:11       ` Palmer Dabbelt
2018-04-23 19:11       ` Palmer Dabbelt
2018-04-24 15:28       ` Eric W. Biederman
2018-04-24 15:28         ` Eric W. Biederman
2018-04-20 14:38   ` [REVIEW][PATCH 15/22] signal/s390: " Eric W. Biederman
2018-04-23  5:44     ` Martin Schwidefsky
2018-04-20 14:38   ` [REVIEW][PATCH 16/22] signal/sh: " Eric W. Biederman
2018-04-20 14:38     ` Eric W. Biederman
2018-04-20 14:55     ` Rich Felker
2018-04-20 14:55       ` Rich Felker
2018-05-28  9:19       ` Geert Uytterhoeven
2018-05-28  9:19         ` Geert Uytterhoeven
2018-05-29 15:00         ` [PATCH] signal/sh: Stop gcc warning about an impossible case in do_divide_error Eric W. Biederman
2018-05-29 15:00           ` Eric W. Biederman
2018-05-30  7:54           ` Sergei Shtylyov
2018-05-30  7:54             ` Sergei Shtylyov
2018-04-20 14:38   ` [REVIEW][PATCH 17/22] signal/sparc: Use send_sig_fault where appropriate Eric W. Biederman
2018-04-20 14:38     ` Eric W. Biederman
2018-04-20 14:38   ` [REVIEW][PATCH 18/22] signal/sparc: Use force_sig_fault " Eric W. Biederman
2018-04-20 14:38     ` Eric W. Biederman
2018-04-20 14:38   ` [REVIEW][PATCH 19/22] signal/um: Use force_sig_fault in relay_signal Eric W. Biederman
2018-04-20 16:06     ` [uml-devel] " Anton Ivanov
2018-04-20 16:06       ` Anton Ivanov
2018-04-24  8:32       ` Richard Weinberger
2018-04-24  8:32         ` Richard Weinberger
2018-04-24  8:44         ` Anton Ivanov
2018-04-24  8:44           ` Anton Ivanov
2018-04-24 15:59           ` Eric W. Biederman
2018-04-24 15:59             ` Eric W. Biederman
     [not found]             ` <CAMD8JhwgcWCp6c=O4-spNW1VdY5M-eAjr7M5PieeAfeTSe_4yw@mail.gmail.com>
2018-04-24 22:03               ` Martin Pärtel
2018-04-24 22:03                 ` Martin Pärtel
2018-04-24 22:24                 ` Eric W. Biederman [this message]
2018-04-24 22:24                   ` Eric W. Biederman
2018-04-24 22:24                   ` Eric W. Biederman
2018-04-25 23:05                   ` Martin Pärtel
2018-04-28 14:02                     ` [REVIEW][PATCH 0/5] Improving siginfo_layout and fixing uml's relay_signal Eric W. Biederman
2018-04-28 14:02                       ` [uml-devel] " Eric W. Biederman
2018-04-28 14:06                       ` [REVIEW][PATCH 1/5] signal/signalfd: Remove __put_user from signalfd_copyinfo Eric W. Biederman
2018-04-28 14:06                       ` [REVIEW][PATCH 2/5] signal/signalfd: Add support for SIGSYS Eric W. Biederman
2018-04-28 14:07                       ` [REVIEW][PATCH 3/5] signal: Remove unncessary #ifdef SEGV_PKUERR in 32bit compat code Eric W. Biederman
2018-04-28 14:07                       ` [REVIEW][PATCH 4/5] signal: Extend siginfo_layout with SIL_FAULT_{MCEERR|BNDERR|PKUERR} Eric W. Biederman
2018-04-28 14:07                       ` [REVIEW][PATCH 5/5] signal/um: More carefully relay signals in relay_signal Eric W. Biederman
2018-04-20 14:38   ` [REVIEW][PATCH 20/22] signal/um: Use force_sig_fault where appropriate Eric W. Biederman
2018-04-20 14:38   ` [REVIEW][PATCH 21/22] signal/xtensa: Consistenly use SIGBUS in do_unaligned_user Eric W. Biederman
2018-04-20 16:06     ` Max Filippov
2018-04-20 14:38   ` [REVIEW][PATCH 22/22] signal/xtensa: Use force_sig_fault where appropriate Eric W. Biederman
2018-04-20 16:27     ` Max Filippov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87lgdc1bvr.fsf@xmission.com \
    --to=ebiederm@xmission.com \
    --cc=anton.ivanov@kot-begemot.co.uk \
    --cc=jdike@addtoit.com \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-um@lists.infradead.org \
    --cc=martin.partel@gmail.com \
    --cc=richard.weinberger@gmail.com \
    --cc=richard@nod.at \
    --cc=user-mode-linux-devel@lists.sourceforge.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.