On Thu, 22 Apr 2010, Eric Paris wrote:
> I screwed up the subject and the changelog and said selinuxfs instead
> of services.c. James do you want to just fix those if you take these
> patches?
I can fix them.
> If I have to redo them I will fix them.
>
> -eric
>
> On Thu, Apr 22, 2010 at 1:36 PM, Eric Paris <eparis@xxxxxxxxxx> wrote:
> > selinuxfs.c has lots of different standards on how to handle return paths on
> > error. For the most part transition to
> >
> > rc=errno
> > if (failure)
> > goto out;
> > [...]
> > out:
> > cleanup()
> > return rc;
> >
> > Instead of doing cleanup mid function, or having multiple returns or other
> > options. This doesn't do that for every function, but most of the complex
> > functions which have cleanup routines on error.
> >
> > Signed-off-by: Eric Paris <eparis@xxxxxxxxxx>
> > ---
> >
> > security/selinux/ss/services.c | 57 ++++++++++++++++------------------------
> > 1 files changed, 23 insertions(+), 34 deletions(-)
> >
> > diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
> > index 1de60ce..5bb58e6 100644
> > --- a/security/selinux/ss/services.c
> > +++ b/security/selinux/ss/services.c
> > @@ -1558,22 +1558,17 @@ static int clone_sid(u32 sid,
> >
> > static inline int convert_context_handle_invalid_context(struct context *context)
> > {
> > - int rc = 0;
> > + char *s;
> > + u32 len;
> >
> > - if (selinux_enforcing) {
> > - rc = -EINVAL;
> > - } else {
> > - char *s;
> > - u32 len;
> > -
> > - if (!context_struct_to_string(context, &s, &len)) {
> > - printk(KERN_WARNING
> > - "SELinux: Context %s would be invalid if enforcing\n",
> > - s);
> > - kfree(s);
> > - }
> > + if (selinux_enforcing)
> > + return -EINVAL;
> > +
> > + if (!context_struct_to_string(context, &s, &len)) {
> > + printk(KERN_WARNING "SELinux: Context %s would be invalid if enforcing\n", s);
> > + kfree(s);
> > }
> > - return rc;
> > + return 0;
> > }
> >
> > struct convert_context_args {
> > @@ -1619,8 +1614,7 @@ static int convert_context(u32 key,
> > c->len, &ctx, SECSID_NULL);
> > kfree(s);
> > if (!rc) {
> > - printk(KERN_INFO
> > - "SELinux: Context %s became valid (mapped).\n",
> > + printk(KERN_INFO "SELinux: Context %s became valid (mapped).\n",
> > c->str);
> > /* Replace string with mapped representation. */
> > kfree(c->str);
> > @@ -1632,8 +1626,7 @@ static int convert_context(u32 key,
> > goto out;
> > } else {
> > /* Other error condition, e.g. ENOMEM. */
> > - printk(KERN_ERR
> > - "SELinux: Unable to map context %s, rc = %d.\n",
> > + printk(KERN_ERR "SELinux: Unable to map context %s, rc = %d.\n",
> > c->str, -rc);
> > goto out;
> > }
> > @@ -1719,8 +1712,7 @@ bad:
> > context_destroy(c);
> > c->str = s;
> > c->len = len;
> > - printk(KERN_INFO
> > - "SELinux: Context %s became invalid (unmapped).\n",
> > + printk(KERN_INFO "SELinux: Context %s became invalid (unmapped).\n",
> > c->str);
> > rc = 0;
> > goto out;
> > @@ -2087,24 +2079,22 @@ int security_get_user_sids(u32 fromsid,
> >
> > context_init(&usercon);
> >
> > + rc = -EINVAL;
> > fromcon = sidtab_search(&sidtab, fromsid);
> > - if (!fromcon) {
> > - rc = -EINVAL;
> > + if (!fromcon)
> > goto out_unlock;
> > - }
> >
> > + rc = -EINVAL;
> > user = hashtab_search(policydb.p_users.table, username);
> > - if (!user) {
> > - rc = -EINVAL;
> > + if (!user)
> > goto out_unlock;
> > - }
> > +
> > usercon.user = user->value;
> >
> > + rc = -ENOMEM;
> > mysids = kcalloc(maxnel, sizeof(*mysids), GFP_ATOMIC);
> > - if (!mysids) {
> > - rc = -ENOMEM;
> > + if (!mysids)
> > goto out_unlock;
> > - }
> >
> > ebitmap_for_each_positive_bit(&user->roles, rnode, i) {
> > role = policydb.role_val_to_struct[i];
> > @@ -2121,12 +2111,11 @@ int security_get_user_sids(u32 fromsid,
> > if (mynel < maxnel) {
> > mysids[mynel++] = sid;
> > } else {
> > + rc = -ENOMEM;
> > maxnel += SIDS_NEL;
> > mysids2 = kcalloc(maxnel, sizeof(*mysids2), GFP_ATOMIC);
> > - if (!mysids2) {
> > - rc = -ENOMEM;
> > + if (!mysids2)
> > goto out_unlock;
> > - }
> > memcpy(mysids2, mysids, mynel * sizeof(*mysids2));
> > kfree(mysids);
> > mysids = mysids2;
> > @@ -2134,7 +2123,7 @@ int security_get_user_sids(u32 fromsid,
> > }
> > }
> > }
> > -
> > + rc = 0;
> > out_unlock:
> > read_unlock(&policy_rwlock);
> > if (rc || !mynel) {
> > @@ -2142,9 +2131,9 @@ out_unlock:
> > goto out;
> > }
> >
> > + rc = -ENOMEM;
> > mysids2 = kcalloc(mynel, sizeof(*mysids2), GFP_KERNEL);
> > if (!mysids2) {
> > - rc = -ENOMEM;
> > kfree(mysids);
> > goto out;
> > }
> >
> >
> > --
> > This message was distributed to subscribers of the selinux mailing list.
> > If you no longer wish to subscribe, send mail to majordomo@xxxxxxxxxxxxx with
> > the words "unsubscribe selinux" without quotes as the message.
> >
>
--
James Morris
<jmorris@xxxxxxxxx>
