Re: [PATCH 02/12] selinux: Create policydb version for Infiniband support

[Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 5492 bytes --]

On Thu, Jun 30, 2016 at 06:01:42PM +0300, Yuval Shaia wrote:
> On Thu, Jun 23, 2016 at 10:52:48PM +0300, Dan Jurgens wrote:
> > From: Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > +				rc = next_entry(nodebuf, fp, sizeof(u32) * 6);
> > +				if (rc)
> > +					goto out;
> > +
> > +				c->u.pkey.subnet_prefix = be64_to_cpu(*((__be64 *)nodebuf));
> 
> More than 80 characters

Yes, we prefer code readability over checkpatch warning.

> 
> > +				/* The subnet prefix is stored as an IPv6
> > +				 * address in the policy.
> > +				 *
> > +				 * Check that the lower 2 DWORDS are 0.
> > +				 */
> > +				if (nodebuf[2] || nodebuf[3]) {
> > +					rc = -EINVAL;
> > +					goto out;
> > +				}
> > +
> > +				if (nodebuf[4] > 0xffff ||
> > +				    nodebuf[5] > 0xffff) {
> > +					rc = -EINVAL;
> > +					goto out;
> > +				}
> > +
> > +				c->u.pkey.low_pkey = le32_to_cpu(nodebuf[4]);
> > +				c->u.pkey.high_pkey = le32_to_cpu(nodebuf[5]);
> > +
> > +				rc = context_read_and_validate(&c->context[0],
> > +							       p,
> > +							       fp);
> > +				if (rc)
> > +					goto out;
> > +				break;
> > +			}
> > +			case OCON_IB_END_PORT:
> > +				rc = next_entry(buf, fp, sizeof(u32) * 2);
> > +				if (rc)
> > +					goto out;
> > +				len = le32_to_cpu(buf[0]);
> > +
> > +				rc = str_read(&c->u.ib_end_port.dev_name, GFP_KERNEL,
> 
> More than 80 characters?

I would recommend to place all in one line.

> 
> > +					      fp,
> > +					      len);
> > +				if (rc)
> > +					goto out;
> > +
> > +				c->u.ib_end_port.port = le32_to_cpu(buf[1]);
> > +
> > +				rc = context_read_and_validate(&c->context[0],
> > +							       p,
> > +							       fp);
> > +				if (rc)
> > +					goto out;
> > +				break;
> >  			}
> >  		}
> >  	}
> > @@ -3147,6 +3209,43 @@ static int ocontext_write(struct policydb *p, struct policydb_compat_info *info,
> 
> More than 80 characters

Agree

> 
> >  				if (rc)
> >  					return rc;
> >  				break;
> > +			case OCON_PKEY: {
> 
> Is "{" needed?

No, I agree, need to remove.

> 
> > +				__be64 *sbn_pfx = (__be64 *)nodebuf;
> > +				*sbn_pfx = cpu_to_be64(c->u.pkey.subnet_prefix);
> > +
> > +				/*
> > +				 * The low order 2 bits were confirmed to be 0
> > +				 * when the policy was loaded. Write them out
> > +				 * as zero
> > +				 */
> > +				nodebuf[2] = 0;
> > +				nodebuf[3] = 0;
> > +
> > +				nodebuf[4] = cpu_to_le32(c->u.pkey.low_pkey);
> > +				nodebuf[5] = cpu_to_le32(c->u.pkey.high_pkey);
> > +
> > +				rc = put_entry(nodebuf, sizeof(u32), 6, fp);
> > +				if (rc)
> > +					return rc;
> > +				rc = context_write(p, &c->context[0], fp);
> > +				if (rc)
> > +					return rc;
> > +				break;
> > +			}
> > +			case OCON_IB_END_PORT:
> > +				len = strlen(c->u.ib_end_port.dev_name);
> > +				buf[0] = cpu_to_le32(len);
> > +				buf[1] = cpu_to_le32(c->u.ib_end_port.port);
> > +				rc = put_entry(buf, sizeof(u32), 2, fp);
> > +				if (rc)
> > +					return rc;
> > +				rc = put_entry(c->u.ib_end_port.dev_name, 1, len, fp);
> 
> More than 80 characters?

Readability

> 
> > +				if (rc)
> > +					return rc;
> > +				rc = context_write(p, &c->context[0], fp);
> > +				if (rc)
> > +					return rc;
> > +				break;
> >  			}
> >  		}
> >  	}
> > diff --git a/security/selinux/ss/policydb.h b/security/selinux/ss/policydb.h
> > index 725d594..751bf604 100644
> > --- a/security/selinux/ss/policydb.h
> > +++ b/security/selinux/ss/policydb.h
> > @@ -187,6 +187,15 @@ struct ocontext {
> >  			u32 addr[4];
> >  			u32 mask[4];
> >  		} node6;        /* IPv6 node information */
> > +		struct {
> > +			u64 subnet_prefix;
> > +			u16 low_pkey;
> > +			u16 high_pkey;
> > +		} pkey;
> > +		struct {
> > +			char *dev_name;
> > +			u8 port;
> > +		} ib_end_port;
> >  	} u;
> >  	union {
> >  		u32 sclass;  /* security class for genfs */
> > @@ -215,14 +224,16 @@ struct genfs {
> >  #define SYM_NUM     8
> >  
> >  /* object context array indices */
> > -#define OCON_ISID  0	/* initial SIDs */
> > -#define OCON_FS    1	/* unlabeled file systems */
> > -#define OCON_PORT  2	/* TCP and UDP port numbers */
> > -#define OCON_NETIF 3	/* network interfaces */
> > -#define OCON_NODE  4	/* nodes */
> > -#define OCON_FSUSE 5	/* fs_use */
> > -#define OCON_NODE6 6	/* IPv6 nodes */
> > -#define OCON_NUM   7
> > +#define OCON_ISID	 0 /* initial SIDs */
> > +#define OCON_FS		 1 /* unlabeled file systems */
> > +#define OCON_PORT	 2 /* TCP and UDP port numbers */
> > +#define OCON_NETIF	 3 /* network interfaces */
> > +#define OCON_NODE	 4 /* nodes */
> > +#define OCON_FSUSE	 5 /* fs_use */
> > +#define OCON_NODE6	 6 /* IPv6 nodes */
> > +#define OCON_PKEY	 7 /* Infiniband PKeys */
> > +#define OCON_IB_END_PORT 8 /* Infiniband End ports */
> > +#define OCON_NUM	 9
> >  
> >  /* The policy database */
> >  struct policydb {
> > -- 
> > 1.8.3.1
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]