mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Roland McGrath <roland@redhat.com>
To: Oleg Nesterov <oleg@redhat.com>
Cc: David Howells <dhowells@redhat.com>,
	Eric Paris <eparis@parisplace.org>,
	James Morris <jmorris@namei.org>,
	Stephen Smalley <sds@tycho.nsa.gov>,
	linux-kernel@vger.kernel.org
Subject: Re: Q: selinux_bprm_committed_creds() && signals/do_wait
Date: Thu, 30 Apr 2009 17:02:22 -0700 (PDT)	[thread overview]
Message-ID: <20090501000222.8BACEFC3BF@magilla.sf.frob.com> (raw)
In-Reply-To: Oleg Nesterov's message of  Wednesday, 29 April 2009 00:30:25 +0200 <20090428223025.GA11997@redhat.com>

The SIGNAL_GROUP_EXIT check is fine for now.  But we should revisit this
area more thoroughly at some point.  (That is a low-priority thing.)

There is a wrinkle here I don't like.  The fatal signal is "committed to"
(sender approved by security modules, etc.) "before" the exec, but gets
delivered "after" the exec.  e.g., acct_process() writes a record for the
killed process showing the uid/gid from after the setuid/setgid exec, when
security would not have allowed the killing signal to be sent after the
exec, only before.  Obviously this is a minor quirk (not to be worried
about today), but it points to a deeper kind of "wrong" that troubles me.
I can't think of any other similar wrinkles that could be observable at
all (or matter more than that one), but there might be another.

This is related to the issue of racing stop signals lost by de_thread().
That is still on our back-burner list to think about more deeply one day.
We don't need to contemplate any of this much more right now, but I would
like to address the whole mess better later on.

I don't understand why install_exec_creds() is called as late as it is.
Can't we do that in flush_old_exec()--you know, where it says:
	/* install the new credentials */
?

What I think would be best is if flush_old_exec() does all the "point of no
return" logic and that includes the credentials changes.  Then we can
define this as the point of transition from "before exec" to "after exec".
It would do the final check for signals interrupting the exec, and if
flush_old_exec() returned 0, then any "new" signals are "after exec".
We'd reorganize things so the final creds switch is under siglock.
Then either a sender precedes the exec and any security module's flushing
logic wipes the pre-exec state as it wants to, or the sender follows the
exec and is subject to signal security checks based on the new credentials.


Thanks,
Roland

  parent reply	other threads:[~2009-05-01  0:15 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-04-28 22:30 Oleg Nesterov
2009-04-28 23:33 ` Oleg Nesterov
2009-04-29 16:01   ` [PATCH] do_wait: do take security_task_wait() into account Oleg Nesterov
2009-04-30 20:31     ` Roland McGrath
2009-04-30 22:51     ` James Morris
2009-05-06 11:46       ` Stephen Smalley
2009-04-29  0:29 ` Q: selinux_bprm_committed_creds() && signals/do_wait James Morris
2009-04-29  6:58   ` Oleg Nesterov
2009-04-29 12:20     ` Stephen Smalley
2009-04-29 12:56       ` Oleg Nesterov
2009-04-29 13:16         ` Stephen Smalley
2009-04-29 13:42           ` Oleg Nesterov
2009-04-29 13:43             ` Stephen Smalley
2009-04-29 14:47           ` Alan Cox
2009-04-29 15:39             ` Stephen Smalley
2009-04-29 10:02   ` David Howells
2009-04-29 10:25     ` Oleg Nesterov
2009-04-29 11:17     ` David Howells
2009-04-29 11:55       ` Oleg Nesterov
2009-04-29 12:42       ` David Howells
2009-04-29 12:45       ` David Howells
2009-04-29 13:28         ` Oleg Nesterov
2009-04-30  0:37           ` James Morris
2009-04-29 13:18 ` Stephen Smalley
2009-04-29 13:30   ` Oleg Nesterov
2009-04-29 14:02   ` ptrace: selinux_bprm_committed_creds: simplify __wake_up_parent() code and s/parent/real_parent/ Oleg Nesterov
2009-04-29 14:08     ` Oleg Nesterov
2009-04-30 22:44       ` Roland McGrath
2009-05-03 20:10         ` Oleg Nesterov
2009-05-04 17:38           ` Roland McGrath
2009-04-30  0:38     ` James Morris
2009-04-30 22:38     ` Roland McGrath
2009-04-29 14:48 ` Q: selinux_bprm_committed_creds() && signals/do_wait Alan Cox
2009-05-01  0:02 ` Roland McGrath [this message]
2009-05-03 20:21   ` Oleg Nesterov
2009-05-04 17:34     ` Roland McGrath
2009-05-01  0:44 ` David Howells
2009-05-01  0:50   ` Roland McGrath

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=20090501000222.8BACEFC3BF@magilla.sf.frob.com \
    --to=roland@redhat.com \
    --cc=dhowells@redhat.com \
    --cc=eparis@parisplace.org \
    --cc=jmorris@namei.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=oleg@redhat.com \
    --cc=sds@tycho.nsa.gov \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome