Re: [PATCH 9/9] sysfs: disallow world-writable files.
On 30 April 2015 at 07:32, Rusty Russell wrote: > You're absolutely right, well spotted! The checks can be tightened. We > don't really care about execute, but logically write is "more > privileged" than read. > > Best to separate the tests; OTHER_WRITABLE <= GROUP_WRITABLE <= OWNER_WRITABLE > and OTHER_READABLE <= GROUP_READABLE <= OWNER_READABLE. > > A patch would be welcome! Thanks for the suggestion. OTHER_WRITABLE is already not permitted. So, added the checks for GROUP_WRITABLE <= OWNER_WRITABLE for write and OTHER_READABLE <= GROUP_READABLE <= OWNER_READABLE for read. I am just sending a separate patch for this. The subject line will be "[PATCH] sysfs: tightened sysfs permission checks" -- Thanks, Gobinda -- 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 9/9] sysfs: disallow world-writable files.
On 30 April 2015 at 07:32, Rusty Russell ru...@rustcorp.com.au wrote: You're absolutely right, well spotted! The checks can be tightened. We don't really care about execute, but logically write is more privileged than read. Best to separate the tests; OTHER_WRITABLE = GROUP_WRITABLE = OWNER_WRITABLE and OTHER_READABLE = GROUP_READABLE = OWNER_READABLE. A patch would be welcome! Thanks for the suggestion. OTHER_WRITABLE is already not permitted. So, added the checks for GROUP_WRITABLE = OWNER_WRITABLE for write and OTHER_READABLE = GROUP_READABLE = OWNER_READABLE for read. I am just sending a separate patch for this. The subject line will be [PATCH] sysfs: tightened sysfs permission checks -- Thanks, Gobinda -- 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 9/9] sysfs: disallow world-writable files.
Gobinda Maji writes: > Hi Rusty, Hi Gobinda, > I have a small doubt about the permission restriction (User perms >= > group perms >= other perms) in VERIFY_OCTAL_PERMISSIONS(). Please Note > that 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. Say for example, permission value is "0431". Here > User has only READ permission whereas Group has both WRITE and EXECUTE > permission and Other has EXECUTE permission. I guess, it is not good > to give Group the WRITE permission whereas User itself has no WRITE > permission. You're absolutely right, well spotted! The checks can be tightened. We don't really care about execute, but logically write is "more privileged" than read. Best to separate the tests; OTHER_WRITABLE <= GROUP_WRITABLE <= OWNER_WRITABLE and OTHER_READABLE <= GROUP_READABLE <= OWNER_READABLE. A patch would be welcome! Thanks, Rusty. -- 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 9/9] sysfs: disallow world-writable files.
Rusty Russell rustcorp.com.au> writes:
>
> This check was introduced in 2006 by Alexey Dobriyan (9774a1f54f173)
> for module parameters; we removed it when we unified the check into
> VERIFY_OCTAL_PERMISSIONS() as sysfs didn't have the same requirement.
> Now all those users are fixed, reintroduce it.
>
> Cc: Alexey Dobriyan gmail.com>
> Cc: Dave Jones redhat.com>
> Cc: Joe Perches perches.com>
> Signed-off-by: Rusty Russell rustcorp.com.au>
> ---
> include/linux/kernel.h | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> index 4c52907a6d8b..43e1c6a9683e 100644
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> -849,5 +849,7 static inline void
ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { }
>/* 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) + \
>(perms))
> #endif
Hi Rusty,
I have a small doubt about the permission restriction (User perms >= group
perms >= other perms) in VERIFY_OCTAL_PERMISSIONS(). Please Note that
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. Say for example, permission value is "0431". Here User has only
READ permission whereas Group has both WRITE and EXECUTE permission and
Other has EXECUTE permission. I guess, it is not good to give Group the
WRITE permission whereas User itself has no WRITE permission.
May be, it's better to check those three permissions bit wise rather than
as a whole.
I already had posted my query at:
http://thread.gmane.org/gmane.linux.kernel/1667095/focus=1737833
But could get any reply.
Please rethink about my point and let me know your opinion.
Thanks,
Gobinda
--
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 9/9] sysfs: disallow world-writable files.
Hi Rusty, I have a small doubt about the permission restriction (User perms >= group perms >= other perms) in VERIFY_OCTAL_PERMISSIONS(). Please Note that 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. Say for example, permission value is "0431". Here User has only READ permission whereas Group has both WRITE and EXECUTE permission and Other has EXECUTE permission. I guess, it is not good to give Group the WRITE permission whereas User itself has no WRITE permission. May be, it's better to check those three permissions bit wise rather than as a whole. I already had posted my query at: http://thread.gmane.org/gmane.linux.kernel/1667095/focus=1737833 But could get any reply. Please rethink about my point and let me know your opinion. Thanks, Gobinda -- 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 9/9] sysfs: disallow world-writable files.
Rusty Russell rusty at rustcorp.com.au writes:
This check was introduced in 2006 by Alexey Dobriyan (9774a1f54f173)
for module parameters; we removed it when we unified the check into
VERIFY_OCTAL_PERMISSIONS() as sysfs didn't have the same requirement.
Now all those users are fixed, reintroduce it.
Cc: Alexey Dobriyan adobriyan at gmail.com
Cc: Dave Jones davej at redhat.com
Cc: Joe Perches joe at perches.com
Signed-off-by: Rusty Russell rusty at rustcorp.com.au
---
include/linux/kernel.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 4c52907a6d8b..43e1c6a9683e 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
at at -849,5 +849,7 at at static inline void
ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { }
/* 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) + \
(perms))
#endif
Hi Rusty,
I have a small doubt about the permission restriction (User perms = group
perms = other perms) in VERIFY_OCTAL_PERMISSIONS(). Please Note that
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. Say for example, permission value is 0431. Here User has only
READ permission whereas Group has both WRITE and EXECUTE permission and
Other has EXECUTE permission. I guess, it is not good to give Group the
WRITE permission whereas User itself has no WRITE permission.
May be, it's better to check those three permissions bit wise rather than
as a whole.
I already had posted my query at:
http://thread.gmane.org/gmane.linux.kernel/1667095/focus=1737833
But could get any reply.
Please rethink about my point and let me know your opinion.
Thanks,
Gobinda
--
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 9/9] sysfs: disallow world-writable files.
Hi Rusty, I have a small doubt about the permission restriction (User perms = group perms = other perms) in VERIFY_OCTAL_PERMISSIONS(). Please Note that 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. Say for example, permission value is 0431. Here User has only READ permission whereas Group has both WRITE and EXECUTE permission and Other has EXECUTE permission. I guess, it is not good to give Group the WRITE permission whereas User itself has no WRITE permission. May be, it's better to check those three permissions bit wise rather than as a whole. I already had posted my query at: http://thread.gmane.org/gmane.linux.kernel/1667095/focus=1737833 But could get any reply. Please rethink about my point and let me know your opinion. Thanks, Gobinda -- 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 9/9] sysfs: disallow world-writable files.
Gobinda Maji gobinda.cem...@gmail.com writes: Hi Rusty, Hi Gobinda, I have a small doubt about the permission restriction (User perms = group perms = other perms) in VERIFY_OCTAL_PERMISSIONS(). Please Note that 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. Say for example, permission value is 0431. Here User has only READ permission whereas Group has both WRITE and EXECUTE permission and Other has EXECUTE permission. I guess, it is not good to give Group the WRITE permission whereas User itself has no WRITE permission. You're absolutely right, well spotted! The checks can be tightened. We don't really care about execute, but logically write is more privileged than read. Best to separate the tests; OTHER_WRITABLE = GROUP_WRITABLE = OWNER_WRITABLE and OTHER_READABLE = GROUP_READABLE = OWNER_READABLE. A patch would be welcome! Thanks, Rusty. -- 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 9/9] sysfs: disallow world-writable files.
This check was introduced in 2006 by Alexey Dobriyan (9774a1f54f173)
for module parameters; we removed it when we unified the check into
VERIFY_OCTAL_PERMISSIONS() as sysfs didn't have the same requirement.
Now all those users are fixed, reintroduce it.
Cc: Alexey Dobriyan
Cc: Dave Jones
Cc: Joe Perches
Signed-off-by: Rusty Russell
---
include/linux/kernel.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 4c52907a6d8b..43e1c6a9683e 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -849,5 +849,7 @@ static inline void ftrace_dump(enum ftrace_dump_mode
oops_dump_mode) { }
/* 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) + \
(perms))
#endif
--
1.8.3.2
--
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 9/9] sysfs: disallow world-writable files.
This check was introduced in 2006 by Alexey Dobriyan (9774a1f54f173)
for module parameters; we removed it when we unified the check into
VERIFY_OCTAL_PERMISSIONS() as sysfs didn't have the same requirement.
Now all those users are fixed, reintroduce it.
Cc: Alexey Dobriyan adobri...@gmail.com
Cc: Dave Jones da...@redhat.com
Cc: Joe Perches j...@perches.com
Signed-off-by: Rusty Russell ru...@rustcorp.com.au
---
include/linux/kernel.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 4c52907a6d8b..43e1c6a9683e 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -849,5 +849,7 @@ static inline void ftrace_dump(enum ftrace_dump_mode
oops_dump_mode) { }
/* 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) + \
(perms))
#endif
--
1.8.3.2
--
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/
