2022-08-30 16:20:15

by Christian Göttsche

[permalink] [raw]
Subject: [PATCH] selinux: declare read-only parameters const

Declare ebitmap, mls_level and mls_context parameters const where they
are only read from. This allows callers to supply pointers to const
as arguments and increases readability.

Signed-off-by: Christian Göttsche <[email protected]>
---
This patch is similar to userspace commits ("libsepol: mark read-only
parameters of ebitmap interfaces const") [1] and ("libsepol: mark
immutable mls and context parameter const") [2].
Pointers to const are for example used in the (for now userspace only)
policy validation code.

[1]: https://github.com/SELinuxProject/selinux/commit/390ec54d278a14d9c29b80cc4fc5cb9ba504ed13
[2]: https://github.com/SELinuxProject/selinux/commit/6359946633507db1d98b24638cf22eb5afdcd272
---
security/selinux/ss/context.h | 17 +++++++++--------
security/selinux/ss/ebitmap.c | 21 +++++++++++----------
security/selinux/ss/ebitmap.h | 18 +++++++++---------
security/selinux/ss/mls_types.h | 4 ++--
4 files changed, 31 insertions(+), 29 deletions(-)

diff --git a/security/selinux/ss/context.h b/security/selinux/ss/context.h
index 62990aa1ec9e..eda32c3d4c0a 100644
--- a/security/selinux/ss/context.h
+++ b/security/selinux/ss/context.h
@@ -38,7 +38,7 @@ static inline void mls_context_init(struct context *c)
memset(&c->range, 0, sizeof(c->range));
}

