Skip to contentSkip to conversation

selinux: fix error codes in cond_read_av_list()

PATCH Superseded Dan Carpenter 2 participants 2 messages Jun 7, 2010 – Jun 8, 2010

Subsystems (2)
Kernel janitors SELinux security module

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH 3/7 selinux: fix error codes in cond_read_av_list()

After this patch cond_read_av_list() no longer returns -1 for any errors. It just propagates error code back from lower levels. Those can either be -EINVAL or -ENOMEM.

I also modified cond_insertf() since cond_read_av_list() passes that as a function pointer to avtab_read_item(). It isn't used anywhere else.

Signed-off-by Dan Carpenter <[email protected]>
security/selinux/ss/conditional.c +6 -5
index acaa6cd..4c39f19 100644
@@ -263,7 +263,7 @@ static int cond_insertf(struct avtab *a, struct avtab_key *k, struct avtab_datum
 	struct cond_av_list *other = data->other, *list, *cur;
 	struct avtab_node *node_ptr;
 	u8 found;
-
+	int ret = -EINVAL;
 
 	/*
 	 * For type rules we have to make certain there aren't any

@@ -317,8 +317,10 @@ static int cond_insertf(struct avtab *a, struct avtab_key *k, struct avtab_datum
 	}
 
 	list = kzalloc(sizeof(struct cond_av_list), GFP_KERNEL);
-	if (!list)
+	if (!list) {
+		ret = -ENOMEM;
 		goto err;
+	}
 
 	list->node = node_ptr;
 	if (!data->head)

@@ -331,7 +333,7 @@ static int cond_insertf(struct avtab *a, struct avtab_key *k, struct avtab_datum
 err:
 	cond_av_list_destroy(data->head);
 	data->head = NULL;
-	return -1;
+	return ret;
 }
 
 static int cond_read_av_list(struct policydb *p, void *fp, struct cond_av_list **ret_list, struct cond_av_list *other)

@@ -346,7 +348,7 @@ static int cond_read_av_list(struct policydb *p, void *fp, struct cond_av_list *
 	len = 0;
 	rc = next_entry(buf, fp, sizeof(u32));
 	if (rc < 0)
-		return -1;
+		return rc;
 
 	len = le32_to_cpu(buf[0]);
 	if (len = 0)

@@ -361,7 +363,6 @@ static int cond_read_av_list(struct policydb *p, void *fp, struct cond_av_list *
 				     &data);
 		if (rc)
 			return rc;
-
 	}
 
 	*ret_list = data.head;
#1 lore raw
Quoted 8 lines · Dan Carpenter

After this patch cond_read_av_list() no longer returns -1 for any errors.  It just propagates error code back from lower levels.  Those can either be -EINVAL or -ENOMEM.

I also modified cond_insertf() since cond_read_av_list() passes that as a function pointer to avtab_read_item().  It isn't used anywhere else.

Signed-off-by Dan Carpenter <[email protected]>

I'm not in love with this one. The change in cond_read_av_list() is fine, but the style and completeness in cond_insertf I think is even worse. My biggest problem is the fact that you sent an error code at the top and just sorta leave it. There are still lots of

if (function())

        goto err;

which really should be

rc = function()
if (rc)

        goto err;

I'd rather keep the crap we have today than introduce a new style of error code handling....

-Eric

Quoted 46 lines
security/selinux/ss/conditional.c +4 -2
index acaa6cd..4c39f19 100644
@@ -263,7 +263,7 @@ static int cond_insertf(struct avtab *a, struct avtab_key *k, struct avtab_datum
        struct cond_av_list *other = data->other, *list, *cur;
        struct avtab_node *node_ptr;
        u8 found;
-
+       int ret = -EINVAL;
 
        /*
         * For type rules we have to make certain there aren't any

@@ -317,8 +317,10 @@ static int cond_insertf(struct avtab *a, struct avtab_key *k, struct avtab_datum
        }
 
        list = kzalloc(sizeof(struct cond_av_list), GFP_KERNEL);
-       if (!list)
+       if (!list) {
+               ret = -ENOMEM;
                goto err;
+       }
 
        list->node = node_ptr;
        if (!data->head)

@@ -331,7 +333,7 @@ static int cond_insertf(struct avtab *a, struct avtab_key *k, struct avtab_datum

 err:
       cond_av_list_destroy(data->head);
       data->head = NULL;
-       return -1;
+       return ret;
 }

 static int cond_read_av_list(struct policydb *p, void *fp, struct cond_av_list **ret_list, struct cond_av_list *other)

security/selinux/ss/conditional.c +1 -1
@@ -346,7 +348,7 @@ static int cond_read_av_list(struct policydb *p, void *fp, struct cond_av_list *
        len = 0;
        rc = next_entry(buf, fp, sizeof(u32));
        if (rc < 0)
-               return -1;
+               return rc;
 
        len = le32_to_cpu(buf[0]);
        if (len = 0)

security/selinux/ss/conditional.c static int cond_read_av_list(struct policydb *p, void *fp, struct cond_av_list * · line 368

Quoted 7 lines
security/selinux/ss/conditional.c +0 -1
@@ -361,7 +363,6 @@ static int cond_read_av_list(struct policydb *p, void *fp, struct cond_av_list *
                                     &data);
                if (rc)
                        return rc;
-
        }
 
        *ret_list = data.head;

--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html