Skip to contentSkip to conversation

security/selinux/ss/policydb.c: fix sparse warnings

PATCH In review [email protected] 4 participants 5 messages Mar 19, 2005 – Mar 22, 2005 SELinux security module

lore
Share & tools
lore

[email protected]

author
#0 lore raw

PATCH 1/4 security/selinux/ss/policydb.c: fix sparse warnings

Signed-off-by Domen Puncer <[email protected]>

---

 kj-domen/security/selinux/ss/policydb.c |   35 ++++++++++++++++++--------------
 1 files changed, 20 insertions(+), 15 deletions(-)
security/selinux/ss/policydb.c renamed from security/selinux/ss/policydb.c~sparse-security_selinux_ss_policydb +20 -15
diff -puN security/selinux/ss/policydb.c~sparse-security_selinux_ss_policydb security/selinux/ss/policydb.c
@@ -780,7 +780,7 @@ static int context_read_and_validate(str
 				     struct policydb *p,
 				     void *fp)
 {
-	u32 buf[3];
+	__le32 buf[3];
 	int rc;
 
 	rc = next_entry(buf, fp, sizeof buf);

@@ -820,7 +820,8 @@ static int perm_read(struct policydb *p,
 	char *key = NULL;
 	struct perm_datum *perdatum;
 	int rc;
-	u32 buf[2], len;
+	__le32 buf[2];
+	u32 len;
 
 	perdatum = kmalloc(sizeof(*perdatum), GFP_KERNEL);
 	if (!perdatum) {

@@ -860,7 +861,8 @@ static int common_read(struct policydb *
 {
 	char *key = NULL;
 	struct common_datum *comdatum;
-	u32 buf[4], len, nel;
+	__le32 buf[4];
+	u32 len, nel;
 	int i, rc;
 
 	comdatum = kmalloc(sizeof(*comdatum), GFP_KERNEL);

@@ -1092,7 +1094,8 @@ static int role_read(struct policydb *p,
 	char *key = NULL;
 	struct role_datum *role;
 	int rc;
-	u32 buf[2], len;
+	__le32 buf[2];
+	u32 len;
 
 	role = kmalloc(sizeof(*role), GFP_KERNEL);
 	if (!role) {

@@ -1152,7 +1155,8 @@ static int type_read(struct policydb *p,
 	char *key = NULL;
 	struct type_datum *typdatum;
 	int rc;
-	u32 buf[3], len;
+	__le32 buf[3];
+	u32 len;
 
 	typdatum = kmalloc(sizeof(*typdatum),GFP_KERNEL);
 	if (!typdatum) {

@@ -1224,7 +1228,8 @@ static int user_read(struct policydb *p,
 	char *key = NULL;
 	struct user_datum *usrdatum;
 	int rc;
-	u32 buf[2], len;
+	__le32 buf[2];
+	u32 len;
 
 	usrdatum = kmalloc(sizeof(*usrdatum), GFP_KERNEL);
 	if (!usrdatum) {

@@ -1392,7 +1397,8 @@ int policydb_read(struct policydb *p, vo
 	struct ocontext *l, *c, *newc;
 	struct genfs *genfs_p, *genfs, *newgenfs;
 	int i, j, rc;
-	u32 buf[8], len, len2, config, nprim, nel, nel2;
+	__le32 buf[8];
+	u32 len, len2, config, nprim, nel, nel2;
 	char *policydb_str;
 	struct policydb_compat_info *info;
 	struct range_trans *rt, *lrt;

@@ -1408,17 +1414,14 @@ int policydb_read(struct policydb *p, vo
 	if (rc < 0)
 		goto bad;
 
-	for (i = 0; i < 2; i++)
-		buf[i] = le32_to_cpu(buf[i]);
-
-	if (buf[0] != POLICYDB_MAGIC) {
+	if (buf[0] != cpu_to_le32(POLICYDB_MAGIC)) {
 		printk(KERN_ERR "security:  policydb magic number 0x%x does "
 		       "not match expected magic number 0x%x\n",
-		       buf[0], POLICYDB_MAGIC);
+		       le32_to_cpu(buf[0]), POLICYDB_MAGIC);
 		goto bad;
 	}
 
-	len = buf[1];
+	len = le32_to_cpu(buf[1]);
 	if (len != strlen(POLICYDB_STRING)) {
 		printk(KERN_ERR "security:  policydb string length %d does not "
 		       "match expected length %Zu\n",

@@ -1494,9 +1497,11 @@ int policydb_read(struct policydb *p, vo
 		goto bad;
 	}
 
-	if (buf[2] != info->sym_num || buf[3] != info->ocon_num) {
+	if (le32_to_cpu(buf[2]) != info->sym_num ||
+	    le32_to_cpu(buf[3]) != info->ocon_num) {
 		printk(KERN_ERR "security:  policydb table sizes (%d,%d) do "
-		       "not match mine (%d,%d)\n", buf[2], buf[3],
+		       "not match mine (%d,%d)\n",
+		       le32_to_cpu(buf[2]), le32_to_cpu(buf[3]),
 		       info->sym_num, info->ocon_num);
 		goto bad;
 	}

_

#1 lore raw
Quoted 2 lines · On Sat, 19 Mar 2005 [email protected] wrote:
 kj-domen/security/selinux/ss/policydb.c |   35 ++++++++++++++++++--------------
 1 files changed, 20 insertions(+), 15 deletions(-)

Have you tested these changes?

- James

Signature · 2 lines
James Morris
<[email protected]>
author
#2 lore raw
In reply to [email protected]
Signed-off-by Alexey Dobriyan <[email protected]>
Signed-off-by Domen Puncer <[email protected]>

---

 kj-domen/security/selinux/ss/policydb.c |   35 ++++++++++++++++++--------------
 1 files changed, 20 insertions(+), 15 deletions(-)
security/selinux/ss/policydb.c renamed from security/selinux/ss/policydb.c~sparse-security_selinux_ss_policydb +20 -15
diff -puN security/selinux/ss/policydb.c~sparse-security_selinux_ss_policydb security/selinux/ss/policydb.c
@@ -780,7 +780,7 @@ static int context_read_and_validate(str
 				     struct policydb *p,
 				     void *fp)
 {
-	u32 buf[3];
+	__le32 buf[3];
 	int rc;
 
 	rc = next_entry(buf, fp, sizeof buf);

@@ -820,7 +820,8 @@ static int perm_read(struct policydb *p,
 	char *key = NULL;
 	struct perm_datum *perdatum;
 	int rc;
-	u32 buf[2], len;
+	__le32 buf[2];
+	u32 len;
 
 	perdatum = kmalloc(sizeof(*perdatum), GFP_KERNEL);
 	if (!perdatum) {

@@ -860,7 +861,8 @@ static int common_read(struct policydb *
 {
 	char *key = NULL;
 	struct common_datum *comdatum;
-	u32 buf[4], len, nel;
+	__le32 buf[4];
+	u32 len, nel;
 	int i, rc;
 
 	comdatum = kmalloc(sizeof(*comdatum), GFP_KERNEL);

@@ -1092,7 +1094,8 @@ static int role_read(struct policydb *p,
 	char *key = NULL;
 	struct role_datum *role;
 	int rc;
-	u32 buf[2], len;
+	__le32 buf[2];
+	u32 len;
 
 	role = kmalloc(sizeof(*role), GFP_KERNEL);
 	if (!role) {

@@ -1152,7 +1155,8 @@ static int type_read(struct policydb *p,
 	char *key = NULL;
 	struct type_datum *typdatum;
 	int rc;
-	u32 buf[3], len;
+	__le32 buf[3];
+	u32 len;
 
 	typdatum = kmalloc(sizeof(*typdatum),GFP_KERNEL);
 	if (!typdatum) {

@@ -1224,7 +1228,8 @@ static int user_read(struct policydb *p,
 	char *key = NULL;
 	struct user_datum *usrdatum;
 	int rc;
-	u32 buf[2], len;
+	__le32 buf[2];
+	u32 len;
 
 	usrdatum = kmalloc(sizeof(*usrdatum), GFP_KERNEL);
 	if (!usrdatum) {

@@ -1392,7 +1397,8 @@ int policydb_read(struct policydb *p, vo
 	struct ocontext *l, *c, *newc;
 	struct genfs *genfs_p, *genfs, *newgenfs;
 	int i, j, rc;
-	u32 buf[8], len, len2, config, nprim, nel, nel2;
+	__le32 buf[8];
+	u32 len, len2, config, nprim, nel, nel2;
 	char *policydb_str;
 	struct policydb_compat_info *info;
 	struct range_trans *rt, *lrt;

@@ -1408,17 +1414,14 @@ int policydb_read(struct policydb *p, vo
 	if (rc < 0)
 		goto bad;
 
-	for (i = 0; i < 2; i++)
-		buf[i] = le32_to_cpu(buf[i]);
-
-	if (buf[0] != POLICYDB_MAGIC) {
+	if (buf[0] != cpu_to_le32(POLICYDB_MAGIC)) {
 		printk(KERN_ERR "security:  policydb magic number 0x%x does "
 		       "not match expected magic number 0x%x\n",
-		       buf[0], POLICYDB_MAGIC);
+		       le32_to_cpu(buf[0]), POLICYDB_MAGIC);
 		goto bad;
 	}
 
-	len = buf[1];
+	len = le32_to_cpu(buf[1]);
 	if (len != strlen(POLICYDB_STRING)) {
 		printk(KERN_ERR "security:  policydb string length %d does not "
 		       "match expected length %Zu\n",

@@ -1494,9 +1497,11 @@ int policydb_read(struct policydb *p, vo
 		goto bad;
 	}
 
-	if (buf[2] != info->sym_num || buf[3] != info->ocon_num) {
+	if (le32_to_cpu(buf[2]) != info->sym_num ||
+	    le32_to_cpu(buf[3]) != info->ocon_num) {
 		printk(KERN_ERR "security:  policydb table sizes (%d,%d) do "
-		       "not match mine (%d,%d)\n", buf[2], buf[3],
+		       "not match mine (%d,%d)\n",
+		       le32_to_cpu(buf[2]), le32_to_cpu(buf[3]),
 		       info->sym_num, info->ocon_num);
 		goto bad;
 	}

_

#3 lore raw
Quoted 25 lines · Domen Puncer
Signed-off-by Alexey Dobriyan <[email protected]>
Signed-off-by Domen Puncer <[email protected]>

---

 kj-domen/security/selinux/ss/policydb.c |   35 ++++++++++++++++++--------------
 1 files changed, 20 insertions(+), 15 deletions(-)

diff -puN security/selinux/ss/policydb.c~sparse-security_selinux_ss_policydb security/selinux/ss/policydb.c

security/selinux/ss/policydb.c renamed from security/selinux/ss/policydb.c~sparse-security_selinux_ss_policydb +4 -2
@@ -1494,9 +1497,11 @@ int policydb_read(struct policydb *p, vo
 		goto bad;
 	}
 
-	if (buf[2] != info->sym_num || buf[3] != info->ocon_num) {
+	if (le32_to_cpu(buf[2]) != info->sym_num ||
+	    le32_to_cpu(buf[3]) != info->ocon_num) {
 		printk(KERN_ERR "security:  policydb table sizes (%d,%d) do "
-		       "not match mine (%d,%d)\n", buf[2], buf[3],
+		       "not match mine (%d,%d)\n",
+		       le32_to_cpu(buf[2]), le32_to_cpu(buf[3]),
 		       info->sym_num, info->ocon_num);
 		goto bad;
 	}
Quoted 1 line

_

You didn't remove the loop that already converted these values to little endian already (no that isn't the same as the earlier loop that you did remove), so now you are converting them twice. And why is this new code better even if you fix this omission?

Signature · 2 lines
Stephen Smalley <[email protected]>
National Security Agency
#4 lore raw
Quoted 1 line · Stephen Smalley

You didn't remove the loop that already converted these values to little

s/ to / from /

Quoted 3 lines

endian already (no that isn't the same as the earlier loop that you did remove), so now you are converting them twice. And why is this new code better even if you fix this omission?

Note btw that you would also need to modify usage of buf[0] and buf[1] if you do remove that loop. But I'm still not clear on the benefit of the change (silencing warnings generated by a checker doesn't count unless they point to a real bug).

Signature · 2 lines
Stephen Smalley <[email protected]>
National Security Agency