From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932151AbdCWVoM (ORCPT ); Thu, 23 Mar 2017 17:44:12 -0400 Received: from mail-vk0-f65.google.com ([209.85.213.65]:35107 "EHLO mail-vk0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751584AbdCWVoJ (ORCPT ); Thu, 23 Mar 2017 17:44:09 -0400 MIME-Version: 1.0 X-Originating-IP: [108.49.102.27] In-Reply-To: <7fdcfc8f-affa-fcf8-adaf-dc8fd9e1b472@users.sourceforge.net> References: <7fdcfc8f-affa-fcf8-adaf-dc8fd9e1b472@users.sourceforge.net> From: Paul Moore Date: Thu, 23 Mar 2017 17:44:07 -0400 Message-ID: Subject: Re: [PATCH 10/46] selinux: Move some assignments for the variable "rc" in policydb_read() To: SF Markus Elfring Cc: linux-security-module@vger.kernel.org, selinux@tycho.nsa.gov, Eric Paris , James Morris , "Serge E. Hallyn" , Stephen Smalley , William Roberts , LKML , kernel-janitors@vger.kernel.org Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sun, Jan 15, 2017 at 10:10 AM, SF Markus Elfring wrote: > From: Markus Elfring > Date: Sat, 14 Jan 2017 15:22:29 +0100 > > One local variable was set to an error code in some cases before > a concrete error situation was detected. Thus move the corresponding > assignments into if branches to indicate a software failure there. > > Signed-off-by: Markus Elfring > --- > security/selinux/ss/policydb.c | 59 +++++++++++++++++++++++++----------------- > 1 file changed, 35 insertions(+), 24 deletions(-) More code churn with no real advantage. I agree with the style you are using, and would support changing it if you are in the function fixing bugs or doing other substantial changes in that code, but I can't justify it as a standalone change, sorry. > diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c > index 53e6d06e772a..506b0228d1f1 100644 > --- a/security/selinux/ss/policydb.c > +++ b/security/selinux/ss/policydb.c > @@ -2250,15 +2250,14 @@ int policydb_read(struct policydb *p, void *fp) > if (rc) > goto bad; > > - rc = -EINVAL; > if (le32_to_cpu(buf[0]) != POLICYDB_MAGIC) { > printk(KERN_ERR "SELinux: policydb magic number 0x%x does " > "not match expected magic number 0x%x\n", > le32_to_cpu(buf[0]), POLICYDB_MAGIC); > + rc = -EINVAL; > goto bad; > } > > - rc = -EINVAL; > len = le32_to_cpu(buf[1]); > if (len != strlen(POLICYDB_STRING)) { > printk(KERN_ERR "SELinux: policydb string length %d does not " > @@ -2265,11 +2265,13 @@ int policydb_read(struct policydb *p, void *fp) > len, strlen(POLICYDB_STRING)); > + rc = -EINVAL; > goto bad; > } > > - rc = -ENOMEM; > policydb_str = kmalloc(len + 1, GFP_KERNEL); > - if (!policydb_str) > + if (!policydb_str) { > + rc = -ENOMEM; > goto bad; > + } > > rc = next_entry(policydb_str, fp, len); > if (rc) { > @@ -2279,12 +2280,12 @@ int policydb_read(struct policydb *p, void *fp) > goto bad; > } > > - rc = -EINVAL; > policydb_str[len] = '\0'; > if (strcmp(policydb_str, POLICYDB_STRING)) { > printk(KERN_ERR "SELinux: policydb string %s does not match " > "my string %s\n", policydb_str, POLICYDB_STRING); > kfree(policydb_str); > + rc = -EINVAL; > goto bad; > } > /* Done with policydb_str. */ > @@ -2296,24 +2297,24 @@ int policydb_read(struct policydb *p, void *fp) > if (rc) > goto bad; > > - rc = -EINVAL; > p->policyvers = le32_to_cpu(buf[0]); > if (p->policyvers < POLICYDB_VERSION_MIN || > p->policyvers > POLICYDB_VERSION_MAX) { > printk(KERN_ERR "SELinux: policydb version %d does not match " > "my version range %d-%d\n", > le32_to_cpu(buf[0]), POLICYDB_VERSION_MIN, POLICYDB_VERSION_MAX); > + rc = -EINVAL; > goto bad; > } > > if ((le32_to_cpu(buf[1]) & POLICYDB_CONFIG_MLS)) { > p->mls_enabled = 1; > > - rc = -EINVAL; > if (p->policyvers < POLICYDB_VERSION_MLS) { > printk(KERN_ERR "SELinux: security policydb version %d " > "(MLS) not backwards compatible\n", > p->policyvers); > + rc = -EINVAL; > goto bad; > } > } > @@ -2332,21 +2333,21 @@ int policydb_read(struct policydb *p, void *fp) > goto bad; > } > > - rc = -EINVAL; > info = policydb_lookup_compat(p->policyvers); > if (!info) { > printk(KERN_ERR "SELinux: unable to find policy compat info " > "for version %d\n", p->policyvers); > + rc = -EINVAL; > goto bad; > } > > - rc = -EINVAL; > if (le32_to_cpu(buf[2]) != info->sym_num || > le32_to_cpu(buf[3]) != info->ocon_num) { > printk(KERN_ERR "SELinux: policydb table sizes (%d,%d) do " > "not match mine (%d,%d)\n", le32_to_cpu(buf[2]), > le32_to_cpu(buf[3]), > info->sym_num, info->ocon_num); > + rc = -EINVAL; > goto bad; > } > > @@ -2365,10 +2366,11 @@ int policydb_read(struct policydb *p, void *fp) > p->symtab[i].nprim = nprim; > } > > - rc = -EINVAL; > p->process_class = string_to_security_class(p, "process"); > - if (!p->process_class) > + if (!p->process_class) { > + rc = -EINVAL; > goto bad; > + } > > rc = avtab_read(&p->te_avtab, fp, p); > if (rc) > @@ -2386,10 +2388,12 @@ int policydb_read(struct policydb *p, void *fp) > nel = le32_to_cpu(buf[0]); > ltr = NULL; > for (i = 0; i < nel; i++) { > - rc = -ENOMEM; > tr = kzalloc(sizeof(*tr), GFP_KERNEL); > - if (!tr) > + if (!tr) { > + rc = -ENOMEM; > goto bad; > + } > + > if (ltr) > ltr->next = tr; > else > @@ -2398,7 +2402,6 @@ int policydb_read(struct policydb *p, void *fp) > if (rc) > goto bad; > > - rc = -EINVAL; > tr->role = le32_to_cpu(buf[0]); > tr->type = le32_to_cpu(buf[1]); > tr->new_role = le32_to_cpu(buf[2]); > @@ -2410,12 +2413,14 @@ int policydb_read(struct policydb *p, void *fp) > } else > tr->tclass = p->process_class; > > - rc = -EINVAL; > if (!policydb_role_isvalid(p, tr->role) || > !policydb_type_isvalid(p, tr->type) || > !policydb_class_isvalid(p, tr->tclass) || > - !policydb_role_isvalid(p, tr->new_role)) > + !policydb_role_isvalid(p, tr->new_role)) { > + rc = -EINVAL; > goto bad; > + } > + > ltr = tr; > } > > @@ -2425,10 +2430,12 @@ int policydb_read(struct policydb *p, void *fp) > nel = le32_to_cpu(buf[0]); > lra = NULL; > for (i = 0; i < nel; i++) { > - rc = -ENOMEM; > ra = kzalloc(sizeof(*ra), GFP_KERNEL); > - if (!ra) > + if (!ra) { > + rc = -ENOMEM; > goto bad; > + } > + > if (lra) > lra->next = ra; > else > @@ -2437,12 +2444,14 @@ int policydb_read(struct policydb *p, void *fp) > if (rc) > goto bad; > > - rc = -EINVAL; > ra->role = le32_to_cpu(buf[0]); > ra->new_role = le32_to_cpu(buf[1]); > if (!policydb_role_isvalid(p, ra->role) || > - !policydb_role_isvalid(p, ra->new_role)) > + !policydb_role_isvalid(p, ra->new_role)) { > + rc = -EINVAL; > goto bad; > + } > + > lra = ra; > } > > @@ -2454,11 +2463,12 @@ int policydb_read(struct policydb *p, void *fp) > if (rc) > goto bad; > > - rc = -EINVAL; > p->process_trans_perms = string_to_av_perm(p, p->process_class, "transition"); > p->process_trans_perms |= string_to_av_perm(p, p->process_class, "dyntransition"); > - if (!p->process_trans_perms) > + if (!p->process_trans_perms) { > + rc = -EINVAL; > goto bad; > + } > > rc = ocontext_read(p, info, fp); > if (rc) > @@ -2472,12 +2482,13 @@ int policydb_read(struct policydb *p, void *fp) > if (rc) > goto bad; > > - rc = -ENOMEM; > p->type_attr_map_array = flex_array_alloc(sizeof(struct ebitmap), > p->p_types.nprim, > GFP_KERNEL | __GFP_ZERO); > - if (!p->type_attr_map_array) > + if (!p->type_attr_map_array) { > + rc = -ENOMEM; > goto bad; > + } > > /* preallocate so we don't have to worry about the put ever failing */ > rc = flex_array_prealloc(p->type_attr_map_array, 0, p->p_types.nprim, > -- > 2.11.0 > -- paul moore www.paul-moore.com From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paul Moore Date: Thu, 23 Mar 2017 21:44:07 +0000 Subject: Re: [PATCH 10/46] selinux: Move some assignments for the variable "rc" in policydb_read() Message-Id: List-Id: References: <7fdcfc8f-affa-fcf8-adaf-dc8fd9e1b472@users.sourceforge.net> In-Reply-To: <7fdcfc8f-affa-fcf8-adaf-dc8fd9e1b472@users.sourceforge.net> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: linux-security-module@vger.kernel.org On Sun, Jan 15, 2017 at 10:10 AM, SF Markus Elfring wrote: > From: Markus Elfring > Date: Sat, 14 Jan 2017 15:22:29 +0100 > > One local variable was set to an error code in some cases before > a concrete error situation was detected. Thus move the corresponding > assignments into if branches to indicate a software failure there. > > Signed-off-by: Markus Elfring > --- > security/selinux/ss/policydb.c | 59 +++++++++++++++++++++++++----------------- > 1 file changed, 35 insertions(+), 24 deletions(-) More code churn with no real advantage. I agree with the style you are using, and would support changing it if you are in the function fixing bugs or doing other substantial changes in that code, but I can't justify it as a standalone change, sorry. > diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c > index 53e6d06e772a..506b0228d1f1 100644 > --- a/security/selinux/ss/policydb.c > +++ b/security/selinux/ss/policydb.c > @@ -2250,15 +2250,14 @@ int policydb_read(struct policydb *p, void *fp) > if (rc) > goto bad; > > - rc = -EINVAL; > if (le32_to_cpu(buf[0]) != POLICYDB_MAGIC) { > printk(KERN_ERR "SELinux: policydb magic number 0x%x does " > "not match expected magic number 0x%x\n", > le32_to_cpu(buf[0]), POLICYDB_MAGIC); > + rc = -EINVAL; > goto bad; > } > > - rc = -EINVAL; > len = le32_to_cpu(buf[1]); > if (len != strlen(POLICYDB_STRING)) { > printk(KERN_ERR "SELinux: policydb string length %d does not " > @@ -2265,11 +2265,13 @@ int policydb_read(struct policydb *p, void *fp) > len, strlen(POLICYDB_STRING)); > + rc = -EINVAL; > goto bad; > } > > - rc = -ENOMEM; > policydb_str = kmalloc(len + 1, GFP_KERNEL); > - if (!policydb_str) > + if (!policydb_str) { > + rc = -ENOMEM; > goto bad; > + } > > rc = next_entry(policydb_str, fp, len); > if (rc) { > @@ -2279,12 +2280,12 @@ int policydb_read(struct policydb *p, void *fp) > goto bad; > } > > - rc = -EINVAL; > policydb_str[len] = '\0'; > if (strcmp(policydb_str, POLICYDB_STRING)) { > printk(KERN_ERR "SELinux: policydb string %s does not match " > "my string %s\n", policydb_str, POLICYDB_STRING); > kfree(policydb_str); > + rc = -EINVAL; > goto bad; > } > /* Done with policydb_str. */ > @@ -2296,24 +2297,24 @@ int policydb_read(struct policydb *p, void *fp) > if (rc) > goto bad; > > - rc = -EINVAL; > p->policyvers = le32_to_cpu(buf[0]); > if (p->policyvers < POLICYDB_VERSION_MIN || > p->policyvers > POLICYDB_VERSION_MAX) { > printk(KERN_ERR "SELinux: policydb version %d does not match " > "my version range %d-%d\n", > le32_to_cpu(buf[0]), POLICYDB_VERSION_MIN, POLICYDB_VERSION_MAX); > + rc = -EINVAL; > goto bad; > } > > if ((le32_to_cpu(buf[1]) & POLICYDB_CONFIG_MLS)) { > p->mls_enabled = 1; > > - rc = -EINVAL; > if (p->policyvers < POLICYDB_VERSION_MLS) { > printk(KERN_ERR "SELinux: security policydb version %d " > "(MLS) not backwards compatible\n", > p->policyvers); > + rc = -EINVAL; > goto bad; > } > } > @@ -2332,21 +2333,21 @@ int policydb_read(struct policydb *p, void *fp) > goto bad; > } > > - rc = -EINVAL; > info = policydb_lookup_compat(p->policyvers); > if (!info) { > printk(KERN_ERR "SELinux: unable to find policy compat info " > "for version %d\n", p->policyvers); > + rc = -EINVAL; > goto bad; > } > > - rc = -EINVAL; > if (le32_to_cpu(buf[2]) != info->sym_num || > le32_to_cpu(buf[3]) != info->ocon_num) { > printk(KERN_ERR "SELinux: policydb table sizes (%d,%d) do " > "not match mine (%d,%d)\n", le32_to_cpu(buf[2]), > le32_to_cpu(buf[3]), > info->sym_num, info->ocon_num); > + rc = -EINVAL; > goto bad; > } > > @@ -2365,10 +2366,11 @@ int policydb_read(struct policydb *p, void *fp) > p->symtab[i].nprim = nprim; > } > > - rc = -EINVAL; > p->process_class = string_to_security_class(p, "process"); > - if (!p->process_class) > + if (!p->process_class) { > + rc = -EINVAL; > goto bad; > + } > > rc = avtab_read(&p->te_avtab, fp, p); > if (rc) > @@ -2386,10 +2388,12 @@ int policydb_read(struct policydb *p, void *fp) > nel = le32_to_cpu(buf[0]); > ltr = NULL; > for (i = 0; i < nel; i++) { > - rc = -ENOMEM; > tr = kzalloc(sizeof(*tr), GFP_KERNEL); > - if (!tr) > + if (!tr) { > + rc = -ENOMEM; > goto bad; > + } > + > if (ltr) > ltr->next = tr; > else > @@ -2398,7 +2402,6 @@ int policydb_read(struct policydb *p, void *fp) > if (rc) > goto bad; > > - rc = -EINVAL; > tr->role = le32_to_cpu(buf[0]); > tr->type = le32_to_cpu(buf[1]); > tr->new_role = le32_to_cpu(buf[2]); > @@ -2410,12 +2413,14 @@ int policydb_read(struct policydb *p, void *fp) > } else > tr->tclass = p->process_class; > > - rc = -EINVAL; > if (!policydb_role_isvalid(p, tr->role) || > !policydb_type_isvalid(p, tr->type) || > !policydb_class_isvalid(p, tr->tclass) || > - !policydb_role_isvalid(p, tr->new_role)) > + !policydb_role_isvalid(p, tr->new_role)) { > + rc = -EINVAL; > goto bad; > + } > + > ltr = tr; > } > > @@ -2425,10 +2430,12 @@ int policydb_read(struct policydb *p, void *fp) > nel = le32_to_cpu(buf[0]); > lra = NULL; > for (i = 0; i < nel; i++) { > - rc = -ENOMEM; > ra = kzalloc(sizeof(*ra), GFP_KERNEL); > - if (!ra) > + if (!ra) { > + rc = -ENOMEM; > goto bad; > + } > + > if (lra) > lra->next = ra; > else > @@ -2437,12 +2444,14 @@ int policydb_read(struct policydb *p, void *fp) > if (rc) > goto bad; > > - rc = -EINVAL; > ra->role = le32_to_cpu(buf[0]); > ra->new_role = le32_to_cpu(buf[1]); > if (!policydb_role_isvalid(p, ra->role) || > - !policydb_role_isvalid(p, ra->new_role)) > + !policydb_role_isvalid(p, ra->new_role)) { > + rc = -EINVAL; > goto bad; > + } > + > lra = ra; > } > > @@ -2454,11 +2463,12 @@ int policydb_read(struct policydb *p, void *fp) > if (rc) > goto bad; > > - rc = -EINVAL; > p->process_trans_perms = string_to_av_perm(p, p->process_class, "transition"); > p->process_trans_perms |= string_to_av_perm(p, p->process_class, "dyntransition"); > - if (!p->process_trans_perms) > + if (!p->process_trans_perms) { > + rc = -EINVAL; > goto bad; > + } > > rc = ocontext_read(p, info, fp); > if (rc) > @@ -2472,12 +2482,13 @@ int policydb_read(struct policydb *p, void *fp) > if (rc) > goto bad; > > - rc = -ENOMEM; > p->type_attr_map_array = flex_array_alloc(sizeof(struct ebitmap), > p->p_types.nprim, > GFP_KERNEL | __GFP_ZERO); > - if (!p->type_attr_map_array) > + if (!p->type_attr_map_array) { > + rc = -ENOMEM; > goto bad; > + } > > /* preallocate so we don't have to worry about the put ever failing */ > rc = flex_array_prealloc(p->type_attr_map_array, 0, p->p_types.nprim, > -- > 2.11.0 > -- paul moore www.paul-moore.com From mboxrd@z Thu Jan 1 00:00:00 1970 From: paul@paul-moore.com (Paul Moore) Date: Thu, 23 Mar 2017 17:44:07 -0400 Subject: [PATCH 10/46] selinux: Move some assignments for the variable "rc" in policydb_read() In-Reply-To: <7fdcfc8f-affa-fcf8-adaf-dc8fd9e1b472@users.sourceforge.net> References: <7fdcfc8f-affa-fcf8-adaf-dc8fd9e1b472@users.sourceforge.net> Message-ID: To: linux-security-module@vger.kernel.org List-Id: linux-security-module.vger.kernel.org On Sun, Jan 15, 2017 at 10:10 AM, SF Markus Elfring wrote: > From: Markus Elfring > Date: Sat, 14 Jan 2017 15:22:29 +0100 > > One local variable was set to an error code in some cases before > a concrete error situation was detected. Thus move the corresponding > assignments into if branches to indicate a software failure there. > > Signed-off-by: Markus Elfring > --- > security/selinux/ss/policydb.c | 59 +++++++++++++++++++++++++----------------- > 1 file changed, 35 insertions(+), 24 deletions(-) More code churn with no real advantage. I agree with the style you are using, and would support changing it if you are in the function fixing bugs or doing other substantial changes in that code, but I can't justify it as a standalone change, sorry. > diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c > index 53e6d06e772a..506b0228d1f1 100644 > --- a/security/selinux/ss/policydb.c > +++ b/security/selinux/ss/policydb.c > @@ -2250,15 +2250,14 @@ int policydb_read(struct policydb *p, void *fp) > if (rc) > goto bad; > > - rc = -EINVAL; > if (le32_to_cpu(buf[0]) != POLICYDB_MAGIC) { > printk(KERN_ERR "SELinux: policydb magic number 0x%x does " > "not match expected magic number 0x%x\n", > le32_to_cpu(buf[0]), POLICYDB_MAGIC); > + rc = -EINVAL; > goto bad; > } > > - rc = -EINVAL; > len = le32_to_cpu(buf[1]); > if (len != strlen(POLICYDB_STRING)) { > printk(KERN_ERR "SELinux: policydb string length %d does not " > @@ -2265,11 +2265,13 @@ int policydb_read(struct policydb *p, void *fp) > len, strlen(POLICYDB_STRING)); > + rc = -EINVAL; > goto bad; > } > > - rc = -ENOMEM; > policydb_str = kmalloc(len + 1, GFP_KERNEL); > - if (!policydb_str) > + if (!policydb_str) { > + rc = -ENOMEM; > goto bad; > + } > > rc = next_entry(policydb_str, fp, len); > if (rc) { > @@ -2279,12 +2280,12 @@ int policydb_read(struct policydb *p, void *fp) > goto bad; > } > > - rc = -EINVAL; > policydb_str[len] = '\0'; > if (strcmp(policydb_str, POLICYDB_STRING)) { > printk(KERN_ERR "SELinux: policydb string %s does not match " > "my string %s\n", policydb_str, POLICYDB_STRING); > kfree(policydb_str); > + rc = -EINVAL; > goto bad; > } > /* Done with policydb_str. */ > @@ -2296,24 +2297,24 @@ int policydb_read(struct policydb *p, void *fp) > if (rc) > goto bad; > > - rc = -EINVAL; > p->policyvers = le32_to_cpu(buf[0]); > if (p->policyvers < POLICYDB_VERSION_MIN || > p->policyvers > POLICYDB_VERSION_MAX) { > printk(KERN_ERR "SELinux: policydb version %d does not match " > "my version range %d-%d\n", > le32_to_cpu(buf[0]), POLICYDB_VERSION_MIN, POLICYDB_VERSION_MAX); > + rc = -EINVAL; > goto bad; > } > > if ((le32_to_cpu(buf[1]) & POLICYDB_CONFIG_MLS)) { > p->mls_enabled = 1; > > - rc = -EINVAL; > if (p->policyvers < POLICYDB_VERSION_MLS) { > printk(KERN_ERR "SELinux: security policydb version %d " > "(MLS) not backwards compatible\n", > p->policyvers); > + rc = -EINVAL; > goto bad; > } > } > @@ -2332,21 +2333,21 @@ int policydb_read(struct policydb *p, void *fp) > goto bad; > } > > - rc = -EINVAL; > info = policydb_lookup_compat(p->policyvers); > if (!info) { > printk(KERN_ERR "SELinux: unable to find policy compat info " > "for version %d\n", p->policyvers); > + rc = -EINVAL; > goto bad; > } > > - rc = -EINVAL; > if (le32_to_cpu(buf[2]) != info->sym_num || > le32_to_cpu(buf[3]) != info->ocon_num) { > printk(KERN_ERR "SELinux: policydb table sizes (%d,%d) do " > "not match mine (%d,%d)\n", le32_to_cpu(buf[2]), > le32_to_cpu(buf[3]), > info->sym_num, info->ocon_num); > + rc = -EINVAL; > goto bad; > } > > @@ -2365,10 +2366,11 @@ int policydb_read(struct policydb *p, void *fp) > p->symtab[i].nprim = nprim; > } > > - rc = -EINVAL; > p->process_class = string_to_security_class(p, "process"); > - if (!p->process_class) > + if (!p->process_class) { > + rc = -EINVAL; > goto bad; > + } > > rc = avtab_read(&p->te_avtab, fp, p); > if (rc) > @@ -2386,10 +2388,12 @@ int policydb_read(struct policydb *p, void *fp) > nel = le32_to_cpu(buf[0]); > ltr = NULL; > for (i = 0; i < nel; i++) { > - rc = -ENOMEM; > tr = kzalloc(sizeof(*tr), GFP_KERNEL); > - if (!tr) > + if (!tr) { > + rc = -ENOMEM; > goto bad; > + } > + > if (ltr) > ltr->next = tr; > else > @@ -2398,7 +2402,6 @@ int policydb_read(struct policydb *p, void *fp) > if (rc) > goto bad; > > - rc = -EINVAL; > tr->role = le32_to_cpu(buf[0]); > tr->type = le32_to_cpu(buf[1]); > tr->new_role = le32_to_cpu(buf[2]); > @@ -2410,12 +2413,14 @@ int policydb_read(struct policydb *p, void *fp) > } else > tr->tclass = p->process_class; > > - rc = -EINVAL; > if (!policydb_role_isvalid(p, tr->role) || > !policydb_type_isvalid(p, tr->type) || > !policydb_class_isvalid(p, tr->tclass) || > - !policydb_role_isvalid(p, tr->new_role)) > + !policydb_role_isvalid(p, tr->new_role)) { > + rc = -EINVAL; > goto bad; > + } > + > ltr = tr; > } > > @@ -2425,10 +2430,12 @@ int policydb_read(struct policydb *p, void *fp) > nel = le32_to_cpu(buf[0]); > lra = NULL; > for (i = 0; i < nel; i++) { > - rc = -ENOMEM; > ra = kzalloc(sizeof(*ra), GFP_KERNEL); > - if (!ra) > + if (!ra) { > + rc = -ENOMEM; > goto bad; > + } > + > if (lra) > lra->next = ra; > else > @@ -2437,12 +2444,14 @@ int policydb_read(struct policydb *p, void *fp) > if (rc) > goto bad; > > - rc = -EINVAL; > ra->role = le32_to_cpu(buf[0]); > ra->new_role = le32_to_cpu(buf[1]); > if (!policydb_role_isvalid(p, ra->role) || > - !policydb_role_isvalid(p, ra->new_role)) > + !policydb_role_isvalid(p, ra->new_role)) { > + rc = -EINVAL; > goto bad; > + } > + > lra = ra; > } > > @@ -2454,11 +2463,12 @@ int policydb_read(struct policydb *p, void *fp) > if (rc) > goto bad; > > - rc = -EINVAL; > p->process_trans_perms = string_to_av_perm(p, p->process_class, "transition"); > p->process_trans_perms |= string_to_av_perm(p, p->process_class, "dyntransition"); > - if (!p->process_trans_perms) > + if (!p->process_trans_perms) { > + rc = -EINVAL; > goto bad; > + } > > rc = ocontext_read(p, info, fp); > if (rc) > @@ -2472,12 +2482,13 @@ int policydb_read(struct policydb *p, void *fp) > if (rc) > goto bad; > > - rc = -ENOMEM; > p->type_attr_map_array = flex_array_alloc(sizeof(struct ebitmap), > p->p_types.nprim, > GFP_KERNEL | __GFP_ZERO); > - if (!p->type_attr_map_array) > + if (!p->type_attr_map_array) { > + rc = -ENOMEM; > goto bad; > + } > > /* preallocate so we don't have to worry about the put ever failing */ > rc = flex_array_prealloc(p->type_attr_map_array, 0, p->p_types.nprim, > -- > 2.11.0 > -- paul moore www.paul-moore.com -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo at vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html