date:20181022

Re: [PATCH] libsepol: add missing ibendport port validity check

2018-10-22 Thread William Roberts

On Mon, Oct 22, 2018 at 1:18 AM Ondrej Mosnacek  wrote:
>
> The kernel checks if the port is in the range 1-255 when loading an
> ibenportcon rule. Add the same check to libsepol.
>
> Fixes: 118c0cd1038e ("libsepol: Add ibendport ocontext handling")
> Signed-off-by: Ondrej Mosnacek 
> ---
>  libsepol/src/policydb.c | 11 +--
>  1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c
> index db6765ba..e2808b2d 100644
> --- a/libsepol/src/policydb.c
> +++ b/libsepol/src/policydb.c
> @@ -2854,7 +2854,9 @@ static int ocontext_read_selinux(struct 
> policydb_compat_info *info,
> return -1;
> break;
> }
> -   case OCON_IBENDPORT:
> +   case OCON_IBENDPORT: {
> +   uint32_t port;
> +
> rc = next_entry(buf, fp, sizeof(uint32_t) * 
> 2);
> if (rc < 0)
> return -1;
> @@ -2862,6 +2864,10 @@ static int ocontext_read_selinux(struct 
> policydb_compat_info *info,
> if (len == 0 || len > IB_DEVICE_NAME_MAX - 1)
> return -1;
>
> +   port = le32_to_cpu(buf[1]);
> +   if (port > 0xff || port == 0)
> +   return -1;

You switched the other code to using UINT16_MAX, should probably use
UINT8_MAX here.

> +
> c->u.ibendport.dev_name = malloc(len + 1);
> if (!c->u.ibendport.dev_name)
> return -1;
> @@ -2869,11 +2875,12 @@ static int ocontext_read_selinux(struct 
> policydb_compat_info *info,
> if (rc < 0)
> return -1;
> c->u.ibendport.dev_name[len] = 0;
> -   c->u.ibendport.port = le32_to_cpu(buf[1]);
> +   c->u.ibendport.port = port;
> if (context_read_and_validate
> (>context[0], p, fp))
> return -1;
> break;
> +   }
> case OCON_PORT:
> rc = next_entry(buf, fp, sizeof(uint32_t) * 
> 3);
> if (rc < 0)
> --
> 2.17.2
>
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.

[PATCH v5] selinux: policydb - fix byte order and alignment issues

2018-10-22 Thread Ondrej Mosnacek

Do the LE conversions before doing the Infiniband-related range checks.
The incorrect checks are otherwise causing a failure to load any policy
with an ibendportcon rule on BE systems. This can be reproduced by
running (on e.g. ppc64):

cat >my_module.cil <
Cc: Eli Cohen 
Cc: James Morris 
Cc: Doug Ledford 
Cc:  # 4.13+
Fixes: a806f7a1616f ("selinux: Create policydb version for Infiniband support")
Signed-off-by: Ondrej Mosnacek 
---
 security/selinux/ss/policydb.c | 51 --
 1 file changed, 36 insertions(+), 15 deletions(-)

Changes in v5:
 - defer also assignment to 8-bit ibendport.port

Changes in v4:
 - defer assignment to 16-bit struct fields to after the range check

Changes in v3:
 - use separate buffer for the 64-bit subnet_prefix
 - add comments on the byte ordering of subnet_prefix
 - deduplicate the le32_to_cpu() calls from checks

Changes in v2:
 - add reproducer to commit message
 - update e-mail address of James Morris
 - better Cc also the old SELinux ML

diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
index f4eadd3f7350..d7e8126f32dc 100644
--- a/security/selinux/ss/policydb.c
+++ b/security/selinux/ss/policydb.c
@@ -2108,6 +2108,7 @@ static int ocontext_read(struct policydb *p, struct 
policydb_compat_info *info,
 {
int i, j, rc;
u32 nel, len;
+   __be64 prefixbuf[1];
__le32 buf[3];
struct ocontext *l, *c;
u32 nodebuf[8];
@@ -2217,21 +2218,30 @@ static int ocontext_read(struct policydb *p, struct 
policydb_compat_info *info,
goto out;
break;
}
-   case OCON_IBPKEY:
-   rc = next_entry(nodebuf, fp, sizeof(u32) * 4);
+   case OCON_IBPKEY: {
+   u32 pkey_lo, pkey_hi;
+
+   rc = next_entry(prefixbuf, fp, sizeof(u64));
+   if (rc)
+   goto out;
+
+   /* we need to have subnet_prefix in CPU order */
+   c->u.ibpkey.subnet_prefix = 
be64_to_cpu(prefixbuf[0]);
+
+   rc = next_entry(buf, fp, sizeof(u32) * 2);
if (rc)
goto out;
 
-   c->u.ibpkey.subnet_prefix = 
be64_to_cpu(*((__be64 *)nodebuf));
+   pkey_lo = le32_to_cpu(buf[0]);
+   pkey_hi = le32_to_cpu(buf[1]);
 
-   if (nodebuf[2] > 0x ||
-   nodebuf[3] > 0x) {
+   if (pkey_lo > U16_MAX || pkey_hi > U16_MAX) {
rc = -EINVAL;
goto out;
}
 
-   c->u.ibpkey.low_pkey = le32_to_cpu(nodebuf[2]);
-   c->u.ibpkey.high_pkey = le32_to_cpu(nodebuf[3]);
+   c->u.ibpkey.low_pkey  = pkey_lo;
+   c->u.ibpkey.high_pkey = pkey_hi;
 
rc = context_read_and_validate(>context[0],
   p,
@@ -2239,7 +2249,10 @@ static int ocontext_read(struct policydb *p, struct 
policydb_compat_info *info,
if (rc)
goto out;
break;
-   case OCON_IBENDPORT:
+   }
+   case OCON_IBENDPORT: {
+   u32 port;
+
rc = next_entry(buf, fp, sizeof(u32) * 2);
if (rc)
goto out;
@@ -2249,12 +2262,13 @@ static int ocontext_read(struct policydb *p, struct 
policydb_compat_info *info,
if (rc)
goto out;
 
-   if (buf[1] > 0xff || buf[1] == 0) {
+   port = le32_to_cpu(buf[1]);
+   if (port > 0xff || port == 0) {
rc = -EINVAL;
goto out;
}
 
-   c->u.ibendport.port = le32_to_cpu(buf[1]);
+   c->u.ibendport.port = port;
 
rc = context_read_and_validate(>context[0],
   p,
@@ -2262,7 +2276,8 @@ static int ocontext_read(struct policydb *p, struct 
policydb_compat_info *info,
if (rc)
goto out;

[PATCH] libsepol: add missing ibendport port validity check

2018-10-22 Thread Ondrej Mosnacek

The kernel checks if the port is in the range 1-255 when loading an
ibenportcon rule. Add the same check to libsepol.

Fixes: 118c0cd1038e ("libsepol: Add ibendport ocontext handling")
Signed-off-by: Ondrej Mosnacek 
---
 libsepol/src/policydb.c | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c
index db6765ba..e2808b2d 100644
--- a/libsepol/src/policydb.c
+++ b/libsepol/src/policydb.c
@@ -2854,7 +2854,9 @@ static int ocontext_read_selinux(struct 
policydb_compat_info *info,
return -1;
break;
}
-   case OCON_IBENDPORT:
+   case OCON_IBENDPORT: {
+   uint32_t port;
+
rc = next_entry(buf, fp, sizeof(uint32_t) * 2);
if (rc < 0)
return -1;
@@ -2862,6 +2864,10 @@ static int ocontext_read_selinux(struct 
policydb_compat_info *info,
if (len == 0 || len > IB_DEVICE_NAME_MAX - 1)
return -1;
 
+   port = le32_to_cpu(buf[1]);
+   if (port > 0xff || port == 0)
+   return -1;
+
c->u.ibendport.dev_name = malloc(len + 1);
if (!c->u.ibendport.dev_name)
return -1;
@@ -2869,11 +2875,12 @@ static int ocontext_read_selinux(struct 
policydb_compat_info *info,
if (rc < 0)
return -1;
c->u.ibendport.dev_name[len] = 0;
-   c->u.ibendport.port = le32_to_cpu(buf[1]);
+   c->u.ibendport.port = port;
if (context_read_and_validate
(>context[0], p, fp))
return -1;
break;
+   }
case OCON_PORT:
rc = next_entry(buf, fp, sizeof(uint32_t) * 3);
if (rc < 0)
-- 
2.17.2

___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.

Re: [PATCH v4] selinux: policydb - fix byte order and alignment issues

2018-10-22 Thread Ondrej Mosnacek

On Sat, Oct 20, 2018 at 3:05 AM William Roberts
 wrote:
> On Fri, Oct 19, 2018 at 7:28 AM Stephen Smalley  wrote:
> >
> > On 10/18/2018 03:47 AM, Ondrej Mosnacek wrote:
> > > Do the LE conversions before doing the Infiniband-related range checks.
> > > The incorrect checks are otherwise causing a failure to load any policy
> > > with an ibendportcon rule on BE systems. This can be reproduced by
> > > running (on e.g. ppc64):
> > >
> > > cat >my_module.cil < > > (type test_ibendport_t)
> > > (roletype object_r test_ibendport_t)
> > > (ibendportcon mlx4_0 1 (system_u object_r test_ibendport_t ((s0) (s0
> > > EOF
> > > semodule -i my_module.cil
> > >
> > > Also, fix loading/storing the 64-bit subnet prefix for OCON_IBPKEY to
> > > use a correctly aligned buffer.
> > >
> > > Finally, do not use the 'nodebuf' (u32) buffer where 'buf' (__le32)
> > > should be used instead.
> > >
> > > Tested internally on a ppc64 machine with a RHEL 7 kernel with this
> > > patch applied.
> > >
> > > Cc: Daniel Jurgens 
> > > Cc: Eli Cohen 
> > > Cc: James Morris 
> > > Cc: Doug Ledford 
> > > Cc:  # 4.13+
> > > Fixes: a806f7a1616f ("selinux: Create policydb version for Infiniband 
> > > support")
> > > Signed-off-by: Ondrej Mosnacek 
> > > ---
> > >   security/selinux/ss/policydb.c | 46 +++---
> > >   1 file changed, 32 insertions(+), 14 deletions(-)
> > >
> > > Changes in v4:
> > >   - defer assignment to 16-bit struct fields to after the range check
> > >
> > > Changes in v3:
> > >   - use separate buffer for the 64-bit subnet_prefix
> > >   - add comments on the byte ordering of subnet_prefix
> > >   - deduplicate the le32_to_cpu() calls from checks
> > >
> > > Changes in v2:
> > >   - add reproducer to commit message
> > >   - update e-mail address of James Morris
> > >   - better Cc also the old SELinux ML
> > >
> > > diff --git a/security/selinux/ss/policydb.c 
> > > b/security/selinux/ss/policydb.c
> > > index f4eadd3f7350..d50668006a52 100644
> > > --- a/security/selinux/ss/policydb.c
> > > +++ b/security/selinux/ss/policydb.c
> > > @@ -2108,6 +2108,7 @@ static int ocontext_read(struct policydb *p, struct 
> > > policydb_compat_info *info,
> > >   {
> > >   int i, j, rc;
> > >   u32 nel, len;
> > > + __be64 prefixbuf[1];
> > >   __le32 buf[3];
> > >   struct ocontext *l, *c;
> > >   u32 nodebuf[8];
> > > @@ -2217,21 +2218,30 @@ static int ocontext_read(struct policydb *p, 
> > > struct policydb_compat_info *info,
> > >   goto out;
> > >   break;
> > >   }
> > > - case OCON_IBPKEY:
> > > - rc = next_entry(nodebuf, fp, sizeof(u32) * 
> > > 4);
> > > + case OCON_IBPKEY: {
> > > + u32 pkey_lo, pkey_hi;
> > > +
> > > + rc = next_entry(prefixbuf, fp, sizeof(u64));
> > > + if (rc)
> > > + goto out;
> > > +
> > > + /* we need to have subnet_prefix in CPU 
> > > order */
> > > + c->u.ibpkey.subnet_prefix = 
> > > be64_to_cpu(prefixbuf[0]);
> > > +
> > > + rc = next_entry(buf, fp, sizeof(u32) * 2);
> > >   if (rc)
> > >   goto out;
> > >
> > > - c->u.ibpkey.subnet_prefix = 
> > > be64_to_cpu(*((__be64 *)nodebuf));
> > > + pkey_lo = le32_to_cpu(buf[0]);
> > > + pkey_hi = le32_to_cpu(buf[1]);
> > >
> > > - if (nodebuf[2] > 0x ||
> > > - nodebuf[3] > 0x) {
> > > + if (pkey_lo > U16_MAX || pkey_hi > U16_MAX) 
> > > {
> > >   rc = -EINVAL;
> > >   goto out;
> > >   }
> > >
> > > - c->u.ibpkey.low_pkey = 
> > > le32_to_cpu(nodebuf[2]);
> > > - c->u.ibpkey.high_pkey = 
> > > le32_to_cpu(nodebuf[3]);
> > > + c->u.ibpkey.low_pkey  = pkey_lo;
> > > + c->u.ibpkey.high_pkey = pkey_hi;
> > >
> > >   rc = 
> > > context_read_and_validate(>context[0],
> > >  p,
> > > @@ -2239,6 +2249,7 @@ static int ocontext_read(struct policydb *p, struct 
> > > policydb_compat_info *info,
> > >   if (rc)
> > >   goto out;
> > >   break;
> > > + }
> > >   case OCON_IBENDPORT:
> > >   rc = next_entry(buf, fp, sizeof(u32) * 2);
> > >   if

Re: [PATCH v4] selinux: policydb - fix byte order and alignment issues

2018-10-22 Thread Ondrej Mosnacek

On Fri, Oct 19, 2018 at 4:26 PM Stephen Smalley  wrote:
> On 10/18/2018 03:47 AM, Ondrej Mosnacek wrote:
> > Do the LE conversions before doing the Infiniband-related range checks.
> > The incorrect checks are otherwise causing a failure to load any policy
> > with an ibendportcon rule on BE systems. This can be reproduced by
> > running (on e.g. ppc64):
> >
> > cat >my_module.cil < > (type test_ibendport_t)
> > (roletype object_r test_ibendport_t)
> > (ibendportcon mlx4_0 1 (system_u object_r test_ibendport_t ((s0) (s0
> > EOF
> > semodule -i my_module.cil
> >
> > Also, fix loading/storing the 64-bit subnet prefix for OCON_IBPKEY to
> > use a correctly aligned buffer.
> >
> > Finally, do not use the 'nodebuf' (u32) buffer where 'buf' (__le32)
> > should be used instead.
> >
> > Tested internally on a ppc64 machine with a RHEL 7 kernel with this
> > patch applied.
> >
> > Cc: Daniel Jurgens 
> > Cc: Eli Cohen 
> > Cc: James Morris 
> > Cc: Doug Ledford 
> > Cc:  # 4.13+
> > Fixes: a806f7a1616f ("selinux: Create policydb version for Infiniband 
> > support")
> > Signed-off-by: Ondrej Mosnacek 
> > ---
> >   security/selinux/ss/policydb.c | 46 +++---
> >   1 file changed, 32 insertions(+), 14 deletions(-)
> >
> > Changes in v4:
> >   - defer assignment to 16-bit struct fields to after the range check
> >
> > Changes in v3:
> >   - use separate buffer for the 64-bit subnet_prefix
> >   - add comments on the byte ordering of subnet_prefix
> >   - deduplicate the le32_to_cpu() calls from checks
> >
> > Changes in v2:
> >   - add reproducer to commit message
> >   - update e-mail address of James Morris
> >   - better Cc also the old SELinux ML
> >
> > diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
> > index f4eadd3f7350..d50668006a52 100644
> > --- a/security/selinux/ss/policydb.c
> > +++ b/security/selinux/ss/policydb.c
> > @@ -2108,6 +2108,7 @@ static int ocontext_read(struct policydb *p, struct 
> > policydb_compat_info *info,
> >   {
> >   int i, j, rc;
> >   u32 nel, len;
> > + __be64 prefixbuf[1];
> >   __le32 buf[3];
> >   struct ocontext *l, *c;
> >   u32 nodebuf[8];
> > @@ -2217,21 +2218,30 @@ static int ocontext_read(struct policydb *p, struct 
> > policydb_compat_info *info,
> >   goto out;
> >   break;
> >   }
> > - case OCON_IBPKEY:
> > - rc = next_entry(nodebuf, fp, sizeof(u32) * 4);
> > + case OCON_IBPKEY: {
> > + u32 pkey_lo, pkey_hi;
> > +
> > + rc = next_entry(prefixbuf, fp, sizeof(u64));
> > + if (rc)
> > + goto out;
> > +
> > + /* we need to have subnet_prefix in CPU order 
> > */
> > + c->u.ibpkey.subnet_prefix = 
> > be64_to_cpu(prefixbuf[0]);
> > +
> > + rc = next_entry(buf, fp, sizeof(u32) * 2);
> >   if (rc)
> >   goto out;
> >
> > - c->u.ibpkey.subnet_prefix = 
> > be64_to_cpu(*((__be64 *)nodebuf));
> > + pkey_lo = le32_to_cpu(buf[0]);
> > + pkey_hi = le32_to_cpu(buf[1]);
> >
> > - if (nodebuf[2] > 0x ||
> > - nodebuf[3] > 0x) {
> > + if (pkey_lo > U16_MAX || pkey_hi > U16_MAX) {
> >   rc = -EINVAL;
> >   goto out;
> >   }
> >
> > - c->u.ibpkey.low_pkey = 
> > le32_to_cpu(nodebuf[2]);
> > - c->u.ibpkey.high_pkey = 
> > le32_to_cpu(nodebuf[3]);
> > + c->u.ibpkey.low_pkey  = pkey_lo;
> > + c->u.ibpkey.high_pkey = pkey_hi;
> >
> >   rc = context_read_and_validate(>context[0],
> >  p,
> > @@ -2239,6 +2249,7 @@ static int ocontext_read(struct policydb *p, struct 
> > policydb_compat_info *info,
> >   if (rc)
> >   goto out;
> >   break;
> > + }
> >   case OCON_IBENDPORT:
> >   rc = next_entry(buf, fp, sizeof(u32) * 2);
> >   if (rc)
> > @@ -2249,13 +2260,14 @@ static int ocontext_read(struct policydb *p, struct 
> > policydb_compat_info *info,
> >   if (rc)
> >   goto out;
> >
> > - if (buf[1] > 0xff || buf[1] == 0) {
> > +

Re: [PATCH] libsepol: add missing ibendport port validity check

[PATCH v5] selinux: policydb - fix byte order and alignment issues

[PATCH] libsepol: add missing ibendport port validity check

Re: [PATCH v4] selinux: policydb - fix byte order and alignment issues

Re: [PATCH v4] selinux: policydb - fix byte order and alignment issues

5 matches

Site Navigation

Mail list logo

Footer information