Skip to contentSkip to conversation

devcgroup: always show unsigned major/minor num

PATCH In review Li Zefan 3 participants 5 messages Jul 5, 2008 – Jul 8, 2008 Security (LSM)

lore
Share & tools
lore

Li Zefan

author
#0 lore raw

PATCH 1/2 devcgroup: always show unsigned major/minor num

 # echo "b $((0x7fffffff)):$((0x80000000)) rwm" > devices.allow
 # cat devices.list
 b 214748364:-21474836 rwm

though a major/minor number of 0x800000000 is meaningless, we should not cast it to a negative value.

Signed-off-by Li Zefan <[email protected]>
---
 security/device_cgroup.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
security/device_cgroup.c +2 -2
index fd764a0..1e2e28a 100644
@@ -222,7 +222,7 @@ static void devcgroup_destroy(struct cgroup_subsys *ss,
 #define DEVCG_DENY 2
 #define DEVCG_LIST 3
 
-#define MAJMINLEN 10
+#define MAJMINLEN 13
 #define ACCLEN 4
 
 static void set_access(char *acc, short access)

@@ -254,7 +254,7 @@ static void set_majmin(char *str, unsigned m)
 	if (m == ~0)
 		sprintf(str, "*");
 	else
-		snprintf(str, MAJMINLEN, "%d", m);
+		snprintf(str, MAJMINLEN, "%u", m);
 }
 
 static int devcgroup_seq_read(struct cgroup *cgroup, struct cftype *cft,
Signature · 1 line
1.5.4.rc3
Acked-by on v1 1/2
#1 lore raw
Quoted 8 lines · Li Zefan
 # echo "b $((0x7fffffff)):$((0x80000000)) rwm" > devices.allow
 # cat devices.list
 b 214748364:-21474836 rwm

though a major/minor number of 0x800000000 is meaningless, we should not cast it to a negative value.

Signed-off-by Li Zefan <[email protected]>

makes sense.

Acked-by Serge Hallyn <[email protected]>

thanks,
-serge

Quoted 28 lines
---
 security/device_cgroup.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
security/device_cgroup.c +2 -2
index fd764a0..1e2e28a 100644
@@ -222,7 +222,7 @@ static void devcgroup_destroy(struct cgroup_subsys *ss,
 #define DEVCG_DENY 2
 #define DEVCG_LIST 3
 
-#define MAJMINLEN 10
+#define MAJMINLEN 13
 #define ACCLEN 4
 
 static void set_access(char *acc, short access)

@@ -254,7 +254,7 @@ static void set_majmin(char *str, unsigned m)
 	if (m == ~0)
 		sprintf(str, "*");
 	else
-		snprintf(str, MAJMINLEN, "%d", m);
+		snprintf(str, MAJMINLEN, "%u", m);
 }
 
 static int devcgroup_seq_read(struct cgroup *cgroup, struct cftype *cft,
Signature · 1 line
1.5.4.rc3
#2 lore raw
In reply to Li Zefan
Quoted 26 lines · Li Zefan
 # echo "b $((0x7fffffff)):$((0x80000000)) rwm" > devices.allow
 # cat devices.list
 b 214748364:-21474836 rwm

though a major/minor number of 0x800000000 is meaningless, we should not cast it to a negative value.

Signed-off-by Li Zefan <[email protected]>
---
 security/device_cgroup.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
security/device_cgroup.c +1 -1
index fd764a0..1e2e28a 100644
@@ -222,7 +222,7 @@ static void devcgroup_destroy(struct cgroup_subsys *ss,
 #define DEVCG_DENY 2
 #define DEVCG_LIST 3
 
-#define MAJMINLEN 10
+#define MAJMINLEN 13
 #define ACCLEN 4
 
 static void set_access(char *acc, short access)

security/device_cgroup.c static void set_majmin(char *str, unsigned m) · line 255

Quoted 2 lines
security/device_cgroup.c +0 -0
@@ -254,7 +254,7 @@ static void set_majmin(char *str, unsigned m)
       if (m == ~0)
               sprintf(str, "*");

Can't this just be strcat(str, "*")?

security/device_cgroup.c static void set_majmin(char *str, unsigned m) · line 257

Quoted 3 lines
security/device_cgroup.c +1 -1
       else
-               snprintf(str, MAJMINLEN, "%d", m);
+               snprintf(str, MAJMINLEN, "%u", m);

I'm not sure that using snprintf() helps here over just sprintf() - we're not actually basing it on the available space in the string, we're just assuming there's enough space.

Paul

author
#3 lore raw
Quoted 25 lines · Paul Menage
Quoted 24 lines · Li Zefan
 # echo "b $((0x7fffffff)):$((0x80000000)) rwm" > devices.allow
 # cat devices.list
 b 214748364:-21474836 rwm

though a major/minor number of 0x800000000 is meaningless, we should not cast it to a negative value.

Signed-off-by Li Zefan <[email protected]>
---
 security/device_cgroup.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
security/device_cgroup.c +1 -1
index fd764a0..1e2e28a 100644
@@ -222,7 +222,7 @@ static void devcgroup_destroy(struct cgroup_subsys *ss,
 #define DEVCG_DENY 2
 #define DEVCG_LIST 3
 
-#define MAJMINLEN 10
+#define MAJMINLEN 13
 #define ACCLEN 4
Quoted 7 lines

security/device_cgroup.c static void set_majmin(char *str, unsigned m) · line 255

Quoted 4 lines
security/device_cgroup.c +0 -0
 static void set_access(char *acc, short access)

@@ -254,7 +254,7 @@ static void set_majmin(char *str, unsigned m)
       if (m == ~0)
               sprintf(str, "*");

Can't this just be strcat(str, "*")?

No, but it can be strcpy(str, "*").

Quoted 8 lines

security/device_cgroup.c static void set_majmin(char *str, unsigned m) · line 257

Quoted 3 lines
security/device_cgroup.c +1 -1
       else
-               snprintf(str, MAJMINLEN, "%d", m);
+               snprintf(str, MAJMINLEN, "%u", m);

I'm not sure that using snprintf() helps here over just sprintf() - we're not actually basing it on the available space in the string, we're just assuming there's enough space.

Agreed.

#4 lore raw
Quoted 8 lines · Li Zefan
Quoted 6 lines

security/device_cgroup.c static void set_majmin(char *str, unsigned m) · line 255

Quoted 3 lines
security/device_cgroup.c +0 -0
@@ -254,7 +254,7 @@ static void set_majmin(char *str, unsigned m)
       if (m == ~0)
               sprintf(str, "*");

Can't this just be strcat(str, "*")?

No, but it can be strcpy(str, "*").

Oops, yes, that's what I should have put ...

Paul