Skip to content

Commit

Permalink
smack: pass error code through pointers
Browse files Browse the repository at this point in the history 
This patch makes the following functions to use ERR_PTR() and related
macros to pass the appropriate error code through returned pointers:

smk_parse_smack()
smk_import_entry()
smk_fetch()

It also makes all the other functions that use them to handle the
error cases properly. This ways correct error codes from places
where they happened can be propagated to the user space if necessary.

Doing this it fixes a bug in onlycap and unconfined files
handling. Previously their content was cleared on any error from
smk_import_entry/smk_parse_smack, be it EINVAL (as originally intended)
or ENOMEM. Right now it only reacts on EINVAL passing other codes
properly to userspace.

Comments have been updated accordingly.

Signed-off-by: Lukasz Pawelczyk <l.pawelczyk@samsung.com>
  • Loading branch information
Lukasz Pawelczyk authored and Casey Schaufler committed May 15, 2015
1 parent 9777582 commit e774ad6
Show file tree
Hide file tree
Showing 3 changed files with 139 additions and 97 deletions.
27 changes: 16 additions & 11 deletions security/smack/smack_access.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,7 @@ void smk_insert_entry(struct smack_known *skp)
* @string: a text string that might be a Smack label
*
* Returns a pointer to the entry in the label list that
* matches the passed string.
* matches the passed string or NULL if not found.
*/
struct smack_known *smk_find_entry(const char *string)
{
Expand All @@ -448,7 +448,7 @@ struct smack_known *smk_find_entry(const char *string)
* @string: a text string that might contain a Smack label
* @len: the maximum size, or zero if it is NULL terminated.
*
* Returns a pointer to the clean label, or NULL
* Returns a pointer to the clean label or an error code.
*/
char *smk_parse_smack(const char *string, int len)
{
Expand All @@ -464,19 +464,21 @@ char *smk_parse_smack(const char *string, int len)
* including /smack/cipso and /smack/cipso2
*/
if (string[0] == '-')
return NULL;
return ERR_PTR(-EINVAL);

for (i = 0; i < len; i++)
if (string[i] > '~' || string[i] <= ' ' || string[i] == '/' ||
string[i] == '"' || string[i] == '\\' || string[i] == '\'')
break;

if (i == 0 || i >= SMK_LONGLABEL)
return NULL;
return ERR_PTR(-EINVAL);

smack = kzalloc(i + 1, GFP_KERNEL);
if (smack != NULL)
strncpy(smack, string, i);
if (smack == NULL)
return ERR_PTR(-ENOMEM);

strncpy(smack, string, i);

return smack;
}
Expand Down Expand Up @@ -523,7 +525,8 @@ int smk_netlbl_mls(int level, char *catset, struct netlbl_lsm_secattr *sap,
* @len: the maximum size, or zero if it is NULL terminated.
*
* Returns a pointer to the entry in the label list that
* matches the passed string, adding it if necessary.
* matches the passed string, adding it if necessary,
* or an error code.
*/
struct smack_known *smk_import_entry(const char *string, int len)
{
Expand All @@ -533,8 +536,8 @@ struct smack_known *smk_import_entry(const char *string, int len)
int rc;

smack = smk_parse_smack(string, len);
if (smack == NULL)
return NULL;
if (IS_ERR(smack))
return ERR_CAST(smack);

mutex_lock(&smack_known_lock);

Expand All @@ -543,8 +546,10 @@ struct smack_known *smk_import_entry(const char *string, int len)
goto freeout;

skp = kzalloc(sizeof(*skp), GFP_KERNEL);
if (skp == NULL)
if (skp == NULL) {
skp = ERR_PTR(-ENOMEM);
goto freeout;
}

skp->smk_known = smack;
skp->smk_secid = smack_next_secid++;
Expand Down Expand Up @@ -577,7 +582,7 @@ struct smack_known *smk_import_entry(const char *string, int len)
* smk_netlbl_mls failed.
*/
kfree(skp);
skp = NULL;
skp = ERR_PTR(rc);
freeout:
kfree(smack);
unlockout:
Expand Down
93 changes: 54 additions & 39 deletions security/smack/smack_lsm.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -245,8 +245,8 @@ static int smk_bu_credfile(const struct cred *cred, struct file *file,
* @ip: a pointer to the inode
* @dp: a pointer to the dentry
*
* Returns a pointer to the master list entry for the Smack label
* or NULL if there was no label to fetch.
* Returns a pointer to the master list entry for the Smack label,
* NULL if there was no label to fetch, or an error code.
*/
static struct smack_known *smk_fetch(const char *name, struct inode *ip,
struct dentry *dp)
Expand All @@ -256,14 +256,18 @@ static struct smack_known *smk_fetch(const char *name, struct inode *ip,
struct smack_known *skp = NULL;

if (ip->i_op->getxattr == NULL)
return NULL;
return ERR_PTR(-EOPNOTSUPP);

buffer = kzalloc(SMK_LONGLABEL, GFP_KERNEL);
if (buffer == NULL)
return NULL;
return ERR_PTR(-ENOMEM);

rc = ip->i_op->getxattr(dp, name, buffer, SMK_LONGLABEL);
if (rc > 0)
if (rc < 0)
skp = ERR_PTR(rc);
else if (rc == 0)
skp = NULL;
else
skp = smk_import_entry(buffer, rc);

kfree(buffer);
Expand Down Expand Up @@ -605,40 +609,44 @@ static int smack_sb_kern_mount(struct super_block *sb, int flags, void *data)
if (strncmp(op, SMK_FSHAT, strlen(SMK_FSHAT)) == 0) {
op += strlen(SMK_FSHAT);
skp = smk_import_entry(op, 0);
if (skp != NULL) {
sp->smk_hat = skp;
specified = 1;
}
if (IS_ERR(skp))
return PTR_ERR(skp);
sp->smk_hat = skp;
specified = 1;

} else if (strncmp(op, SMK_FSFLOOR, strlen(SMK_FSFLOOR)) == 0) {
op += strlen(SMK_FSFLOOR);
skp = smk_import_entry(op, 0);
if (skp != NULL) {
sp->smk_floor = skp;
specified = 1;
}
if (IS_ERR(skp))
return PTR_ERR(skp);
sp->smk_floor = skp;
specified = 1;

} else if (strncmp(op, SMK_FSDEFAULT,
strlen(SMK_FSDEFAULT)) == 0) {
op += strlen(SMK_FSDEFAULT);
skp = smk_import_entry(op, 0);
if (skp != NULL) {
sp->smk_default = skp;
specified = 1;
}
if (IS_ERR(skp))
return PTR_ERR(skp);
sp->smk_default = skp;
specified = 1;

} else if (strncmp(op, SMK_FSROOT, strlen(SMK_FSROOT)) == 0) {
op += strlen(SMK_FSROOT);
skp = smk_import_entry(op, 0);
if (skp != NULL) {
sp->smk_root = skp;
specified = 1;
}
if (IS_ERR(skp))
return PTR_ERR(skp);
sp->smk_root = skp;
specified = 1;

} else if (strncmp(op, SMK_FSTRANS, strlen(SMK_FSTRANS)) == 0) {
op += strlen(SMK_FSTRANS);
skp = smk_import_entry(op, 0);
if (skp != NULL) {
sp->smk_root = skp;
transmute = 1;
specified = 1;
}
if (IS_ERR(skp))
return PTR_ERR(skp);
sp->smk_root = skp;
transmute = 1;
specified = 1;
}
}

Expand Down Expand Up @@ -1118,7 +1126,9 @@ static int smack_inode_setxattr(struct dentry *dentry, const char *name,

if (rc == 0 && check_import) {
skp = size ? smk_import_entry(value, size) : NULL;
if (skp == NULL || (check_star &&
if (IS_ERR(skp))
rc = PTR_ERR(skp);
else if (skp == NULL || (check_star &&
(skp == &smack_known_star || skp == &smack_known_web)))
rc = -EINVAL;
}
Expand Down Expand Up @@ -1158,19 +1168,19 @@ static void smack_inode_post_setxattr(struct dentry *dentry, const char *name,

if (strcmp(name, XATTR_NAME_SMACK) == 0) {
skp = smk_import_entry(value, size);
if (skp != NULL)
if (!IS_ERR(skp))
isp->smk_inode = skp;
else
isp->smk_inode = &smack_known_invalid;
} else if (strcmp(name, XATTR_NAME_SMACKEXEC) == 0) {
skp = smk_import_entry(value, size);
if (skp != NULL)
if (!IS_ERR(skp))
isp->smk_task = skp;
else
isp->smk_task = &smack_known_invalid;
} else if (strcmp(name, XATTR_NAME_SMACKMMAP) == 0) {
skp = smk_import_entry(value, size);
if (skp != NULL)
if (!IS_ERR(skp))
isp->smk_mmap = skp;
else
isp->smk_mmap = &smack_known_invalid;
Expand Down Expand Up @@ -2403,8 +2413,8 @@ static int smack_inode_setsecurity(struct inode *inode, const char *name,
return -EINVAL;

skp = smk_import_entry(value, size);
if (skp == NULL)
return -EINVAL;
if (IS_ERR(skp))
return PTR_ERR(skp);

if (strcmp(name, XATTR_SMACK_SUFFIX) == 0) {
nsp->smk_inode = skp;
Expand Down Expand Up @@ -3177,7 +3187,7 @@ static void smack_d_instantiate(struct dentry *opt_dentry, struct inode *inode)
*/
dp = dget(opt_dentry);
skp = smk_fetch(XATTR_NAME_SMACK, inode, dp);
if (skp != NULL)
if (!IS_ERR_OR_NULL(skp))
final = skp;

/*
Expand Down Expand Up @@ -3214,11 +3224,14 @@ static void smack_d_instantiate(struct dentry *opt_dentry, struct inode *inode)
* Don't let the exec or mmap label be "*" or "@".
*/
skp = smk_fetch(XATTR_NAME_SMACKEXEC, inode, dp);
if (skp == &smack_known_star || skp == &smack_known_web)
if (IS_ERR(skp) || skp == &smack_known_star ||
skp == &smack_known_web)
skp = NULL;
isp->smk_task = skp;

skp = smk_fetch(XATTR_NAME_SMACKMMAP, inode, dp);
if (skp == &smack_known_star || skp == &smack_known_web)
if (IS_ERR(skp) || skp == &smack_known_star ||
skp == &smack_known_web)
skp = NULL;
isp->smk_mmap = skp;

Expand Down Expand Up @@ -3302,8 +3315,8 @@ static int smack_setprocattr(struct task_struct *p, char *name,
return -EINVAL;

skp = smk_import_entry(value, size);
if (skp == NULL)
return -EINVAL;
if (IS_ERR(skp))
return PTR_ERR(skp);

/*
* No process is ever allowed the web ("@") label.
Expand Down Expand Up @@ -4078,8 +4091,10 @@ static int smack_audit_rule_init(u32 field, u32 op, char *rulestr, void **vrule)
return -EINVAL;

skp = smk_import_entry(rulestr, 0);
if (skp)
*rule = skp->smk_known;
if (IS_ERR(skp))
return PTR_ERR(skp);

*rule = skp->smk_known;

return 0;
}
Expand Down
Loading

0 comments on commit e774ad6

Please sign in to comment.