Re: [PATCH] sysfs: tightened sysfs permission checks

2015-05-01 Thread Rusty Russell
Gobinda Charan Maji  writes:
> There were some inconsistency in restriction to VERIFY_OCTAL_PERMISSIONS().
> Previously the test was "User perms >= group perms >= other perms". The
> permission field of User, Group or Other consists of three bits. LSB is
> EXECUTE permission, MSB is READ permission and the middle bit is WRITE
> permission. But logically WRITE is "more privileged" than READ.
>
> Say for example, permission value is "0430". Here User has only READ
> permission whereas Group has both WRITE and EXECUTE permission.
>
> So, the checks could be tightened and the tests are separated to
> USER_READABLE >= GROUP_READABLE >= OTHER_READABLE,
> USER_WRITABLE >= GROUP_WRITABLE and OTHER_WRITABLE is not permitted.
>
> Signed-off-by: Gobinda Charan Maji 

Thanks, applied!

Cheers,
Rusty.

> ---
>  include/linux/kernel.h | 18 ++
>  1 file changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> index 3a5b48e..cd54b35 100644
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -818,13 +818,15 @@ static inline void ftrace_dump(enum ftrace_dump_mode 
> oops_dump_mode) { }
>  #endif
>  
>  /* Permissions on a sysfs file: you didn't miss the 0 prefix did you? */
> -#define VERIFY_OCTAL_PERMISSIONS(perms)  
> \
> - (BUILD_BUG_ON_ZERO((perms) < 0) +   \
> -  BUILD_BUG_ON_ZERO((perms) > 0777) +\
> -  /* User perms >= group perms >= other perms */ \
> -  BUILD_BUG_ON_ZERO(((perms) >> 6) < (((perms) >> 3) & 7)) + \
> -  BUILD_BUG_ON_ZEROperms) >> 3) & 7) < ((perms) & 7)) +  \
> -  /* Other writable?  Generally considered a bad idea. */\
> -  BUILD_BUG_ON_ZERO((perms) & 2) +   \
> +#define VERIFY_OCTAL_PERMISSIONS(perms)  
> \
> + (BUILD_BUG_ON_ZERO((perms) < 0) +   
> \
> +  BUILD_BUG_ON_ZERO((perms) > 0777) +
> \
> +  /* USER_READABLE >= GROUP_READABLE >= OTHER_READABLE */
> \
> +  BUILD_BUG_ON_ZEROperms) >> 6) & 4) < (((perms) >> 3) & 4)) +   
> \
> +  BUILD_BUG_ON_ZEROperms) >> 3) & 4) < ((perms) & 4)) +  
> \
> +  /* USER_WRITABLE >= GROUP_WRITABLE */  
> \
> +  BUILD_BUG_ON_ZEROperms) >> 6) & 2) < (((perms) >> 3) & 2)) +   
> \
> +  /* OTHER_WRITABLE?  Generally considered a bad idea. */
> \
> +  BUILD_BUG_ON_ZERO((perms) & 2) +   
> \
>(perms))
>  #endif
> -- 
> 1.8.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] sysfs: tightened sysfs permission checks

2015-05-01 Thread Gobinda Charan Maji
There were some inconsistency in restriction to VERIFY_OCTAL_PERMISSIONS().
Previously the test was "User perms >= group perms >= other perms". The
permission field of User, Group or Other consists of three bits. LSB is
EXECUTE permission, MSB is READ permission and the middle bit is WRITE
permission. But logically WRITE is "more privileged" than READ.

Say for example, permission value is "0430". Here User has only READ
permission whereas Group has both WRITE and EXECUTE permission.

So, the checks could be tightened and the tests are separated to
USER_READABLE >= GROUP_READABLE >= OTHER_READABLE,
USER_WRITABLE >= GROUP_WRITABLE and OTHER_WRITABLE is not permitted.

