public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Eric Paris <eparis@redhat.com>
To: linux-kernel@vger.kernel.org,
	linux-security-module@vger.kernel.org, selinux@tycho.nsa.gov
Cc: sds@tycho.nsa.gov, jmorris@namei.org, casey@schaufler-ca.com,
	john.johansen@canonical.com, penguin-kernel@I-love.SAKURA.ne.jp,
	takedakn@nttdata.co.jp
Subject: SELinux/SMACK/TOMOYO: ioctl permissions handling is wrong and nonsensicle
Date: Fri, 21 Jan 2011 14:30:35 -0500	[thread overview]
Message-ID: <1295638239.3403.15.camel@localhost.localdomain> (raw)

[I've included an AA person as well in case you ever decide to try to
mediate ioctl operations]

SELinux used to recognize certain individual ioctls and check
permissions based on the knowledge of the individual ioctl.  In commit
242631c49d4cf396 the SELinux code stopped trying to understand
individual ioctls and to instead looked at the ioctl access bits to
determine in we should check read or write for that operation.  This
same suggestion was made to SMACK (and I believe copied into TOMOYO).
But this suggestion is total rubbish.  The ioctl access bits are
actually the access requirements for the structure being passed into the
ioctl, and are completely unrelated to the operation of the ioctl or the
object the ioctl is being performed upon.

Take FS_IOC_FIEMAP as an example.  FS_IOC_FIEMAP is defined as:

FS_IOC_FIEMAP _IOWR('f', 11, struct fiemap)

So it has access bits R and W.  What this really means is that the
kernel is going to both read and write to the struct fiemap.  It has
nothing at all to do with the operations that this ioctl might perform
on the file itself!

If anything, our logic is exactly backwards, since an ioctl which writes
to userspace would be 'reading' something from the file and an ioctl
which reads from userspace would be 'writing' something to the file...

I'm planning to revert this SELinux commit, but I want other LSM authors
to realize that (assuming I'm not completely off in the woods somewhere)
you should take a look at your ioctl permissions checking as well....

-Eric


             reply	other threads:[~2011-01-21 19:31 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-01-21 19:30 Eric Paris [this message]
2011-01-21 19:50 ` SELinux/SMACK/TOMOYO: ioctl permissions handling is wrong and nonsensicle Casey Schaufler
2011-01-21 21:37 ` Stephen Smalley
2011-01-22  2:01 ` SELinux/SMACK/TOMOYO: ioctl permissions handling is wrong andnonsensicle Tetsuo Handa
2011-01-22  2:15   ` Eric Paris

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=1295638239.3403.15.camel@localhost.localdomain \
    --to=eparis@redhat.com \
    --cc=casey@schaufler-ca.com \
    --cc=jmorris@namei.org \
    --cc=john.johansen@canonical.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=penguin-kernel@I-love.SAKURA.ne.jp \
    --cc=sds@tycho.nsa.gov \
    --cc=selinux@tycho.nsa.gov \
    --cc=takedakn@nttdata.co.jp \
    /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