All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
To: rgb@redhat.com
Cc: peterz@infradead.org, paulmck@linux.vnet.ibm.com,
	laijs@cn.fujitsu.com, akpm@linux-foundation.org, joe@perches.com,
	keescook@chromium.org, geert@linux-m68k.org, jkosina@suse.cz,
	viro@zeniv.linux.org.uk, davem@davemloft.net,
	linux-kernel@vger.kernel.org, mingo@elte.hu, rostedt@goodmis.org,
	tglx@linutronix.de, linux-security-module@vger.kernel.org
Subject: Re: [PATCH] Change task_struct->comm to use RCU.
Date: Sat, 8 Mar 2014 21:43:36 +0900	[thread overview]
Message-ID: <201403082143.BIH86903.QtVMHJFFOOSFOL@I-love.SAKURA.ne.jp> (raw)
In-Reply-To: <20140307155415.GB16640@madcap2.tricolour.ca>

Richard Guy Briggs wrote:
> > > >   Likewise, audit_log_untrustedstring(ab, current->comm) is racy.
> > > >   If task->comm was "Hello Linux" until audit_string_contains_control() in
> > > >   audit_log_n_untrustedstring() returns false, and becomes "Penguin" before
> > > >   memcpy() in audit_log_n_string() is called, memcpy() will emit "Penguin\0nux"
> > > >   into the audit log, which results in loss of information (e.g. SELinux
> > > >   context) due to the unexpected '\0' byte.
> > > 
> > > I expect the audit people don't like this? Also, how do audit and the
> > > LSM crap things interact? I thought they were both different piles of
> > > ignorable goo?
> > 
> > I think the audit people do not like loss of information. Some of LSM modules
> > are using audit subsystem for recording security related events. An example is
> > shown later.
> 
> This is true, however since comm it untrusted because it can be modified
> by the user audit doesn't trust it anyways, so who cares?

Excuse me, but did you understand this side effect correctly?

# ln /bin/true /tmp/printable-comm
# auditctl -a exit,always -S execve -F path=/tmp/printable-comm
# /tmp/printable-comm
# cat /var/log/audit/audit.log

If we didn't race, everything is fine.

---------- An audit log without race ----------
type=SYSCALL msg=audit(1394281486.738:62): arch=40000003 syscall=11 success=yes exit=0 a0=8589c48 a1=85a6620 a2=858e4a0 a3=85a6620 items=2 ppid=1747 pid=2657 auid=0 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=pts1 ses=2 comm="printable-comm" exe="/tmp/printable-comm" subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 key=(null)
type=EXECVE msg=audit(1394281486.738:62): argc=1 a0="/tmp/printable-comm"
type=CWD msg=audit(1394281486.738:62):  cwd="/root"
type=PATH msg=audit(1394281486.738:62): item=0 name="/tmp/printable-comm" inode=1970955 dev=08:01 mode=0100755 ouid=0 ogid=0 rdev=00:00 obj=system_u:object_r:bin_t:s0 nametype=NORMAL
type=PATH msg=audit(1394281486.738:62): item=1 name=(null) inode=2360187 dev=08:01 mode=0100755 ouid=0 ogid=0 rdev=00:00 obj=system_u:object_r:ld_so_t:s0 nametype=NORMAL
---------- An audit log without race ----------

But if we raced (you can use a (dangerous) SystemTap script shown below for
emulating this race condition

# stap -g -e '
function rewrite_comm(str:long) %{
  strlcpy((char *) (long) STAP_ARG_str, "truncated", sizeof(current->comm));
%}
probe kernel.function("audit_log_n_string") {
  if ($ab && $slen == 14 && kernel_string($string) == "printable-comm") {
    rewrite_comm($string); printf("<%s>\n", kernel_string($string))
  };
}
'

), you can see that fields after comm= (e.g. exe= subj= key= ) are missing.

---------- An audit log with race ----------
type=SYSCALL msg=audit(1394281498.566:63): arch=40000003 syscall=11 success=yes exit=0 a0=858c9c8 a1=85a6620 a2=858e4a0 a3=85a6620 items=2 ppid=1747 pid=2662 auid=0 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=pts1 ses=2 comm="truncated
type=EXECVE msg=audit(1394281498.566:63): argc=1 a0="/tmp/printable-comm"
type=CWD msg=audit(1394281498.566:63):  cwd="/root"
type=PATH msg=audit(1394281498.566:63): item=0 name="/tmp/printable-comm" inode=1970955 dev=08:01 mode=0100755 ouid=0 ogid=0 rdev=00:00 obj=system_u:object_r:bin_t:s0 nametype=NORMAL
type=PATH msg=audit(1394281498.566:63): item=1 name=(null) inode=2360187 dev=08:01 mode=0100755 ouid=0 ogid=0 rdev=00:00 obj=system_u:object_r:ld_so_t:s0 nametype=NORMAL
---------- An audit log with race ----------

Even if you don't trust the comm= field, it is annoying for me that fields
after comm= are missing in the audit log.

  reply	other threads:[~2014-03-08 12:45 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <201402072303.DJD13007.JFFMSLHOOFQOtV@I-love.SAKURA.ne.jp>
     [not found] ` <20140207140536.943daf965008b9428cdcb468@linux-foundation.org>
     [not found]   ` <201402081055.BGJ73403.tQMLFVOJSOOFFH@I-love.SAKURA.ne.jp>
     [not found]     ` <20140207180647.5944fe3d.akpm@linux-foundation.org>
     [not found]       ` <201402092327.JAD12489.QOLSFVMHJtFOOF@I-love.SAKURA.ne.jp>
2014-02-10 13:43         ` [PATCH (draft)] Change task_struct->comm to use RCU Tetsuo Handa
2014-02-17 11:27           ` [PATCH] " Tetsuo Handa
2014-02-24 23:51             ` Paul E. McKenney
2014-02-26 13:44               ` Tetsuo Handa
2014-02-26 15:26                 ` Paul E. McKenney
2014-02-25  1:49             ` Lai Jiangshan
2014-02-25 10:05               ` Peter Zijlstra
2014-02-25 12:54               ` Tetsuo Handa
2014-02-25 14:46                 ` Peter Zijlstra
2014-03-07 12:20                   ` Tetsuo Handa
2014-03-07 15:54                     ` Richard Guy Briggs
2014-03-08 12:43                       ` Tetsuo Handa [this message]
2014-03-10 20:21                         ` Richard Guy Briggs
2014-03-11 12:02                           ` Tetsuo Handa
2014-03-11 12:16                             ` Tetsuo Handa
2014-03-11 13:55                               ` James Morris
2014-03-24 15:43                                 ` Richard Guy Briggs
2014-03-27 17:20                                 ` [PATCH] LSM: Pass comm name via get_task_comm() [was: Re: [PATCH] Change task_struct->comm to use RCU.] Richard Guy Briggs
2014-03-27 18:06                                   ` Stephen Smalley
2014-09-19  3:30                                     ` Richard Guy Briggs

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=201403082143.BIH86903.QtVMHJFFOOSFOL@I-love.SAKURA.ne.jp \
    --to=penguin-kernel@i-love.sakura.ne.jp \
    --cc=akpm@linux-foundation.org \
    --cc=davem@davemloft.net \
    --cc=geert@linux-m68k.org \
    --cc=jkosina@suse.cz \
    --cc=joe@perches.com \
    --cc=keescook@chromium.org \
    --cc=laijs@cn.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=peterz@infradead.org \
    --cc=rgb@redhat.com \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    --cc=viro@zeniv.linux.org.uk \
    /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.