* Some patches for ppp_generic.c and proc/base.c
@ 2011-05-19 10:32 samsonov
2011-05-19 11:15 ` Jesper Juhl
2011-05-19 11:27 ` Alexey Dobriyan
0 siblings, 2 replies; 3+ messages in thread
From: samsonov @ 2011-05-19 10:32 UTC (permalink / raw)
To: linux-kernel
Good day!
I mean that /proc file permission for process information must be
secure:
--- ./linux-2.6.33.4.orig/fs/proc/base.c 2010-05-13 02:04:27.000000000 +0400
+++ ./linux-2.6.33.4/fs/proc/base.c 2011-05-16 15:36:29.385923198 +0400
@@ -2570,11 +2570,11 @@
static const struct inode_operations proc_task_inode_operations;
static const struct pid_entry tgid_base_stuff[] = {
- DIR("task", S_IRUGO|S_IXUGO, proc_task_inode_operations, proc_task_operations),
- DIR("fd", S_IRUSR|S_IXUSR, proc_fd_inode_operations, proc_fd_operations),
- DIR("fdinfo", S_IRUSR|S_IXUSR, proc_fdinfo_inode_operations, proc_fdinfo_operations),
+ DIR("task", S_IWUSR|S_IRUGO|S_IXUGO, proc_task_inode_operations, proc_task_operations),
+ DIR("fd", S_IWUSR|S_IRUSR|S_IXUSR, proc_fd_inode_operations, proc_fd_operations),
+ DIR("fdinfo", S_IRUSR|S_IWUSR|S_IXUSR, proc_fdinfo_inode_operations, proc_fdinfo_operations),
#ifdef CONFIG_NET
- DIR("net", S_IRUGO|S_IXUGO, proc_net_inode_operations, proc_net_operations),
+ DIR("net", S_IRUGO|S_IWUSR|S_IXUGO, proc_net_inode_operations, proc_net_operations),
#endif
REG("environ", S_IRUSR, proc_environ_operations),
INF("auxv", S_IRUSR, proc_pid_auxv),
@@ -2608,7 +2608,7 @@
REG("pagemap", S_IRUSR, proc_pagemap_operations),
#endif
#ifdef CONFIG_SECURITY
- DIR("attr", S_IRUGO|S_IXUGO, proc_attr_dir_inode_operations, proc_attr_dir_operations),
+ DIR("attr", S_IRUGO|S_IWUSR|S_IXUGO, proc_attr_dir_inode_operations, proc_attr_dir_operations),
#endif
#ifdef CONFIG_KALLSYMS
INF("wchan", S_IRUGO, proc_pid_wchan),
@@ -2767,7 +2767,7 @@
if (!inode)
goto out;
- inode->i_mode = S_IFDIR|S_IRUGO|S_IXUGO;
+ inode->i_mode = S_IFDIR|S_IRUSR|S_IWUSR|S_IXUSR|S_IRGRP|S_IXGRP;
inode->i_op = &proc_tgid_base_inode_operations;
inode->i_fop = &proc_tgid_base_operations;
inode->i_flags|=S_IMMUTABLE;
@@ -2909,8 +2909,8 @@
* Tasks
*/
static const struct pid_entry tid_base_stuff[] = {
- DIR("fd", S_IRUSR|S_IXUSR, proc_fd_inode_operations, proc_fd_operations),
- DIR("fdinfo", S_IRUSR|S_IXUSR, proc_fdinfo_inode_operations, proc_fdinfo_operations),
+ DIR("fd", S_IRUSR|S_IWUSR|S_IXUSR, proc_fd_inode_operations, proc_fd_operations),
+ DIR("fdinfo", S_IRUSR|S_IWUSR|S_IXUSR, proc_fdinfo_inode_operations, proc_fdinfo_operations),
REG("environ", S_IRUSR, proc_environ_operations),
INF("auxv", S_IRUSR, proc_pid_auxv),
ONE("status", S_IRUGO, proc_pid_status),
@@ -2942,7 +2942,7 @@
REG("pagemap", S_IRUSR, proc_pagemap_operations),
#endif
#ifdef CONFIG_SECURITY
- DIR("attr", S_IRUGO|S_IXUGO, proc_attr_dir_inode_operations, proc_attr_dir_operations),
+ DIR("attr", S_IRUGO|S_IWUSR|S_IXUGO, proc_attr_dir_inode_operations, proc_attr_dir_operations),
#endif
#ifdef CONFIG_KALLSYMS
INF("wchan", S_IRUGO, proc_pid_wchan),
@@ -3008,7 +3008,7 @@
if (!inode)
goto out;
- inode->i_mode = S_IFDIR|S_IRUGO|S_IXUGO;
+ inode->i_mode = S_IFDIR|S_IRUSR|S_IWUSR|S_IXUSR|S_IRGRP|S_IXGRP;
inode->i_op = &proc_tid_base_inode_operations;
inode->i_fop = &proc_tid_base_operations;
inode->i_flags|=S_IMMUTABLE;
This example is for system with private system groups, with common
system group the processes dirs in proc must have permissions 600, IMHO.
In ppp driver IMHO:
--- ./linux-2.6.33.4.orig/drivers/net/ppp_generic.c 2010-05-13 02:04:27.000000000 +0400
+++ ./linux-2.6.33.4/drivers/net/ppp_generic.c 2011-05-10 13:51:11.909607463 +0400
@@ -366,8 +366,8 @@
/*
* This could (should?) be enforced by the permissions on /dev/ppp.
*/
- if (!capable(CAP_NET_ADMIN))
- return -EPERM;
+// if (!capable(CAP_NET_ADMIN))
+// return -EPERM;
return 0;
}
I mean, that change group of file must able fileowner (CAP_FOWNER), but
not the CAP_CHOWN. May be I right?
--- ./linux-2.6.33.4.orig/fs/attr.c 2010-05-13 02:04:27.000000000 +0400
+++ ./linux-2.6.33.4/fs/attr.c 2011-05-10 14:25:57.727062904 +0400
@@ -35,11 +35,10 @@
/* Make sure caller can chgrp. */
if ((ia_valid & ATTR_GID) &&
- (current_fsuid() != inode->i_uid ||
- (!in_group_p(attr->ia_gid) && attr->ia_gid != inode->i_gid)) &&
- !capable(CAP_CHOWN))
+ !(in_group_p(attr->ia_gid) && is_owner_or_cap(inode)))
goto error;
+
/* Make sure a caller can chmod. */
if (ia_valid & ATTR_MODE) {
if (!is_owner_or_cap(inode))
Thanks for attention!
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: Some patches for ppp_generic.c and proc/base.c
2011-05-19 10:32 Some patches for ppp_generic.c and proc/base.c samsonov
@ 2011-05-19 11:15 ` Jesper Juhl
2011-05-19 11:27 ` Alexey Dobriyan
1 sibling, 0 replies; 3+ messages in thread
From: Jesper Juhl @ 2011-05-19 11:15 UTC (permalink / raw)
To: samsonov; +Cc: linux-kernel
On Thu, 19 May 2011, samsonov@krista.ru wrote:
> Good day!
> I mean that /proc file permission for process information must be
> secure:
>
I'm not going to comment on whether or not changing the mode is
appropriate, but there are a few other details I will comment on:
[...]
> --- ./linux-2.6.33.4.orig/drivers/net/ppp_generic.c 2010-05-13 02:04:27.000000000 +0400
> +++ ./linux-2.6.33.4/drivers/net/ppp_generic.c 2011-05-10 13:51:11.909607463 +0400
> @@ -366,8 +366,8 @@
> /*
> * This could (should?) be enforced by the permissions on /dev/ppp.
> */
> - if (!capable(CAP_NET_ADMIN))
> - return -EPERM;
> +// if (!capable(CAP_NET_ADMIN))
> +// return -EPERM;
Don't just comment out lines. If they should go away, just remove them.
[...]
> --- ./linux-2.6.33.4.orig/fs/attr.c 2010-05-13 02:04:27.000000000 +0400
> +++ ./linux-2.6.33.4/fs/attr.c 2011-05-10 14:25:57.727062904 +0400
> @@ -35,11 +35,10 @@
>
> /* Make sure caller can chgrp. */
> if ((ia_valid & ATTR_GID) &&
> - (current_fsuid() != inode->i_uid ||
> - (!in_group_p(attr->ia_gid) && attr->ia_gid != inode->i_gid)) &&
> - !capable(CAP_CHOWN))
> + !(in_group_p(attr->ia_gid) && is_owner_or_cap(inode)))
> goto error;
>
> +
Why are you adding an extra blank line here? Seems rather pointless.
--
Jesper Juhl <jj@chaosbits.net> http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: Some patches for ppp_generic.c and proc/base.c
2011-05-19 10:32 Some patches for ppp_generic.c and proc/base.c samsonov
2011-05-19 11:15 ` Jesper Juhl
@ 2011-05-19 11:27 ` Alexey Dobriyan
1 sibling, 0 replies; 3+ messages in thread
From: Alexey Dobriyan @ 2011-05-19 11:27 UTC (permalink / raw)
To: samsonov; +Cc: linux-kernel
2011/5/19 <samsonov@krista.ru>:
> I mean that /proc file permission for process information must be
> secure:
And how exactly adding -w------- bit helps this?
> --- ./linux-2.6.33.4.orig/fs/proc/base.c
> +++ ./linux-2.6.33.4/fs/proc/base.c
2.6.39 was released today.
Never, ever mix several patches in one email.
> @@ -2570,11 +2570,11 @@
> static const struct inode_operations proc_task_inode_operations;
>
> static const struct pid_entry tgid_base_stuff[] = {
> - DIR("task", S_IRUGO|S_IXUGO, proc_task_inode_operations, proc_task_operations),
> - DIR("fd", S_IRUSR|S_IXUSR, proc_fd_inode_operations, proc_fd_operations),
> - DIR("fdinfo", S_IRUSR|S_IXUSR, proc_fdinfo_inode_operations, proc_fdinfo_operations),
> + DIR("task", S_IWUSR|S_IRUGO|S_IXUGO, proc_task_inode_operations, proc_task_operations),
> + DIR("fd", S_IWUSR|S_IRUSR|S_IXUSR, proc_fd_inode_operations, proc_fd_operations),
> + DIR("fdinfo", S_IRUSR|S_IWUSR|S_IXUSR, proc_fdinfo_inode_operations, proc_fdinfo_operations),
> #ifdef CONFIG_NET
> - DIR("net", S_IRUGO|S_IXUGO, proc_net_inode_operations, proc_net_operations),
> + DIR("net", S_IRUGO|S_IWUSR|S_IXUGO, proc_net_inode_operations, proc_net_operations),
> #endif
> REG("environ", S_IRUSR, proc_environ_operations),
> INF("auxv", S_IRUSR, proc_pid_auxv),
> @@ -2608,7 +2608,7 @@
> REG("pagemap", S_IRUSR, proc_pagemap_operations),
> #endif
> #ifdef CONFIG_SECURITY
> - DIR("attr", S_IRUGO|S_IXUGO, proc_attr_dir_inode_operations, proc_attr_dir_operations),
> + DIR("attr", S_IRUGO|S_IWUSR|S_IXUGO, proc_attr_dir_inode_operations, proc_attr_dir_operations),
> #endif
> #ifdef CONFIG_KALLSYMS
> INF("wchan", S_IRUGO, proc_pid_wchan),
> @@ -2767,7 +2767,7 @@
> if (!inode)
> goto out;
>
> - inode->i_mode = S_IFDIR|S_IRUGO|S_IXUGO;
> + inode->i_mode = S_IFDIR|S_IRUSR|S_IWUSR|S_IXUSR|S_IRGRP|S_IXGRP;
> inode->i_op = &proc_tgid_base_inode_operations;
> inode->i_fop = &proc_tgid_base_operations;
> inode->i_flags|=S_IMMUTABLE;
> @@ -2909,8 +2909,8 @@
> * Tasks
> */
> static const struct pid_entry tid_base_stuff[] = {
> - DIR("fd", S_IRUSR|S_IXUSR, proc_fd_inode_operations, proc_fd_operations),
> - DIR("fdinfo", S_IRUSR|S_IXUSR, proc_fdinfo_inode_operations, proc_fdinfo_operations),
> + DIR("fd", S_IRUSR|S_IWUSR|S_IXUSR, proc_fd_inode_operations, proc_fd_operations),
> + DIR("fdinfo", S_IRUSR|S_IWUSR|S_IXUSR, proc_fdinfo_inode_operations, proc_fdinfo_operations),
> REG("environ", S_IRUSR, proc_environ_operations),
> INF("auxv", S_IRUSR, proc_pid_auxv),
> ONE("status", S_IRUGO, proc_pid_status),
> @@ -2942,7 +2942,7 @@
> REG("pagemap", S_IRUSR, proc_pagemap_operations),
> #endif
> #ifdef CONFIG_SECURITY
> - DIR("attr", S_IRUGO|S_IXUGO, proc_attr_dir_inode_operations, proc_attr_dir_operations),
> + DIR("attr", S_IRUGO|S_IWUSR|S_IXUGO, proc_attr_dir_inode_operations, proc_attr_dir_operations),
> #endif
> #ifdef CONFIG_KALLSYMS
> INF("wchan", S_IRUGO, proc_pid_wchan),
> @@ -3008,7 +3008,7 @@
>
> if (!inode)
> goto out;
> - inode->i_mode = S_IFDIR|S_IRUGO|S_IXUGO;
> + inode->i_mode = S_IFDIR|S_IRUSR|S_IWUSR|S_IXUSR|S_IRGRP|S_IXGRP;
> inode->i_op = &proc_tid_base_inode_operations;
> inode->i_fop = &proc_tid_base_operations;
> inode->i_flags|=S_IMMUTABLE;
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2011-05-19 11:28 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-05-19 10:32 Some patches for ppp_generic.c and proc/base.c samsonov
2011-05-19 11:15 ` Jesper Juhl
2011-05-19 11:27 ` Alexey Dobriyan
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.