From: Casey Schaufler <casey@schaufler-ca.com>
To: Roberto Sassu <roberto.sassu@polito.it>
Cc: linux-security-module@vger.kernel.org,
linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
dhowells@redhat.com, jmorris@namei.org, zohar@linux.vnet.ibm.com,
safford@watson.ibm.com, tyhicks@linux.vnet.ibm.com,
kirkland@canonical.com, ecryptfs-devel@lists.launchpad.net,
eparis@redhat.com, sds@tycho.nsa.gov, selinux@tycho.nsa.gov,
viro@zeniv.linux.org.uk
Subject: Re: [RFC][PATCH 4/7] smack: fix label check in smack_kernel_act_as()
Date: Wed, 27 Apr 2011 16:22:34 -0700 [thread overview]
Message-ID: <4DB8A53A.4010709@schaufler-ca.com> (raw)
In-Reply-To: <1303907657-18366-5-git-send-email-roberto.sassu@polito.it>
On 4/27/2011 5:34 AM, Roberto Sassu wrote:
> The function smack_kernel_act_as() must return -EINVAL if the label
> returned by smack_from_secid() is equal to 'smack_known_invalid.smk_known',
> which means that no entries in the 'smack_known_list' list matching the
> security identifier given are found.
I'll admit that the code here is wrong, but I disagree with the fix.
smack_from_secid() will never return NULL, so the check for NULL is
pointless. Checking for known_invalid is not right either, as the
Smack philosophy is to return a label in all cases, as is evident
by the behavior of smack_from_secid(). Thus, the correct change
would be to remove the error check completely and set the new task
value to the value obtained from smack_from_secid in all cases.
Besides, where did the caller of this function get a secid that isn't
going to map to a Smack label?
> Signed-off-by: Roberto Sassu <roberto.sassu@polito.it>
> ---
> security/smack/smack_lsm.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index e3c9e54..0e7ed31 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -1415,7 +1415,7 @@ static int smack_kernel_act_as(struct cred *new, u32 secid)
> struct task_smack *new_tsp = new->security;
> char *smack = smack_from_secid(secid);
>
> - if (smack == NULL)
> + if (smack == smack_known_invalid.smk_known)
> return -EINVAL;
>
> new_tsp->smk_task = smack;
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
WARNING: multiple messages have this Message-ID (diff)
From: Casey Schaufler <casey@schaufler-ca.com>
To: Roberto Sassu <roberto.sassu@polito.it>
Cc: linux-security-module@vger.kernel.org,
linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
dhowells@redhat.com, jmorris@namei.org, zohar@linux.vnet.ibm.com,
safford@watson.ibm.com, tyhicks@linux.vnet.ibm.com,
kirkland@canonical.com, ecryptfs-devel@lists.launchpad.net,
eparis@redhat.com, sds@tycho.nsa.gov, selinux@tycho.nsa.gov,
viro@zeniv.linux.org.uk
Subject: Re: [RFC][PATCH 4/7] smack: fix label check in smack_kernel_act_as()
Date: Wed, 27 Apr 2011 16:22:34 -0700 [thread overview]
Message-ID: <4DB8A53A.4010709@schaufler-ca.com> (raw)
In-Reply-To: <1303907657-18366-5-git-send-email-roberto.sassu@polito.it>
On 4/27/2011 5:34 AM, Roberto Sassu wrote:
> The function smack_kernel_act_as() must return -EINVAL if the label
> returned by smack_from_secid() is equal to 'smack_known_invalid.smk_known',
> which means that no entries in the 'smack_known_list' list matching the
> security identifier given are found.
I'll admit that the code here is wrong, but I disagree with the fix.
smack_from_secid() will never return NULL, so the check for NULL is
pointless. Checking for known_invalid is not right either, as the
Smack philosophy is to return a label in all cases, as is evident
by the behavior of smack_from_secid(). Thus, the correct change
would be to remove the error check completely and set the new task
value to the value obtained from smack_from_secid in all cases.
Besides, where did the caller of this function get a secid that isn't
going to map to a Smack label?
> Signed-off-by: Roberto Sassu <roberto.sassu@polito.it>
> ---
> security/smack/smack_lsm.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index e3c9e54..0e7ed31 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -1415,7 +1415,7 @@ static int smack_kernel_act_as(struct cred *new, u32 secid)
> struct task_smack *new_tsp = new->security;
> char *smack = smack_from_secid(secid);
>
> - if (smack == NULL)
> + if (smack == smack_known_invalid.smk_known)
> return -EINVAL;
>
> new_tsp->smk_task = smack;
next prev parent reply other threads:[~2011-04-27 23:22 UTC|newest]
Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-04-27 12:34 [RFC][PATCH 0/7] File descriptor labeling Roberto Sassu
2011-04-27 12:34 ` [RFC][PATCH 1/7] fs: initialize file->f_cred with credentials provided Roberto Sassu
2011-04-27 12:34 ` [RFC][PATCH 2/7] selinux: label new file descriptors using file->f_cred Roberto Sassu
2011-04-27 12:34 ` [RFC][PATCH 3/7] smack: assign the label set in file->f_cred to new file descriptors Roberto Sassu
2011-04-27 23:26 ` Casey Schaufler
2011-04-27 23:26 ` Casey Schaufler
2011-04-28 8:06 ` Roberto Sassu
2011-04-28 8:06 ` Roberto Sassu
2011-04-27 12:34 ` [RFC][PATCH 4/7] smack: fix label check in smack_kernel_act_as() Roberto Sassu
2011-04-27 23:22 ` Casey Schaufler [this message]
2011-04-27 23:22 ` Casey Schaufler
2011-04-28 9:22 ` Roberto Sassu
2011-04-28 9:22 ` Roberto Sassu
2011-04-27 12:34 ` [RFC][PATCH 5/7] smack: import the security label in smack_secctx_to_secid() Roberto Sassu
2011-04-27 23:47 ` Casey Schaufler
2011-04-27 23:47 ` Casey Schaufler
2011-04-27 12:34 ` [RFC][PATCH 6/7] security: new LSM hook security_file_getsecid() Roberto Sassu
2011-04-27 23:50 ` Casey Schaufler
2011-04-27 23:50 ` Casey Schaufler
2011-04-28 9:41 ` Roberto Sassu
2011-04-28 9:41 ` Roberto Sassu
2011-04-27 12:34 ` [RFC][PATCH 7/7] ima: added new LSM conditions in the policy Roberto Sassu
2011-04-28 13:32 ` Mimi Zohar
2011-04-28 13:32 ` Mimi Zohar
2011-04-28 13:52 ` Roberto Sassu
2011-04-28 13:52 ` Roberto Sassu
2011-04-27 15:52 ` [RFC][PATCH 0/7] File descriptor labeling Casey Schaufler
2011-04-27 15:52 ` Casey Schaufler
2011-04-27 20:19 ` Casey Schaufler
2011-04-27 20:19 ` Casey Schaufler
2011-04-27 23:27 ` Tyler Hicks
2011-04-27 23:27 ` Tyler Hicks
2011-04-27 23:57 ` Casey Schaufler
2011-04-27 23:57 ` Casey Schaufler
2011-04-28 0:06 ` Tyler Hicks
2011-04-28 0:06 ` Tyler Hicks
2011-04-28 12:35 ` Roberto Sassu
2011-04-28 12:35 ` Roberto Sassu
2011-04-28 17:37 ` Casey Schaufler
2011-04-28 17:37 ` Casey Schaufler
2011-04-28 17:56 ` Eric Paris
2011-04-28 17:56 ` Eric Paris
2011-04-29 9:26 ` Roberto Sassu
2011-04-29 9:26 ` Roberto Sassu
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=4DB8A53A.4010709@schaufler-ca.com \
--to=casey@schaufler-ca.com \
--cc=dhowells@redhat.com \
--cc=ecryptfs-devel@lists.launchpad.net \
--cc=eparis@redhat.com \
--cc=jmorris@namei.org \
--cc=kirkland@canonical.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=roberto.sassu@polito.it \
--cc=safford@watson.ibm.com \
--cc=sds@tycho.nsa.gov \
--cc=selinux@tycho.nsa.gov \
--cc=tyhicks@linux.vnet.ibm.com \
--cc=viro@zeniv.linux.org.uk \
--cc=zohar@linux.vnet.ibm.com \
/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.