-static inline int mls_context_cpy(struct context *dst, struct context *src)
+static inline int mls_context_cpy(struct context *dst, const struct context *src)
{
int rc;

@@ -58,7 +58,7 @@ static inline int mls_context_cpy(struct context *dst, struct context *src)
/*
* Sets both levels in the MLS range of 'dst' to the low level of 'src'.
*/
-static inline int mls_context_cpy_low(struct context *dst, struct context *src)
+static inline int mls_context_cpy_low(struct context *dst, const struct context *src)
{
int rc;

@@ -78,7 +78,7 @@ static inline int mls_context_cpy_low(struct context *dst, struct context *src)
/*
* Sets both levels in the MLS range of 'dst' to the high level of 'src'.
*/
-static inline int mls_context_cpy_high(struct context *dst, struct context *src)
+static inline int mls_context_cpy_high(struct context *dst, const struct context *src)
{
int rc;

@@ -97,9 +97,10 @@ static inline int mls_context_cpy_high(struct context *dst, struct context *src)


static inline int mls_context_glblub(struct context *dst,
- struct context *c1, struct context *c2)
+ const struct context *c1, const struct context *c2)
{
- struct mls_range *dr = &dst->range, *r1 = &c1->range, *r2 = &c2->range;
+ struct mls_range *dr = &dst->range;
+ const struct mls_range *r1 = &c1->range, *r2 = &c2->range;
int rc = 0;

if (r1->level[1].sens < r2->level[0].sens ||
@@ -127,7 +128,7 @@ static inline int mls_context_glblub(struct context *dst,
return rc;
}

-static inline int mls_context_cmp(struct context *c1, struct context *c2)
+static inline int mls_context_cmp(const struct context *c1, const struct context *c2)
{
return ((c1->range.level[0].sens == c2->range.level[0].sens) &&
ebitmap_cmp(&c1->range.level[0].cat, &c2->range.level[0].cat) &&
@@ -147,7 +148,7 @@ static inline void context_init(struct context *c)
memset(c, 0, sizeof(*c));
}

-static inline int context_cpy(struct context *dst, struct context *src)
+static inline int context_cpy(struct context *dst, const struct context *src)
{
int rc;

@@ -180,7 +181,7 @@ static inline void context_destroy(struct context *c)
mls_context_destroy(c);
}

-static inline int context_cmp(struct context *c1, struct context *c2)
+static inline int context_cmp(const struct context *c1, const struct context *c2)
{
if (c1->len && c2->len)
return (c1->len == c2->len && !strcmp(c1->str, c2->str));
diff --git a/security/selinux/ss/ebitmap.c b/security/selinux/ss/ebitmap.c
index abde349c8321..d31b87be9a1e 100644
--- a/security/selinux/ss/ebitmap.c
+++ b/security/selinux/ss/ebitmap.c
@@ -28,9 +28,9 @@

static struct kmem_cache *ebitmap_node_cachep __ro_after_init;

-int ebitmap_cmp(struct ebitmap *e1, struct ebitmap *e2)
+int ebitmap_cmp(const struct ebitmap *e1, const struct ebitmap *e2)
{
- struct ebitmap_node *n1, *n2;
+ const struct ebitmap_node *n1, *n2;

if (e1->highbit != e2->highbit)
return 0;
@@ -50,9 +50,10 @@ int ebitmap_cmp(struct ebitmap *e1, struct ebitmap *e2)
return 1;
}

-int ebitmap_cpy(struct ebitmap *dst, struct ebitmap *src)
+int ebitmap_cpy(struct ebitmap *dst, const struct ebitmap *src)
{
- struct ebitmap_node *n, *new, *prev;
+ struct ebitmap_node *new, *prev;
+ const struct ebitmap_node *n;

ebitmap_init(dst);
n = src->node;
@@ -78,7 +79,7 @@ int ebitmap_cpy(struct ebitmap *dst, struct ebitmap *src)
return 0;
}

-int ebitmap_and(struct ebitmap *dst, struct ebitmap *e1, struct ebitmap *e2)
+int ebitmap_and(struct ebitmap *dst, const struct ebitmap *e1, const struct ebitmap *e2)
{
struct ebitmap_node *n;
int bit, rc;
@@ -217,9 +218,9 @@ int ebitmap_netlbl_import(struct ebitmap *ebmap,
* if last_e2bit is non-zero, the highest set bit in e2 cannot exceed
* last_e2bit.
*/
-int ebitmap_contains(struct ebitmap *e1, struct ebitmap *e2, u32 last_e2bit)
+int ebitmap_contains(const struct ebitmap *e1, const struct ebitmap *e2, u32 last_e2bit)
{
- struct ebitmap_node *n1, *n2;
+ const struct ebitmap_node *n1, *n2;
int i;

if (e1->highbit < e2->highbit)
@@ -258,9 +259,9 @@ int ebitmap_contains(struct ebitmap *e1, struct ebitmap *e2, u32 last_e2bit)
return 1;
}

-int ebitmap_get_bit(struct ebitmap *e, unsigned long bit)
+int ebitmap_get_bit(const struct ebitmap *e, unsigned long bit)
{
- struct ebitmap_node *n;
+ const struct ebitmap_node *n;

if (e->highbit < bit)
return 0;
@@ -467,7 +468,7 @@ int ebitmap_read(struct ebitmap *e, void *fp)
goto out;
}

-int ebitmap_write(struct ebitmap *e, void *fp)
+int ebitmap_write(const struct ebitmap *e, void *fp)
{
struct ebitmap_node *n;
u32 count;
diff --git a/security/selinux/ss/ebitmap.h b/security/selinux/ss/ebitmap.h
index 58eb822f11ee..e5b57dc3fc53 100644
--- a/security/selinux/ss/ebitmap.h
+++ b/security/selinux/ss/ebitmap.h
@@ -44,7 +44,7 @@ struct ebitmap {

#define ebitmap_length(e) ((e)->highbit)

-static inline unsigned int ebitmap_start_positive(struct ebitmap *e,
+static inline unsigned int ebitmap_start_positive(const struct ebitmap *e,
struct ebitmap_node **n)
{
unsigned int ofs;
@@ -62,7 +62,7 @@ static inline void ebitmap_init(struct ebitmap *e)
memset(e, 0, sizeof(*e));
}

-static inline unsigned int ebitmap_next_positive(struct ebitmap *e,
+static inline unsigned int ebitmap_next_positive(const struct ebitmap *e,
struct ebitmap_node **n,
unsigned int bit)
{
@@ -85,7 +85,7 @@ static inline unsigned int ebitmap_next_positive(struct ebitmap *e,
#define EBITMAP_NODE_OFFSET(node, bit) \
(((bit) - (node)->startbit) % EBITMAP_UNIT_SIZE)

-static inline int ebitmap_node_get_bit(struct ebitmap_node *n,
+static inline int ebitmap_node_get_bit(const struct ebitmap_node *n,
unsigned int bit)
{
unsigned int index = EBITMAP_NODE_INDEX(n, bit);
@@ -122,15 +122,15 @@ static inline void ebitmap_node_clr_bit(struct ebitmap_node *n,
(bit) < ebitmap_length(e); \
(bit) = ebitmap_next_positive(e, &(n), bit)) \

-int ebitmap_cmp(struct ebitmap *e1, struct ebitmap *e2);
-int ebitmap_cpy(struct ebitmap *dst, struct ebitmap *src);
-int ebitmap_and(struct ebitmap *dst, struct ebitmap *e1, struct ebitmap *e2);
-int ebitmap_contains(struct ebitmap *e1, struct ebitmap *e2, u32 last_e2bit);
-int ebitmap_get_bit(struct ebitmap *e, unsigned long bit);
+int ebitmap_cmp(const struct ebitmap *e1, const struct ebitmap *e2);
+int ebitmap_cpy(struct ebitmap *dst, const struct ebitmap *src);
+int ebitmap_and(struct ebitmap *dst, const struct ebitmap *e1, const struct ebitmap *e2);
+int ebitmap_contains(const struct ebitmap *e1, const struct ebitmap *e2, u32 last_e2bit);
+int ebitmap_get_bit(const struct ebitmap *e, unsigned long bit);
int ebitmap_set_bit(struct ebitmap *e, unsigned long bit, int value);
void ebitmap_destroy(struct ebitmap *e);
int ebitmap_read(struct ebitmap *e, void *fp);
-int ebitmap_write(struct ebitmap *e, void *fp);
+int ebitmap_write(const struct ebitmap *e, void *fp);
u32 ebitmap_hash(const struct ebitmap *e, u32 hash);

#ifdef CONFIG_NETLABEL
diff --git a/security/selinux/ss/mls_types.h b/security/selinux/ss/mls_types.h
index 068e0d7809db..7d48d5e52233 100644
--- a/security/selinux/ss/mls_types.h
+++ b/security/selinux/ss/mls_types.h
@@ -27,13 +27,13 @@ struct mls_range {
struct mls_level level[2]; /* low == level[0], high == level[1] */
};

-static inline int mls_level_eq(struct mls_level *l1, struct mls_level *l2)
+static inline int mls_level_eq(const struct mls_level *l1, const struct mls_level *l2)
{
return ((l1->sens == l2->sens) &&
ebitmap_cmp(&l1->cat, &l2->cat));
}

-static inline int mls_level_dom(struct mls_level *l1, struct mls_level *l2)
+static inline int mls_level_dom(const struct mls_level *l1, const struct mls_level *l2)
{
return ((l1->sens >= l2->sens) &&
ebitmap_contains(&l1->cat, &l2->cat, 0));
--
2.37.2


2022-08-30 21:19:25

by Paul Moore

[permalink] [raw]
Subject: Re: [PATCH] selinux: declare read-only parameters const

On Tue, Aug 30, 2022 at 11:53 AM Christian Göttsche
<[email protected]> wrote:
>
> Declare ebitmap, mls_level and mls_context parameters const where they
> are only read from. This allows callers to supply pointers to const
> as arguments and increases readability.
>
> Signed-off-by: Christian Göttsche <[email protected]>
> ---
> This patch is similar to userspace commits ("libsepol: mark read-only
> parameters of ebitmap interfaces const") [1] and ("libsepol: mark
> immutable mls and context parameter const") [2].
> Pointers to const are for example used in the (for now userspace only)
> policy validation code.
>
> [1]: https://github.com/SELinuxProject/selinux/commit/390ec54d278a14d9c29b80cc4fc5cb9ba504ed13
> [2]: https://github.com/SELinuxProject/selinux/commit/6359946633507db1d98b24638cf22eb5afdcd272
> ---
> security/selinux/ss/context.h | 17 +++++++++--------
> security/selinux/ss/ebitmap.c | 21 +++++++++++----------
> security/selinux/ss/ebitmap.h | 18 +++++++++---------
> security/selinux/ss/mls_types.h | 4 ++--
> 4 files changed, 31 insertions(+), 29 deletions(-)

Merged into selinux/next, thanks!

--
paul-moore.com