Re: [PATCH v2] libsepol: fix endianity in ibpkey range checks
On Thu, Oct 18, 2018 at 12:50 AM Ondrej Mosnacek wrote:
>
> We need to convert from little-endian before dong range checks on the
> ibpkey port numbers, otherwise we would be checking a wrong value on
> big-endian systems.
>
> Fixes: 9fbb3112769a ("libsepol: Add ibpkey ocontext handling")
> Signed-off-by: Ondrej Mosnacek
> ---
> libsepol/src/policydb.c | 21 -
> 1 file changed, 16 insertions(+), 5 deletions(-)
>
> Changes in v2:
> - defer assignment to 16-bit struct fields to after the range check
>
> diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c
> index a6d76ca3..db6765ba 100644
> --- a/libsepol/src/policydb.c
> +++ b/libsepol/src/policydb.c
> @@ -2828,21 +2828,32 @@ static int ocontext_read_selinux(struct
> policydb_compat_info *info,
> (>context[1], p, fp))
> return -1;
> break;
> - case OCON_IBPKEY:
> + case OCON_IBPKEY: {
> + uint32_t pkey_lo, pkey_hi;
> +
> rc = next_entry(buf, fp, sizeof(uint32_t) *
> 4);
> - if (rc < 0 || buf[2] > 0x || buf[3] >
> 0x)
> + if (rc < 0)
> + return -1;
> +
> + pkey_lo = le32_to_cpu(buf[2]);
> + pkey_hi = le32_to_cpu(buf[3]);
> +
> + if (pkey_lo > UINT16_MAX || pkey_hi >
> UINT16_MAX)
> return -1;
>
> + c->u.ibpkey.low_pkey = pkey_lo;
> + c->u.ibpkey.high_pkey = pkey_hi;
> +
> + /* we want c->u.ibpkey.subnet_prefix in
> network
> +* (big-endian) order, just memcpy it */
> memcpy(>u.ibpkey.subnet_prefix, buf,
>sizeof(c->u.ibpkey.subnet_prefix));
>
> - c->u.ibpkey.low_pkey = le32_to_cpu(buf[2]);
> - c->u.ibpkey.high_pkey = le32_to_cpu(buf[3]);
> -
> if (context_read_and_validate
> (>context[0], p, fp))
> return -1;
> break;
> + }
> case OCON_IBENDPORT:
> rc = next_entry(buf, fp, sizeof(uint32_t) *
> 2);
> if (rc < 0)
> --
> 2.17.2
>
Acked-by: William Roberts william.c.robe...@intel.com
___
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
On Thu, Oct 18, 2018 at 5:57 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) {
> + c->u.ibendport.port = le32_to_cpu(buf[1]);
> +
> + if (c->u.ibendport.port > 0xff ||
> + c->u.ibendport.port == 0) {
>
[PATCH v4] selinux: policydb - fix byte order and alignment issues
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 | 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) {
+ c->u.ibendport.port = le32_to_cpu(buf[1]);
+
+ if (c->u.ibendport.port > 0xff ||
+ c->u.ibendport.port == 0) {
rc = -EINVAL;
goto out;
}
- c->u.ibendport.port = le32_to_cpu(buf[1]);
-
rc = context_read_and_validate(>context[0],
p,
fp);
@@ -3105,6 +3117,7 @@ static int ocontext_write(struct policydb *p, struct
policydb_compat_info *info,
{
unsigned int i, j, rc;
size_t nel, len;
+ __be64 prefixbuf[1];
__le32 buf[3];
u32 nodebuf[8];
struct ocontext *c;
@@ -3192,12 +3205,17 @@ static int ocontext_write(struct policydb *p,
Re: Fix alias handling in sepolicy and semaange
Please ignore this patch set. I sent a new version (including sign-off and proper "Resolves") to vger.kernel.org. Sorry for the noise. On 16. 10. 18 10:25, Vit Mojzis wrote: Sepolicy and semanage do not work with aliases properly (aliases are mostly treated as invalid types). Fix this by determining corresponding type when an alias is used and working with the type instead. python/semanage/seobject.py | 21 ++--- python/sepolicy/sepolicy.py | 8 +++- python/sepolicy/sepolicy/__init__.py | 22 ++ 3 files changed, 31 insertions(+), 20 deletions(-) ___ 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. ___ 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 v2] libsepol: fix endianity in ibpkey range checks
We need to convert from little-endian before dong range checks on the
ibpkey port numbers, otherwise we would be checking a wrong value on
big-endian systems.
Fixes: 9fbb3112769a ("libsepol: Add ibpkey ocontext handling")
Signed-off-by: Ondrej Mosnacek
---
libsepol/src/policydb.c | 21 -
1 file changed, 16 insertions(+), 5 deletions(-)
Changes in v2:
- defer assignment to 16-bit struct fields to after the range check
diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c
index a6d76ca3..db6765ba 100644
--- a/libsepol/src/policydb.c
+++ b/libsepol/src/policydb.c
@@ -2828,21 +2828,32 @@ static int ocontext_read_selinux(struct
policydb_compat_info *info,
(>context[1], p, fp))
return -1;
break;
- case OCON_IBPKEY:
+ case OCON_IBPKEY: {
+ uint32_t pkey_lo, pkey_hi;
+
rc = next_entry(buf, fp, sizeof(uint32_t) * 4);
- if (rc < 0 || buf[2] > 0x || buf[3] >
0x)
+ if (rc < 0)
+ return -1;
+
+ pkey_lo = le32_to_cpu(buf[2]);
+ pkey_hi = le32_to_cpu(buf[3]);
+
+ if (pkey_lo > UINT16_MAX || pkey_hi >
UINT16_MAX)
return -1;
+ c->u.ibpkey.low_pkey = pkey_lo;
+ c->u.ibpkey.high_pkey = pkey_hi;
+
+ /* we want c->u.ibpkey.subnet_prefix in network
+* (big-endian) order, just memcpy it */
memcpy(>u.ibpkey.subnet_prefix, buf,
sizeof(c->u.ibpkey.subnet_prefix));
- c->u.ibpkey.low_pkey = le32_to_cpu(buf[2]);
- c->u.ibpkey.high_pkey = le32_to_cpu(buf[3]);
-
if (context_read_and_validate
(>context[0], p, fp))
return -1;
break;
+ }
case OCON_IBENDPORT:
rc = next_entry(buf, fp, sizeof(uint32_t) * 2);
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] libsepol: fix endianity in ibpkey range checks
On Wed, Oct 17, 2018 at 6:07 PM William Roberts
wrote:
> On Wed, Oct 17, 2018 at 7:48 AM Ondrej Mosnacek wrote:
> >
> > We need to convert from little-endian before dong range checks on the
> > ibpkey port numbers, otherwise we would be checking a wrong value.
> >
> > Fixes: 9fbb3112769a ("libsepol: Add ibpkey ocontext handling")
> > Signed-off-by: Ondrej Mosnacek
> > ---
> > libsepol/src/policydb.c | 14 ++
> > 1 file changed, 10 insertions(+), 4 deletions(-)
> >
> > diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c
> > index a6d76ca3..dc201e2f 100644
> > --- a/libsepol/src/policydb.c
> > +++ b/libsepol/src/policydb.c
> > @@ -2830,15 +2830,21 @@ static int ocontext_read_selinux(struct
> > policydb_compat_info *info,
> > break;
> > case OCON_IBPKEY:
> > rc = next_entry(buf, fp, sizeof(uint32_t) *
> > 4);
> > - if (rc < 0 || buf[2] > 0x || buf[3] >
> > 0x)
> > + if (rc < 0)
> > return -1;
> >
> > + c->u.ibpkey.low_pkey = le32_to_cpu(buf[2]);
> > + c->u.ibpkey.high_pkey = le32_to_cpu(buf[3]);
>
> Ahh you're assigning a 32 bit value to a 16 bit variable low|high_pkey. I
> think
> you need to assign them to a uint32_t if you want to actually range check
> them.
Oh right, I didn't realize those fields are 16-bit... Let me fix it and re-spin.
>
> > +
> > + if (c->u.ibpkey.low_pkey > 0x ||
> > + c->u.ibpkey.high_pkey > 0x)
> > + return -1;
> > +
> > + /* we want c->u.ibpkey.subnet_prefix in
> > network
> > +* (big-endian) order, just memcpy it */
> > memcpy(>u.ibpkey.subnet_prefix, buf,
> >sizeof(c->u.ibpkey.subnet_prefix));
> >
> > - c->u.ibpkey.low_pkey = le32_to_cpu(buf[2]);
> > - c->u.ibpkey.high_pkey = le32_to_cpu(buf[3]);
> > -
> > if (context_read_and_validate
> > (>context[0], p, fp))
> > return -1;
> > --
> > 2.17.2
> >
> See job: https://travis-ci.org/SELinuxProject/selinux/jobs/442750208
>
> Build fail with gcc:
>
> policydb.c:2839:31: error: comparison is always false due to limited
> range of data type [-Werror=type-limits]
> if (c->u.ibpkey.low_pkey > 0x ||
>^
> policydb.c:2840:31: error: comparison is always false due to limited
> range of data type [-Werror=type-limits]
> c->u.ibpkey.high_pkey > 0x)
--
Ondrej Mosnacek
Associate Software Engineer, Security Technologies
Red Hat, Inc.
___
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.