Signed-off-by: Gobinda Charan Maji 
---
 include/linux/kernel.h | 18 ++
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 3a5b48e..cd54b35 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -818,13 +818,15 @@ static inline void ftrace_dump(enum ftrace_dump_mode 
oops_dump_mode) { }
 #endif
 
 /* Permissions on a sysfs file: you didn't miss the 0 prefix did you? */
-#define VERIFY_OCTAL_PERMISSIONS(perms)
\
-   (BUILD_BUG_ON_ZERO((perms) < 0) +   \
-BUILD_BUG_ON_ZERO((perms) > 0777) +\
-/* User perms >= group perms >= other perms */ \
-BUILD_BUG_ON_ZERO(((perms) >> 6) < (((perms) >> 3) & 7)) + \
-BUILD_BUG_ON_ZEROperms) >> 3) & 7) < ((perms) & 7)) +  \
-/* Other writable?  Generally considered a bad idea. */\
-BUILD_BUG_ON_ZERO((perms) & 2) +   \
+#define VERIFY_OCTAL_PERMISSIONS(perms)
\
+   (BUILD_BUG_ON_ZERO((perms) < 0) +   
\
+BUILD_BUG_ON_ZERO((perms) > 0777) +
\
+/* USER_READABLE >= GROUP_READABLE >= OTHER_READABLE */
\
+BUILD_BUG_ON_ZEROperms) >> 6) & 4) < (((perms) >> 3) & 4)) +   
\
+BUILD_BUG_ON_ZEROperms) >> 3) & 4) < ((perms) & 4)) +  
\
+/* USER_WRITABLE >= GROUP_WRITABLE */  
\
+BUILD_BUG_ON_ZEROperms) >> 6) & 2) < (((perms) >> 3) & 2)) +   
\
+/* OTHER_WRITABLE?  Generally considered a bad idea. */
\
+BUILD_BUG_ON_ZERO((perms) & 2) +   
\
 (perms))
 #endif
-- 
1.8.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] sysfs: tightened sysfs permission checks

2015-05-01 Thread Rusty Russell
Gobinda Charan Maji gobinda.cem...@gmail.com writes:
 There were some inconsistency in restriction to VERIFY_OCTAL_PERMISSIONS().
 Previously the test was User perms = group perms = other perms. The
 permission field of User, Group or Other consists of three bits. LSB is
 EXECUTE permission, MSB is READ permission and the middle bit is WRITE
 permission. But logically WRITE is more privileged than READ.

 Say for example, permission value is 0430. Here User has only READ
 permission whereas Group has both WRITE and EXECUTE permission.

 So, the checks could be tightened and the tests are separated to
 USER_READABLE = GROUP_READABLE = OTHER_READABLE,
 USER_WRITABLE = GROUP_WRITABLE and OTHER_WRITABLE is not permitted.

 Signed-off-by: Gobinda Charan Maji gobinda.cem...@gmail.com

Thanks, applied!

Cheers,
Rusty.

 ---
  include/linux/kernel.h | 18 ++
  1 file changed, 10 insertions(+), 8 deletions(-)

 diff --git a/include/linux/kernel.h b/include/linux/kernel.h
 index 3a5b48e..cd54b35 100644
 --- a/include/linux/kernel.h
 +++ b/include/linux/kernel.h
 @@ -818,13 +818,15 @@ static inline void ftrace_dump(enum ftrace_dump_mode 
 oops_dump_mode) { }
  #endif
  
  /* Permissions on a sysfs file: you didn't miss the 0 prefix did you? */
 -#define VERIFY_OCTAL_PERMISSIONS(perms)  
 \
 - (BUILD_BUG_ON_ZERO((perms)  0) +   \
 -  BUILD_BUG_ON_ZERO((perms)  0777) +\
 -  /* User perms = group perms = other perms */ \
 -  BUILD_BUG_ON_ZERO(((perms)  6)  (((perms)  3)  7)) + \
 -  BUILD_BUG_ON_ZEROperms)  3)  7)  ((perms)  7)) +  \
 -  /* Other writable?  Generally considered a bad idea. */\
 -  BUILD_BUG_ON_ZERO((perms)  2) +   \
 +#define VERIFY_OCTAL_PERMISSIONS(perms)  
 \
 + (BUILD_BUG_ON_ZERO((perms)  0) +   
 \
 +  BUILD_BUG_ON_ZERO((perms)  0777) +
 \
 +  /* USER_READABLE = GROUP_READABLE = OTHER_READABLE */
 \
 +  BUILD_BUG_ON_ZEROperms)  6)  4)  (((perms)  3)  4)) +   
 \
 +  BUILD_BUG_ON_ZEROperms)  3)  4)  ((perms)  4)) +  
 \
 +  /* USER_WRITABLE = GROUP_WRITABLE */  
 \
 +  BUILD_BUG_ON_ZEROperms)  6)  2)  (((perms)  3)  2)) +   
 \
 +  /* OTHER_WRITABLE?  Generally considered a bad idea. */
 \
 +  BUILD_BUG_ON_ZERO((perms)  2) +   
 \
(perms))
  #endif
 -- 
 1.8.1.4
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] sysfs: tightened sysfs permission checks

2015-05-01 Thread Gobinda Charan Maji
There were some inconsistency in restriction to VERIFY_OCTAL_PERMISSIONS().
Previously the test was User perms = group perms = other perms. The
permission field of User, Group or Other consists of three bits. LSB is
EXECUTE permission, MSB is READ permission and the middle bit is WRITE
permission. But logically WRITE is more privileged than READ.

Say for example, permission value is 0430. Here User has only READ
permission whereas Group has both WRITE and EXECUTE permission.

So, the checks could be tightened and the tests are separated to
USER_READABLE = GROUP_READABLE = OTHER_READABLE,
USER_WRITABLE = GROUP_WRITABLE and OTHER_WRITABLE is not permitted.

Signed-off-by: Gobinda Charan Maji gobinda.cem...@gmail.com
---
 include/linux/kernel.h | 18 ++
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 3a5b48e..cd54b35 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -818,13 +818,15 @@ static inline void ftrace_dump(enum ftrace_dump_mode 
oops_dump_mode) { }
 #endif
 
 /* Permissions on a sysfs file: you didn't miss the 0 prefix did you? */
-#define VERIFY_OCTAL_PERMISSIONS(perms)
\
-   (BUILD_BUG_ON_ZERO((perms)  0) +   \
-BUILD_BUG_ON_ZERO((perms)  0777) +\
-/* User perms = group perms = other perms */ \
-BUILD_BUG_ON_ZERO(((perms)  6)  (((perms)  3)  7)) + \
-BUILD_BUG_ON_ZEROperms)  3)  7)  ((perms)  7)) +  \
-/* Other writable?  Generally considered a bad idea. */\
-BUILD_BUG_ON_ZERO((perms)  2) +   \
+#define VERIFY_OCTAL_PERMISSIONS(perms)
\
+   (BUILD_BUG_ON_ZERO((perms)  0) +   
\
+BUILD_BUG_ON_ZERO((perms)  0777) +
\
+/* USER_READABLE = GROUP_READABLE = OTHER_READABLE */
\
+BUILD_BUG_ON_ZEROperms)  6)  4)  (((perms)  3)  4)) +   
\
+BUILD_BUG_ON_ZEROperms)  3)  4)  ((perms)  4)) +  
\
+/* USER_WRITABLE = GROUP_WRITABLE */  
\
+BUILD_BUG_ON_ZEROperms)  6)  2)  (((perms)  3)  2)) +   
\
+/* OTHER_WRITABLE?  Generally considered a bad idea. */
\
+BUILD_BUG_ON_ZERO((perms)  2) +   
\
 (perms))
 #endif
-- 
1.8.1.4

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/