* [PATCH] security/selinux: fix /proc/sys/ labeling @ 2011-02-01 0:17 Lucian Adrian Grijincu 2011-02-01 1:32 ` [PATCH] security: remove unused security_sysctl hook Lucian Adrian Grijincu 2011-02-01 15:02 ` [PATCH] security/selinux: fix /proc/sys/ labeling Stephen Smalley 0 siblings, 2 replies; 20+ messages in thread From: Lucian Adrian Grijincu @ 2011-02-01 0:17 UTC (permalink / raw) To: Stephen Smalley, James Morris, Eric Paris, Nick Piggin, Eric W. Biederman, linux-kernel, linux-security-module Cc: Lucian Adrian Grijincu From: Eric W. Biederman <ebiederm@xmission.com> This fixes an old (2007) selinux regression: filesystem labeling for /proc/sys returned -r--r--r-- unknown /proc/sys/fs/file-nr instead of -r--r--r-- system_u:object_r:sysctl_fs_t:s0 /proc/sys/fs/file-nr Events that lead to breaking of /proc/sys selinux labeling: 1) sysctl was reimplemented to route all calls through /proc/sysctl commit 77b14db502cb85a031fe8fde6c85d52f3e0acb63 [PATCH] sysctl: reimplement the sysctl proc support 2) proc_dir_entry was removed from ctl_table: commit 3fbfa98112fc3962c416452a0baf2214381030e6 [PATCH] sysctl: remove the proc_dir_entry member for the sysctl tables 3) selinux still walked the proc_dir_entry tree to apply labeling. Because ctl_tables don't have a proc_dir_entry, we did not label /proc/sys/ inodes any more. To achieve this the /proc/sys inodes were marked private and private inodes were ignored by selinux. commit bbaca6c2e7ef0f663bc31be4dad7cf530f6c4962 [PATCH] selinux: enhance selinux to always ignore private inodes commit 86a71dbd3e81e8870d0f0e56b87875f57e58222b [PATCH] sysctl: hide the sysctl proc inodes from selinux Access control checks have been done by means of a special sysctl hook that was called for read/write accesses to any /proc/sys/ entry. We don't have to do this because, instead of walking the proc_dir_entry tree we can walk the dentry tree (as done in this patch). With this patch: * we don't mark /proc/sys inodes as private * we don't need the sysclt security hook * we walk the dentry tree to find the path to the inode. We have to strip the PID in /proc/PID/ entries that have a proc_dir_entry because selinux does not know how to label paths like '/1/net/rpc/nfsd.fh' (and defaults to 'proc_t' labeling). Selinux does know of '/net/rpc/nfsd.fh' (and applies the 'sysctl_rpc_t' label). PID stripping from the path was done implicitly in the previous code because the proc_dir_entry tree had the root in '/net' in the example from above. The dentry tree has the root in '/1'. Signed-off-by: Eric W. Biederman <ebiederm@xmission.com> Signed-off-by: Lucian Adrian Grijincu <lucian.grijincu@gmail.com> --- fs/proc/proc_sysctl.c | 1 - security/selinux/hooks.c | 119 +++++++--------------------------------------- 2 files changed, 18 insertions(+), 102 deletions(-) diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c index 09a1f92..fb707e0 100644 --- a/fs/proc/proc_sysctl.c +++ b/fs/proc/proc_sysctl.c @@ -32,7 +32,6 @@ static struct inode *proc_sys_make_inode(struct super_block *sb, ei->sysctl_entry = table; inode->i_mtime = inode->i_atime = inode->i_ctime = CURRENT_TIME; - inode->i_flags |= S_PRIVATE; /* tell selinux to ignore this inode */ inode->i_mode = table->mode; if (!table->child) { inode->i_mode |= S_IFREG; diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index e276eb4..7c5dfb1 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -43,7 +43,6 @@ #include <linux/fdtable.h> #include <linux/namei.h> #include <linux/mount.h> -#include <linux/proc_fs.h> #include <linux/netfilter_ipv4.h> #include <linux/netfilter_ipv6.h> #include <linux/tty.h> @@ -70,7 +69,6 @@ #include <net/ipv6.h> #include <linux/hugetlb.h> #include <linux/personality.h> -#include <linux/sysctl.h> #include <linux/audit.h> #include <linux/string.h> #include <linux/selinux.h> @@ -1120,39 +1118,35 @@ static inline u16 socket_type_to_security_class(int family, int type, int protoc } #ifdef CONFIG_PROC_FS -static int selinux_proc_get_sid(struct proc_dir_entry *de, +static int selinux_proc_get_sid(struct dentry *dentry, u16 tclass, u32 *sid) { - int buflen, rc; - char *buffer, *path, *end; + int rc; + char *buffer, *path; buffer = (char *)__get_free_page(GFP_KERNEL); if (!buffer) return -ENOMEM; - buflen = PAGE_SIZE; - end = buffer+buflen; - *--end = '\0'; - buflen--; - path = end-1; - *path = '/'; - while (de && de != de->parent) { - buflen -= de->namelen + 1; - if (buflen < 0) - break; - end -= de->namelen; - memcpy(end, de->name, de->namelen); - *--end = '/'; - path = end; - de = de->parent; + path = dentry_path_raw(dentry, buffer, PAGE_SIZE); + if (IS_ERR(path)) + rc = PTR_ERR(path); + else { + /* each process gets a /proc/PID/ entry. Strip off the + * PID part to get a valid selinux labeling. + * e.g. /proc/1/net/rpc/nfs -> /net/rpc/nfs */ + while (path[1] >= '0' && path[1] <= '9') { + path[1] = '/'; + path++; + } + rc = security_genfs_sid("proc", path, tclass, sid); } - rc = security_genfs_sid("proc", path, tclass, sid); free_page((unsigned long)buffer); return rc; } #else -static int selinux_proc_get_sid(struct proc_dir_entry *de, +static int selinux_proc_get_sid(struct dentry *dentry, u16 tclass, u32 *sid) { @@ -1317,9 +1311,9 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent if ((sbsec->flags & SE_SBPROC) && !S_ISLNK(inode->i_mode)) { struct proc_inode *proci = PROC_I(inode); - if (proci->pde) { + if (opt_dentry && (proci->pde || proci->sysctl)) { isec->sclass = inode_mode_to_security_class(inode->i_mode); - rc = selinux_proc_get_sid(proci->pde, + rc = selinux_proc_get_sid(opt_dentry, isec->sclass, &sid); if (rc) @@ -1862,82 +1856,6 @@ static int selinux_capable(struct task_struct *tsk, const struct cred *cred, return task_has_capability(tsk, cred, cap, audit); } -static int selinux_sysctl_get_sid(ctl_table *table, u16 tclass, u32 *sid) -{ - int buflen, rc; - char *buffer, *path, *end; - - rc = -ENOMEM; - buffer = (char *)__get_free_page(GFP_KERNEL); - if (!buffer) - goto out; - - buflen = PAGE_SIZE; - end = buffer+buflen; - *--end = '\0'; - buflen--; - path = end-1; - *path = '/'; - while (table) { - const char *name = table->procname; - size_t namelen = strlen(name); - buflen -= namelen + 1; - if (buflen < 0) - goto out_free; - end -= namelen; - memcpy(end, name, namelen); - *--end = '/'; - path = end; - table = table->parent; - } - buflen -= 4; - if (buflen < 0) - goto out_free; - end -= 4; - memcpy(end, "/sys", 4); - path = end; - rc = security_genfs_sid("proc", path, tclass, sid); -out_free: - free_page((unsigned long)buffer); -out: - return rc; -} - -static int selinux_sysctl(ctl_table *table, int op) -{ - int error = 0; - u32 av; - u32 tsid, sid; - int rc; - - sid = current_sid(); - - rc = selinux_sysctl_get_sid(table, (op == 0001) ? - SECCLASS_DIR : SECCLASS_FILE, &tsid); - if (rc) { - /* Default to the well-defined sysctl SID. */ - tsid = SECINITSID_SYSCTL; - } - - /* The op values are "defined" in sysctl.c, thereby creating - * a bad coupling between this module and sysctl.c */ - if (op == 001) { - error = avc_has_perm(sid, tsid, - SECCLASS_DIR, DIR__SEARCH, NULL); - } else { - av = 0; - if (op & 004) - av |= FILE__READ; - if (op & 002) - av |= FILE__WRITE; - if (av) - error = avc_has_perm(sid, tsid, - SECCLASS_FILE, av, NULL); - } - - return error; -} - static int selinux_quotactl(int cmds, int type, int id, struct super_block *sb) { const struct cred *cred = current_cred(); @@ -5398,7 +5316,6 @@ static struct security_operations selinux_ops = { .ptrace_traceme = selinux_ptrace_traceme, .capget = selinux_capget, .capset = selinux_capset, - .sysctl = selinux_sysctl, .capable = selinux_capable, .quotactl = selinux_quotactl, .quota_on = selinux_quota_on, -- 1.7.4.rc1.7.g2cf08.dirty ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH] security: remove unused security_sysctl hook 2011-02-01 0:17 [PATCH] security/selinux: fix /proc/sys/ labeling Lucian Adrian Grijincu @ 2011-02-01 1:32 ` Lucian Adrian Grijincu 2011-02-01 15:02 ` [PATCH] security/selinux: fix /proc/sys/ labeling Stephen Smalley 1 sibling, 0 replies; 20+ messages in thread From: Lucian Adrian Grijincu @ 2011-02-01 1:32 UTC (permalink / raw) To: James Morris, Eric Paris, Stephen Smalley, ebiederm, linux-kernel, linux-security-module Cc: Lucian Adrian Grijincu The only user for this hook was selinux. sysctl routes every call through /proc/sys/. Selinux and other security modules use the file system checks for sysctl too, so no need for this hook any more. Signed-off-by: Lucian Adrian Grijincu <lucian.grijincu@gmail.com> --- include/linux/security.h | 13 ------------- kernel/sysctl.c | 5 ----- security/capability.c | 6 ------ security/security.c | 5 ----- 4 files changed, 0 insertions(+), 29 deletions(-) diff --git a/include/linux/security.h b/include/linux/security.h index c642bb8..e7b48dc 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -1257,12 +1257,6 @@ static inline void security_free_mnt_opts(struct security_mnt_opts *opts) * @cap contains the capability <include/linux/capability.h>. * @audit: Whether to write an audit message or not * Return 0 if the capability is granted for @tsk. - * @sysctl: - * Check permission before accessing the @table sysctl variable in the - * manner specified by @op. - * @table contains the ctl_table structure for the sysctl variable. - * @op contains the operation (001 = search, 002 = write, 004 = read). - * Return 0 if permission is granted. * @syslog: * Check permission before accessing the kernel message ring or changing * logging to the console. @@ -1383,7 +1377,6 @@ struct security_operations { const kernel_cap_t *permitted); int (*capable) (struct task_struct *tsk, const struct cred *cred, int cap, int audit); - int (*sysctl) (struct ctl_table *table, int op); int (*quotactl) (int cmds, int type, int id, struct super_block *sb); int (*quota_on) (struct dentry *dentry); int (*syslog) (int type); @@ -1665,7 +1658,6 @@ int security_capset(struct cred *new, const struct cred *old, int security_capable(int cap); int security_real_capable(struct task_struct *tsk, int cap); int security_real_capable_noaudit(struct task_struct *tsk, int cap); -int security_sysctl(struct ctl_table *table, int op); int security_quotactl(int cmds, int type, int id, struct super_block *sb); int security_quota_on(struct dentry *dentry); int security_syslog(int type); @@ -1883,11 +1875,6 @@ int security_real_capable_noaudit(struct task_struct *tsk, int cap) return ret; } -static inline int security_sysctl(struct ctl_table *table, int op) -{ - return 0; -} - static inline int security_quotactl(int cmds, int type, int id, struct super_block *sb) { diff --git a/kernel/sysctl.c b/kernel/sysctl.c index 0f1bd83..56f6fc1 100644 --- a/kernel/sysctl.c +++ b/kernel/sysctl.c @@ -1685,13 +1685,8 @@ static int test_perm(int mode, int op) int sysctl_perm(struct ctl_table_root *root, struct ctl_table *table, int op) { - int error; int mode; - error = security_sysctl(table, op & (MAY_READ | MAY_WRITE | MAY_EXEC)); - if (error) - return error; - if (root->permissions) mode = root->permissions(root, current->nsproxy, table); else diff --git a/security/capability.c b/security/capability.c index 2a5df2b..ebe3b5d 100644 --- a/security/capability.c +++ b/security/capability.c @@ -12,11 +12,6 @@ #include <linux/security.h> -static int cap_sysctl(ctl_table *table, int op) -{ - return 0; -} - static int cap_syslog(int type) { return 0; @@ -880,7 +875,6 @@ void __init security_fixup_ops(struct security_operations *ops) set_to_cap_if_null(ops, capable); set_to_cap_if_null(ops, quotactl); set_to_cap_if_null(ops, quota_on); - set_to_cap_if_null(ops, sysctl); set_to_cap_if_null(ops, syslog); set_to_cap_if_null(ops, settime); set_to_cap_if_null(ops, vm_enough_memory); diff --git a/security/security.c b/security/security.c index 739e403..53d793a 100644 --- a/security/security.c +++ b/security/security.c @@ -182,11 +182,6 @@ int security_real_capable_noaudit(struct task_struct *tsk, int cap) return ret; } -int security_sysctl(struct ctl_table *table, int op) -{ - return security_ops->sysctl(table, op); -} - int security_quotactl(int cmds, int type, int id, struct super_block *sb) { return security_ops->quotactl(cmds, type, id, sb); -- 1.7.4.rc1.7.g2cf08.dirty ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH] security/selinux: fix /proc/sys/ labeling 2011-02-01 0:17 [PATCH] security/selinux: fix /proc/sys/ labeling Lucian Adrian Grijincu 2011-02-01 1:32 ` [PATCH] security: remove unused security_sysctl hook Lucian Adrian Grijincu @ 2011-02-01 15:02 ` Stephen Smalley 2011-02-01 15:53 ` Lucian Adrian Grijincu 1 sibling, 1 reply; 20+ messages in thread From: Stephen Smalley @ 2011-02-01 15:02 UTC (permalink / raw) To: Lucian Adrian Grijincu Cc: James Morris, Eric Paris, Nick Piggin, Eric W. Biederman, linux-kernel, linux-security-module On Tue, 2011-02-01 at 02:17 +0200, Lucian Adrian Grijincu wrote: > From: Eric W. Biederman <ebiederm@xmission.com> Is this patch really from Eric or just derived from an earlier patch by him? > > This fixes an old (2007) selinux regression: filesystem labeling for > /proc/sys returned > -r--r--r-- unknown /proc/sys/fs/file-nr > instead of > -r--r--r-- system_u:object_r:sysctl_fs_t:s0 /proc/sys/fs/file-nr > > Events that lead to breaking of /proc/sys selinux labeling: > > 1) sysctl was reimplemented to route all calls through /proc/sysctl > > commit 77b14db502cb85a031fe8fde6c85d52f3e0acb63 > [PATCH] sysctl: reimplement the sysctl proc support > > 2) proc_dir_entry was removed from ctl_table: > > commit 3fbfa98112fc3962c416452a0baf2214381030e6 > [PATCH] sysctl: remove the proc_dir_entry member for the sysctl tables > > 3) selinux still walked the proc_dir_entry tree to apply > labeling. Because ctl_tables don't have a proc_dir_entry, we did > not label /proc/sys/ inodes any more. To achieve this the /proc/sys > inodes were marked private and private inodes were ignored by > selinux. > > commit bbaca6c2e7ef0f663bc31be4dad7cf530f6c4962 > [PATCH] selinux: enhance selinux to always ignore private inodes > > commit 86a71dbd3e81e8870d0f0e56b87875f57e58222b > [PATCH] sysctl: hide the sysctl proc inodes from selinux > > Access control checks have been done by means of a special sysctl hook > that was called for read/write accesses to any /proc/sys/ entry. > > We don't have to do this because, instead of walking the > proc_dir_entry tree we can walk the dentry tree (as done in this > patch). With this patch: > * we don't mark /proc/sys inodes as private > * we don't need the sysclt security hook > * we walk the dentry tree to find the path to the inode. > > We have to strip the PID in /proc/PID/ entries that have a > proc_dir_entry because selinux does not know how to label paths like > '/1/net/rpc/nfsd.fh' (and defaults to 'proc_t' labeling). Selinux does > know of '/net/rpc/nfsd.fh' (and applies the 'sysctl_rpc_t' label). > > PID stripping from the path was done implicitly in the previous code > because the proc_dir_entry tree had the root in '/net' in the example > from above. The dentry tree has the root in '/1'. > > Signed-off-by: Eric W. Biederman <ebiederm@xmission.com> And did Eric truly sign off on this patch or just on an earlier one? > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index e276eb4..7c5dfb1 100644 > --- a/security/selinux/hooks.c > +++ b/security/selinux/hooks.c > @@ -1317,9 +1311,9 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent > > if ((sbsec->flags & SE_SBPROC) && !S_ISLNK(inode->i_mode)) { > struct proc_inode *proci = PROC_I(inode); > - if (proci->pde) { > + if (opt_dentry && (proci->pde || proci->sysctl)) { > isec->sclass = inode_mode_to_security_class(inode->i_mode); > - rc = selinux_proc_get_sid(proci->pde, > + rc = selinux_proc_get_sid(opt_dentry, > isec->sclass, > &sid); > if (rc) It would be nice if we could eliminate the last remaining piece of proc internal knowledge from this code - why do we need the proci->pde || proci->sysctl test here? What changes without it? -- Stephen Smalley National Security Agency ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] security/selinux: fix /proc/sys/ labeling 2011-02-01 15:02 ` [PATCH] security/selinux: fix /proc/sys/ labeling Stephen Smalley @ 2011-02-01 15:53 ` Lucian Adrian Grijincu 2011-02-01 15:59 ` Stephen Smalley 0 siblings, 1 reply; 20+ messages in thread From: Lucian Adrian Grijincu @ 2011-02-01 15:53 UTC (permalink / raw) To: Stephen Smalley Cc: James Morris, Eric Paris, Nick Piggin, Eric W. Biederman, linux-kernel, linux-security-module On Tue, Feb 1, 2011 at 5:02 PM, Stephen Smalley <sds@tycho.nsa.gov> wrote: > Is this patch really from Eric or just derived from an earlier patch by him? No, sorry for the confusion. I seem to have triggered a git send-email bug. >> Signed-off-by: Eric W. Biederman <ebiederm@xmission.com> > > And did Eric truly sign off on this patch or just on an earlier one? Just the earlier one. I added his sign-off because of this paragraph in SubmittingPatches: | The Signed-off-by: tag indicates that the signer was involved in the | development of the patch, or that he/she was in the patch's delivery path. > >> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c >> index e276eb4..7c5dfb1 100644 >> --- a/security/selinux/hooks.c >> +++ b/security/selinux/hooks.c >> @@ -1317,9 +1311,9 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent >> >> if ((sbsec->flags & SE_SBPROC) && !S_ISLNK(inode->i_mode)) { >> struct proc_inode *proci = PROC_I(inode); >> - if (proci->pde) { >> + if (opt_dentry && (proci->pde || proci->sysctl)) { >> isec->sclass = inode_mode_to_security_class(inode->i_mode); >> - rc = selinux_proc_get_sid(proci->pde, >> + rc = selinux_proc_get_sid(opt_dentry, >> isec->sclass, >> &sid); >> if (rc) > > It would be nice if we could eliminate the last remaining piece of proc > internal knowledge from this code - why do we need the proci->pde || > proci->sysctl test here? What changes without it? Without we label all nodes in /proc/ through selinux_proc_get_sid. /proc/1/limits should not get it's sid from here, but from security_task_to_inode -> selinux_task_to_inode. Without the check we send "/1/limits" to selinux_proc_get_sid, which strips off "/1" leaving "/limits". This will be labeled with "proc_t" IIRC. -- . ..: Lucian ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] security/selinux: fix /proc/sys/ labeling 2011-02-01 15:53 ` Lucian Adrian Grijincu @ 2011-02-01 15:59 ` Stephen Smalley 2011-02-01 16:32 ` Lucian Adrian Grijincu 0 siblings, 1 reply; 20+ messages in thread From: Stephen Smalley @ 2011-02-01 15:59 UTC (permalink / raw) To: Lucian Adrian Grijincu Cc: James Morris, Eric Paris, Nick Piggin, Eric W. Biederman, linux-kernel, linux-security-module On Tue, 2011-02-01 at 17:53 +0200, Lucian Adrian Grijincu wrote: > On Tue, Feb 1, 2011 at 5:02 PM, Stephen Smalley <sds@tycho.nsa.gov> wrote: > > Is this patch really from Eric or just derived from an earlier patch by him? > > > No, sorry for the confusion. > I seem to have triggered a git send-email bug. > > >> Signed-off-by: Eric W. Biederman <ebiederm@xmission.com> > > > > And did Eric truly sign off on this patch or just on an earlier one? > > > Just the earlier one. I added his sign-off because of this paragraph > in SubmittingPatches: > | The Signed-off-by: tag indicates that the signer was involved in the > | development of the patch, or that he/she was in the patch's delivery path. > > > > >> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > >> index e276eb4..7c5dfb1 100644 > >> --- a/security/selinux/hooks.c > >> +++ b/security/selinux/hooks.c > >> @@ -1317,9 +1311,9 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent > >> > >> if ((sbsec->flags & SE_SBPROC) && !S_ISLNK(inode->i_mode)) { > >> struct proc_inode *proci = PROC_I(inode); > >> - if (proci->pde) { > >> + if (opt_dentry && (proci->pde || proci->sysctl)) { > >> isec->sclass = inode_mode_to_security_class(inode->i_mode); > >> - rc = selinux_proc_get_sid(proci->pde, > >> + rc = selinux_proc_get_sid(opt_dentry, > >> isec->sclass, > >> &sid); > >> if (rc) > > > > It would be nice if we could eliminate the last remaining piece of proc > > internal knowledge from this code - why do we need the proci->pde || > > proci->sysctl test here? What changes without it? > > > Without we label all nodes in /proc/ through selinux_proc_get_sid. > > /proc/1/limits should not get it's sid from here, but from > security_task_to_inode -> selinux_task_to_inode. > > Without the check we send "/1/limits" to selinux_proc_get_sid, which > strips off "/1" leaving "/limits". This will be labeled with "proc_t" > IIRC. Are you sure? Those inodes should be labeled by proc_pid_make_inode() -> security_task_to_inode() -> selinux_task_to_inode(), which will set the inode SID to match the associated task SID, and set the isec->initialized flag. Then when inode_doinit_with_dentry gets called later, it should bail immediately due to isec->initialized already being set. -- Stephen Smalley National Security Agency ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] security/selinux: fix /proc/sys/ labeling 2011-02-01 15:59 ` Stephen Smalley @ 2011-02-01 16:32 ` Lucian Adrian Grijincu 2011-02-01 16:37 ` Stephen Smalley 0 siblings, 1 reply; 20+ messages in thread From: Lucian Adrian Grijincu @ 2011-02-01 16:32 UTC (permalink / raw) To: Stephen Smalley Cc: James Morris, Eric Paris, Nick Piggin, Eric W. Biederman, linux-kernel, linux-security-module On Tue, Feb 1, 2011 at 5:59 PM, Stephen Smalley <sds@tycho.nsa.gov> wrote: >> Just the earlier one. I added his sign-off because of this paragraph >> in SubmittingPatches: >> | The Signed-off-by: tag indicates that the signer was involved in the >> | development of the patch, or that he/she was in the patch's delivery path. So should I leave Eric's sign-off here? >> Without we label all nodes in /proc/ through selinux_proc_get_sid. >> >> /proc/1/limits should not get it's sid from here, but from >> security_task_to_inode -> selinux_task_to_inode. >> >> Without the check we send "/1/limits" to selinux_proc_get_sid, which >> strips off "/1" leaving "/limits". This will be labeled with "proc_t" >> IIRC. > > Are you sure? Those inodes should be labeled by proc_pid_make_inode() > -> security_task_to_inode() -> selinux_task_to_inode(), which will set > the inode SID to match the associated task SID, and set the > isec->initialized flag. Then when inode_doinit_with_dentry gets called > later, it should bail immediately due to isec->initialized already being > set. I'll post an updated patch without those checks. I tested and 'find /proc | xargs ls -Z' said the same thing with and without those checks. I remember doing the same test yesterday and saw some differences, but I must have compared the wrong files. -- . ..: Lucian ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] security/selinux: fix /proc/sys/ labeling 2011-02-01 16:32 ` Lucian Adrian Grijincu @ 2011-02-01 16:37 ` Stephen Smalley 2011-02-01 16:42 ` [PATCH 1/2] " Lucian Adrian Grijincu 0 siblings, 1 reply; 20+ messages in thread From: Stephen Smalley @ 2011-02-01 16:37 UTC (permalink / raw) To: Lucian Adrian Grijincu Cc: James Morris, Eric Paris, Nick Piggin, Eric W. Biederman, linux-kernel, linux-security-module On Tue, 2011-02-01 at 18:32 +0200, Lucian Adrian Grijincu wrote: > On Tue, Feb 1, 2011 at 5:59 PM, Stephen Smalley <sds@tycho.nsa.gov> wrote: > >> Just the earlier one. I added his sign-off because of this paragraph > >> in SubmittingPatches: > >> | The Signed-off-by: tag indicates that the signer was involved in the > >> | development of the patch, or that he/she was in the patch's delivery path. > > > So should I leave Eric's sign-off here? I guess so, given that paragraph. > >> Without we label all nodes in /proc/ through selinux_proc_get_sid. > >> > >> /proc/1/limits should not get it's sid from here, but from > >> security_task_to_inode -> selinux_task_to_inode. > >> > >> Without the check we send "/1/limits" to selinux_proc_get_sid, which > >> strips off "/1" leaving "/limits". This will be labeled with "proc_t" > >> IIRC. > > > > Are you sure? Those inodes should be labeled by proc_pid_make_inode() > > -> security_task_to_inode() -> selinux_task_to_inode(), which will set > > the inode SID to match the associated task SID, and set the > > isec->initialized flag. Then when inode_doinit_with_dentry gets called > > later, it should bail immediately due to isec->initialized already being > > set. > > > > I'll post an updated patch without those checks. I tested and 'find > /proc | xargs ls -Z' said the same thing with and without those > checks. > > I remember doing the same test yesterday and saw some differences, but > I must have compared the wrong files. Ok, good. That gets rid of the last vestige of proc implementation details in selinux. -- Stephen Smalley National Security Agency ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 1/2] security/selinux: fix /proc/sys/ labeling 2011-02-01 16:37 ` Stephen Smalley @ 2011-02-01 16:42 ` Lucian Adrian Grijincu 2011-02-01 16:44 ` [PATCH 2/2] security: remove unused security_sysctl hook Lucian Adrian Grijincu ` (3 more replies) 0 siblings, 4 replies; 20+ messages in thread From: Lucian Adrian Grijincu @ 2011-02-01 16:42 UTC (permalink / raw) To: Stephen Smalley, James Morris, Eric Paris, Eric W. Biederman, linux-kernel, linux-security-module Cc: Lucian Adrian Grijincu This fixes an old (2007) selinux regression: filesystem labeling for /proc/sys returned -r--r--r-- unknown /proc/sys/fs/file-nr instead of -r--r--r-- system_u:object_r:sysctl_fs_t:s0 /proc/sys/fs/file-nr Events that lead to breaking of /proc/sys/ selinux labeling: 1) sysctl was reimplemented to route all calls through /proc/sys/ commit 77b14db502cb85a031fe8fde6c85d52f3e0acb63 [PATCH] sysctl: reimplement the sysctl proc support 2) proc_dir_entry was removed from ctl_table: commit 3fbfa98112fc3962c416452a0baf2214381030e6 [PATCH] sysctl: remove the proc_dir_entry member for the sysctl tables 3) selinux still walked the proc_dir_entry tree to apply labeling. Because ctl_tables don't have a proc_dir_entry, we did not label /proc/sys/ inodes any more. To achieve this the /proc/sys/ inodes were marked private and private inodes were ignored by selinux. commit bbaca6c2e7ef0f663bc31be4dad7cf530f6c4962 [PATCH] selinux: enhance selinux to always ignore private inodes commit 86a71dbd3e81e8870d0f0e56b87875f57e58222b [PATCH] sysctl: hide the sysctl proc inodes from selinux Access control checks have been done by means of a special sysctl hook that was called for read/write accesses to any /proc/sys/ entry. We don't have to do this because, instead of walking the proc_dir_entry tree we can walk the dentry tree (as done in this patch). With this patch: * we don't mark /proc/sys/ inodes as private * we don't need the sysclt security hook * we walk the dentry tree to find the path to the inode. We have to strip the PID in /proc/PID/ entries that have a proc_dir_entry because selinux does not know how to label paths like '/1/net/rpc/nfsd.fh' (and defaults to 'proc_t' labeling). Selinux does know of '/net/rpc/nfsd.fh' (and applies the 'sysctl_rpc_t' label). PID stripping from the path was done implicitly in the previous code because the proc_dir_entry tree had the root in '/net' in the example from above. The dentry tree has the root in '/1'. Signed-off-by: Eric W. Biederman <ebiederm@xmission.com> Signed-off-by: Lucian Adrian Grijincu <lucian.grijincu@gmail.com> --- fs/proc/proc_sysctl.c | 1 - security/selinux/hooks.c | 120 +++++++--------------------------------------- 2 files changed, 18 insertions(+), 103 deletions(-) diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c index 09a1f92..fb707e0 100644 --- a/fs/proc/proc_sysctl.c +++ b/fs/proc/proc_sysctl.c @@ -32,7 +32,6 @@ static struct inode *proc_sys_make_inode(struct super_block *sb, ei->sysctl_entry = table; inode->i_mtime = inode->i_atime = inode->i_ctime = CURRENT_TIME; - inode->i_flags |= S_PRIVATE; /* tell selinux to ignore this inode */ inode->i_mode = table->mode; if (!table->child) { inode->i_mode |= S_IFREG; diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index e276eb4..5231b95 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -43,7 +43,6 @@ #include <linux/fdtable.h> #include <linux/namei.h> #include <linux/mount.h> -#include <linux/proc_fs.h> #include <linux/netfilter_ipv4.h> #include <linux/netfilter_ipv6.h> #include <linux/tty.h> @@ -70,7 +69,6 @@ #include <net/ipv6.h> #include <linux/hugetlb.h> #include <linux/personality.h> -#include <linux/sysctl.h> #include <linux/audit.h> #include <linux/string.h> #include <linux/selinux.h> @@ -1120,39 +1118,35 @@ static inline u16 socket_type_to_security_class(int family, int type, int protoc } #ifdef CONFIG_PROC_FS -static int selinux_proc_get_sid(struct proc_dir_entry *de, +static int selinux_proc_get_sid(struct dentry *dentry, u16 tclass, u32 *sid) { - int buflen, rc; - char *buffer, *path, *end; + int rc; + char *buffer, *path; buffer = (char *)__get_free_page(GFP_KERNEL); if (!buffer) return -ENOMEM; - buflen = PAGE_SIZE; - end = buffer+buflen; - *--end = '\0'; - buflen--; - path = end-1; - *path = '/'; - while (de && de != de->parent) { - buflen -= de->namelen + 1; - if (buflen < 0) - break; - end -= de->namelen; - memcpy(end, de->name, de->namelen); - *--end = '/'; - path = end; - de = de->parent; + path = dentry_path_raw(dentry, buffer, PAGE_SIZE); + if (IS_ERR(path)) + rc = PTR_ERR(path); + else { + /* each process gets a /proc/PID/ entry. Strip off the + * PID part to get a valid selinux labeling. + * e.g. /proc/1/net/rpc/nfs -> /net/rpc/nfs */ + while (path[1] >= '0' && path[1] <= '9') { + path[1] = '/'; + path++; + } + rc = security_genfs_sid("proc", path, tclass, sid); } - rc = security_genfs_sid("proc", path, tclass, sid); free_page((unsigned long)buffer); return rc; } #else -static int selinux_proc_get_sid(struct proc_dir_entry *de, +static int selinux_proc_get_sid(struct dentry *dentry, u16 tclass, u32 *sid) { @@ -1316,10 +1310,9 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent isec->sid = sbsec->sid; if ((sbsec->flags & SE_SBPROC) && !S_ISLNK(inode->i_mode)) { - struct proc_inode *proci = PROC_I(inode); - if (proci->pde) { + if (opt_dentry) { isec->sclass = inode_mode_to_security_class(inode->i_mode); - rc = selinux_proc_get_sid(proci->pde, + rc = selinux_proc_get_sid(opt_dentry, isec->sclass, &sid); if (rc) @@ -1862,82 +1855,6 @@ static int selinux_capable(struct task_struct *tsk, const struct cred *cred, return task_has_capability(tsk, cred, cap, audit); } -static int selinux_sysctl_get_sid(ctl_table *table, u16 tclass, u32 *sid) -{ - int buflen, rc; - char *buffer, *path, *end; - - rc = -ENOMEM; - buffer = (char *)__get_free_page(GFP_KERNEL); - if (!buffer) - goto out; - - buflen = PAGE_SIZE; - end = buffer+buflen; - *--end = '\0'; - buflen--; - path = end-1; - *path = '/'; - while (table) { - const char *name = table->procname; - size_t namelen = strlen(name); - buflen -= namelen + 1; - if (buflen < 0) - goto out_free; - end -= namelen; - memcpy(end, name, namelen); - *--end = '/'; - path = end; - table = table->parent; - } - buflen -= 4; - if (buflen < 0) - goto out_free; - end -= 4; - memcpy(end, "/sys", 4); - path = end; - rc = security_genfs_sid("proc", path, tclass, sid); -out_free: - free_page((unsigned long)buffer); -out: - return rc; -} - -static int selinux_sysctl(ctl_table *table, int op) -{ - int error = 0; - u32 av; - u32 tsid, sid; - int rc; - - sid = current_sid(); - - rc = selinux_sysctl_get_sid(table, (op == 0001) ? - SECCLASS_DIR : SECCLASS_FILE, &tsid); - if (rc) { - /* Default to the well-defined sysctl SID. */ - tsid = SECINITSID_SYSCTL; - } - - /* The op values are "defined" in sysctl.c, thereby creating - * a bad coupling between this module and sysctl.c */ - if (op == 001) { - error = avc_has_perm(sid, tsid, - SECCLASS_DIR, DIR__SEARCH, NULL); - } else { - av = 0; - if (op & 004) - av |= FILE__READ; - if (op & 002) - av |= FILE__WRITE; - if (av) - error = avc_has_perm(sid, tsid, - SECCLASS_FILE, av, NULL); - } - - return error; -} - static int selinux_quotactl(int cmds, int type, int id, struct super_block *sb) { const struct cred *cred = current_cred(); @@ -5398,7 +5315,6 @@ static struct security_operations selinux_ops = { .ptrace_traceme = selinux_ptrace_traceme, .capget = selinux_capget, .capset = selinux_capset, - .sysctl = selinux_sysctl, .capable = selinux_capable, .quotactl = selinux_quotactl, .quota_on = selinux_quota_on, -- 1.7.4.rc1.7.g2cf08.dirty ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 2/2] security: remove unused security_sysctl hook 2011-02-01 16:42 ` [PATCH 1/2] " Lucian Adrian Grijincu @ 2011-02-01 16:44 ` Lucian Adrian Grijincu 2011-02-01 19:05 ` Stephen Smalley 2011-02-01 19:04 ` [PATCH 1/2] security/selinux: fix /proc/sys/ labeling Stephen Smalley ` (2 subsequent siblings) 3 siblings, 1 reply; 20+ messages in thread From: Lucian Adrian Grijincu @ 2011-02-01 16:44 UTC (permalink / raw) To: James Morris, Eric Paris, Stephen Smalley, ebiederm, linux-kernel, linux-security-module Cc: Lucian Adrian Grijincu The only user for this hook was selinux. sysctl routes every call through /proc/sys/. Selinux and other security modules use the file system checks for sysctl too, so no need for this hook any more. Signed-off-by: Lucian Adrian Grijincu <lucian.grijincu@gmail.com> --- include/linux/security.h | 13 ------------- kernel/sysctl.c | 5 ----- security/capability.c | 6 ------ security/security.c | 5 ----- 4 files changed, 0 insertions(+), 29 deletions(-) diff --git a/include/linux/security.h b/include/linux/security.h index c642bb8..e7b48dc 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -1257,12 +1257,6 @@ static inline void security_free_mnt_opts(struct security_mnt_opts *opts) * @cap contains the capability <include/linux/capability.h>. * @audit: Whether to write an audit message or not * Return 0 if the capability is granted for @tsk. - * @sysctl: - * Check permission before accessing the @table sysctl variable in the - * manner specified by @op. - * @table contains the ctl_table structure for the sysctl variable. - * @op contains the operation (001 = search, 002 = write, 004 = read). - * Return 0 if permission is granted. * @syslog: * Check permission before accessing the kernel message ring or changing * logging to the console. @@ -1383,7 +1377,6 @@ struct security_operations { const kernel_cap_t *permitted); int (*capable) (struct task_struct *tsk, const struct cred *cred, int cap, int audit); - int (*sysctl) (struct ctl_table *table, int op); int (*quotactl) (int cmds, int type, int id, struct super_block *sb); int (*quota_on) (struct dentry *dentry); int (*syslog) (int type); @@ -1665,7 +1658,6 @@ int security_capset(struct cred *new, const struct cred *old, int security_capable(int cap); int security_real_capable(struct task_struct *tsk, int cap); int security_real_capable_noaudit(struct task_struct *tsk, int cap); -int security_sysctl(struct ctl_table *table, int op); int security_quotactl(int cmds, int type, int id, struct super_block *sb); int security_quota_on(struct dentry *dentry); int security_syslog(int type); @@ -1883,11 +1875,6 @@ int security_real_capable_noaudit(struct task_struct *tsk, int cap) return ret; } -static inline int security_sysctl(struct ctl_table *table, int op) -{ - return 0; -} - static inline int security_quotactl(int cmds, int type, int id, struct super_block *sb) { diff --git a/kernel/sysctl.c b/kernel/sysctl.c index 0f1bd83..56f6fc1 100644 --- a/kernel/sysctl.c +++ b/kernel/sysctl.c @@ -1685,13 +1685,8 @@ static int test_perm(int mode, int op) int sysctl_perm(struct ctl_table_root *root, struct ctl_table *table, int op) { - int error; int mode; - error = security_sysctl(table, op & (MAY_READ | MAY_WRITE | MAY_EXEC)); - if (error) - return error; - if (root->permissions) mode = root->permissions(root, current->nsproxy, table); else diff --git a/security/capability.c b/security/capability.c index 2a5df2b..ebe3b5d 100644 --- a/security/capability.c +++ b/security/capability.c @@ -12,11 +12,6 @@ #include <linux/security.h> -static int cap_sysctl(ctl_table *table, int op) -{ - return 0; -} - static int cap_syslog(int type) { return 0; @@ -880,7 +875,6 @@ void __init security_fixup_ops(struct security_operations *ops) set_to_cap_if_null(ops, capable); set_to_cap_if_null(ops, quotactl); set_to_cap_if_null(ops, quota_on); - set_to_cap_if_null(ops, sysctl); set_to_cap_if_null(ops, syslog); set_to_cap_if_null(ops, settime); set_to_cap_if_null(ops, vm_enough_memory); diff --git a/security/security.c b/security/security.c index 739e403..53d793a 100644 --- a/security/security.c +++ b/security/security.c @@ -182,11 +182,6 @@ int security_real_capable_noaudit(struct task_struct *tsk, int cap) return ret; } -int security_sysctl(struct ctl_table *table, int op) -{ - return security_ops->sysctl(table, op); -} - int security_quotactl(int cmds, int type, int id, struct super_block *sb) { return security_ops->quotactl(cmds, type, id, sb); -- 1.7.4.rc1.7.g2cf08.dirty ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] security: remove unused security_sysctl hook 2011-02-01 16:44 ` [PATCH 2/2] security: remove unused security_sysctl hook Lucian Adrian Grijincu @ 2011-02-01 19:05 ` Stephen Smalley 2011-02-01 20:06 ` Eric Paris 0 siblings, 1 reply; 20+ messages in thread From: Stephen Smalley @ 2011-02-01 19:05 UTC (permalink / raw) To: Lucian Adrian Grijincu Cc: James Morris, Eric Paris, ebiederm, linux-kernel, linux-security-module On Tue, 2011-02-01 at 18:44 +0200, Lucian Adrian Grijincu wrote: > The only user for this hook was selinux. sysctl routes every call > through /proc/sys/. Selinux and other security modules use the file > system checks for sysctl too, so no need for this hook any more. > > Signed-off-by: Lucian Adrian Grijincu <lucian.grijincu@gmail.com> Acked-by: Stephen Smalley <sds@tycho.nsa.gov> > --- > include/linux/security.h | 13 ------------- > kernel/sysctl.c | 5 ----- > security/capability.c | 6 ------ > security/security.c | 5 ----- > 4 files changed, 0 insertions(+), 29 deletions(-) > > diff --git a/include/linux/security.h b/include/linux/security.h > index c642bb8..e7b48dc 100644 > --- a/include/linux/security.h > +++ b/include/linux/security.h > @@ -1257,12 +1257,6 @@ static inline void security_free_mnt_opts(struct security_mnt_opts *opts) > * @cap contains the capability <include/linux/capability.h>. > * @audit: Whether to write an audit message or not > * Return 0 if the capability is granted for @tsk. > - * @sysctl: > - * Check permission before accessing the @table sysctl variable in the > - * manner specified by @op. > - * @table contains the ctl_table structure for the sysctl variable. > - * @op contains the operation (001 = search, 002 = write, 004 = read). > - * Return 0 if permission is granted. > * @syslog: > * Check permission before accessing the kernel message ring or changing > * logging to the console. > @@ -1383,7 +1377,6 @@ struct security_operations { > const kernel_cap_t *permitted); > int (*capable) (struct task_struct *tsk, const struct cred *cred, > int cap, int audit); > - int (*sysctl) (struct ctl_table *table, int op); > int (*quotactl) (int cmds, int type, int id, struct super_block *sb); > int (*quota_on) (struct dentry *dentry); > int (*syslog) (int type); > @@ -1665,7 +1658,6 @@ int security_capset(struct cred *new, const struct cred *old, > int security_capable(int cap); > int security_real_capable(struct task_struct *tsk, int cap); > int security_real_capable_noaudit(struct task_struct *tsk, int cap); > -int security_sysctl(struct ctl_table *table, int op); > int security_quotactl(int cmds, int type, int id, struct super_block *sb); > int security_quota_on(struct dentry *dentry); > int security_syslog(int type); > @@ -1883,11 +1875,6 @@ int security_real_capable_noaudit(struct task_struct *tsk, int cap) > return ret; > } > > -static inline int security_sysctl(struct ctl_table *table, int op) > -{ > - return 0; > -} > - > static inline int security_quotactl(int cmds, int type, int id, > struct super_block *sb) > { > diff --git a/kernel/sysctl.c b/kernel/sysctl.c > index 0f1bd83..56f6fc1 100644 > --- a/kernel/sysctl.c > +++ b/kernel/sysctl.c > @@ -1685,13 +1685,8 @@ static int test_perm(int mode, int op) > > int sysctl_perm(struct ctl_table_root *root, struct ctl_table *table, int op) > { > - int error; > int mode; > > - error = security_sysctl(table, op & (MAY_READ | MAY_WRITE | MAY_EXEC)); > - if (error) > - return error; > - > if (root->permissions) > mode = root->permissions(root, current->nsproxy, table); > else > diff --git a/security/capability.c b/security/capability.c > index 2a5df2b..ebe3b5d 100644 > --- a/security/capability.c > +++ b/security/capability.c > @@ -12,11 +12,6 @@ > > #include <linux/security.h> > > -static int cap_sysctl(ctl_table *table, int op) > -{ > - return 0; > -} > - > static int cap_syslog(int type) > { > return 0; > @@ -880,7 +875,6 @@ void __init security_fixup_ops(struct security_operations *ops) > set_to_cap_if_null(ops, capable); > set_to_cap_if_null(ops, quotactl); > set_to_cap_if_null(ops, quota_on); > - set_to_cap_if_null(ops, sysctl); > set_to_cap_if_null(ops, syslog); > set_to_cap_if_null(ops, settime); > set_to_cap_if_null(ops, vm_enough_memory); > diff --git a/security/security.c b/security/security.c > index 739e403..53d793a 100644 > --- a/security/security.c > +++ b/security/security.c > @@ -182,11 +182,6 @@ int security_real_capable_noaudit(struct task_struct *tsk, int cap) > return ret; > } > > -int security_sysctl(struct ctl_table *table, int op) > -{ > - return security_ops->sysctl(table, op); > -} > - > int security_quotactl(int cmds, int type, int id, struct super_block *sb) > { > return security_ops->quotactl(cmds, type, id, sb); > -- > 1.7.4.rc1.7.g2cf08.dirty -- Stephen Smalley National Security Agency ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] security: remove unused security_sysctl hook 2011-02-01 19:05 ` Stephen Smalley @ 2011-02-01 20:06 ` Eric Paris 2011-02-14 19:33 ` Lucian Adrian Grijincu 0 siblings, 1 reply; 20+ messages in thread From: Eric Paris @ 2011-02-01 20:06 UTC (permalink / raw) To: Stephen Smalley Cc: Lucian Adrian Grijincu, James Morris, ebiederm, linux-kernel, linux-security-module On Feb 1, 2011, at 2:05 PM, Stephen Smalley <sds@tycho.nsa.gov> wrote: > On Tue, 2011-02-01 at 18:44 +0200, Lucian Adrian Grijincu wrote: >> The only user for this hook was selinux. sysctl routes every call >> through /proc/sys/. Selinux and other security modules use the file >> system checks for sysctl too, so no need for this hook any more. >> >> Signed-off-by: Lucian Adrian Grijincu <lucian.grijincu@gmail.com> > > Acked-by: Stephen Smalley <sds@tycho.nsa.gov> I've applied both to the selinux tree. Thanks -Eric > >> --- >> include/linux/security.h | 13 ------------- >> kernel/sysctl.c | 5 ----- >> security/capability.c | 6 ------ >> security/security.c | 5 ----- >> 4 files changed, 0 insertions(+), 29 deletions(-) >> >> diff --git a/include/linux/security.h b/include/linux/security.h >> index c642bb8..e7b48dc 100644 >> --- a/include/linux/security.h >> +++ b/include/linux/security.h >> @@ -1257,12 +1257,6 @@ static inline void security_free_mnt_opts(struct security_mnt_opts *opts) >> * @cap contains the capability <include/linux/capability.h>. >> * @audit: Whether to write an audit message or not >> * Return 0 if the capability is granted for @tsk. >> - * @sysctl: >> - * Check permission before accessing the @table sysctl variable in the >> - * manner specified by @op. >> - * @table contains the ctl_table structure for the sysctl variable. >> - * @op contains the operation (001 = search, 002 = write, 004 = read). >> - * Return 0 if permission is granted. >> * @syslog: >> * Check permission before accessing the kernel message ring or changing >> * logging to the console. >> @@ -1383,7 +1377,6 @@ struct security_operations { >> const kernel_cap_t *permitted); >> int (*capable) (struct task_struct *tsk, const struct cred *cred, >> int cap, int audit); >> - int (*sysctl) (struct ctl_table *table, int op); >> int (*quotactl) (int cmds, int type, int id, struct super_block *sb); >> int (*quota_on) (struct dentry *dentry); >> int (*syslog) (int type); >> @@ -1665,7 +1658,6 @@ int security_capset(struct cred *new, const struct cred *old, >> int security_capable(int cap); >> int security_real_capable(struct task_struct *tsk, int cap); >> int security_real_capable_noaudit(struct task_struct *tsk, int cap); >> -int security_sysctl(struct ctl_table *table, int op); >> int security_quotactl(int cmds, int type, int id, struct super_block *sb); >> int security_quota_on(struct dentry *dentry); >> int security_syslog(int type); >> @@ -1883,11 +1875,6 @@ int security_real_capable_noaudit(struct task_struct *tsk, int cap) >> return ret; >> } >> >> -static inline int security_sysctl(struct ctl_table *table, int op) >> -{ >> - return 0; >> -} >> - >> static inline int security_quotactl(int cmds, int type, int id, >> struct super_block *sb) >> { >> diff --git a/kernel/sysctl.c b/kernel/sysctl.c >> index 0f1bd83..56f6fc1 100644 >> --- a/kernel/sysctl.c >> +++ b/kernel/sysctl.c >> @@ -1685,13 +1685,8 @@ static int test_perm(int mode, int op) >> >> int sysctl_perm(struct ctl_table_root *root, struct ctl_table *table, int op) >> { >> - int error; >> int mode; >> >> - error = security_sysctl(table, op & (MAY_READ | MAY_WRITE | MAY_EXEC)); >> - if (error) >> - return error; >> - >> if (root->permissions) >> mode = root->permissions(root, current->nsproxy, table); >> else >> diff --git a/security/capability.c b/security/capability.c >> index 2a5df2b..ebe3b5d 100644 >> --- a/security/capability.c >> +++ b/security/capability.c >> @@ -12,11 +12,6 @@ >> >> #include <linux/security.h> >> >> -static int cap_sysctl(ctl_table *table, int op) >> -{ >> - return 0; >> -} >> - >> static int cap_syslog(int type) >> { >> return 0; >> @@ -880,7 +875,6 @@ void __init security_fixup_ops(struct security_operations *ops) >> set_to_cap_if_null(ops, capable); >> set_to_cap_if_null(ops, quotactl); >> set_to_cap_if_null(ops, quota_on); >> - set_to_cap_if_null(ops, sysctl); >> set_to_cap_if_null(ops, syslog); >> set_to_cap_if_null(ops, settime); >> set_to_cap_if_null(ops, vm_enough_memory); >> diff --git a/security/security.c b/security/security.c >> index 739e403..53d793a 100644 >> --- a/security/security.c >> +++ b/security/security.c >> @@ -182,11 +182,6 @@ int security_real_capable_noaudit(struct task_struct *tsk, int cap) >> return ret; >> } >> >> -int security_sysctl(struct ctl_table *table, int op) >> -{ >> - return security_ops->sysctl(table, op); >> -} >> - >> int security_quotactl(int cmds, int type, int id, struct super_block *sb) >> { >> return security_ops->quotactl(cmds, type, id, sb); >> -- >> 1.7.4.rc1.7.g2cf08.dirty > > -- > Stephen Smalley > National Security Agency > ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] security: remove unused security_sysctl hook 2011-02-01 20:06 ` Eric Paris @ 2011-02-14 19:33 ` Lucian Adrian Grijincu 2011-02-14 19:53 ` Eric Paris 0 siblings, 1 reply; 20+ messages in thread From: Lucian Adrian Grijincu @ 2011-02-14 19:33 UTC (permalink / raw) To: Eric Paris Cc: Stephen Smalley, James Morris, ebiederm, linux-kernel, linux-security-module On Tue, Feb 1, 2011 at 10:06 PM, Eric Paris <eparis@redhat.com> wrote: > > > > > On Feb 1, 2011, at 2:05 PM, Stephen Smalley <sds@tycho.nsa.gov> wrote: > >> On Tue, 2011-02-01 at 18:44 +0200, Lucian Adrian Grijincu wrote: >>> The only user for this hook was selinux. sysctl routes every call >>> through /proc/sys/. Selinux and other security modules use the file >>> system checks for sysctl too, so no need for this hook any more. >>> >>> Signed-off-by: Lucian Adrian Grijincu <lucian.grijincu@gmail.com> >> >> Acked-by: Stephen Smalley <sds@tycho.nsa.gov> > > I've applied both to the selinux tree. Thanks I've checked both these trees (on all published branches): git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/security-testing-2.6.git git://git.infradead.org/users/eparis/selinux.git and there's no trace of these patches. Am I not looking in the right places for these patches? -- . ..: Lucian ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] security: remove unused security_sysctl hook 2011-02-14 19:33 ` Lucian Adrian Grijincu @ 2011-02-14 19:53 ` Eric Paris 2011-02-14 20:06 ` Lucian Adrian Grijincu 0 siblings, 1 reply; 20+ messages in thread From: Eric Paris @ 2011-02-14 19:53 UTC (permalink / raw) To: Lucian Adrian Grijincu Cc: Stephen Smalley, James Morris, ebiederm, linux-kernel, linux-security-module On Mon, 2011-02-14 at 21:33 +0200, Lucian Adrian Grijincu wrote: > On Tue, Feb 1, 2011 at 10:06 PM, Eric Paris <eparis@redhat.com> wrote: > > > > > > > > > > On Feb 1, 2011, at 2:05 PM, Stephen Smalley <sds@tycho.nsa.gov> wrote: > > > >> On Tue, 2011-02-01 at 18:44 +0200, Lucian Adrian Grijincu wrote: > >>> The only user for this hook was selinux. sysctl routes every call > >>> through /proc/sys/. Selinux and other security modules use the file > >>> system checks for sysctl too, so no need for this hook any more. > >>> > >>> Signed-off-by: Lucian Adrian Grijincu <lucian.grijincu@gmail.com> > >> > >> Acked-by: Stephen Smalley <sds@tycho.nsa.gov> > > > > I've applied both to the selinux tree. Thanks > > > I've checked both these trees (on all published branches): > git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/security-testing-2.6.git > git://git.infradead.org/users/eparis/selinux.git > > and there's no trace of these patches. > > > Am I not looking in the right places for these patches? The second one is the right tree, I just forgot to push! Pushed now! Sorry. -Eric ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] security: remove unused security_sysctl hook 2011-02-14 19:53 ` Eric Paris @ 2011-02-14 20:06 ` Lucian Adrian Grijincu 2011-02-14 22:06 ` James Morris 0 siblings, 1 reply; 20+ messages in thread From: Lucian Adrian Grijincu @ 2011-02-14 20:06 UTC (permalink / raw) To: Eric Paris Cc: Stephen Smalley, James Morris, ebiederm, linux-kernel, linux-security-module On Mon, Feb 14, 2011 at 9:53 PM, Eric Paris <eparis@redhat.com> wrote: >> >> Acked-by: Stephen Smalley <sds@tycho.nsa.gov> FWIW you forgot Stephen's Acked-by. >> I've checked both these trees (on all published branches): >> git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/security-testing-2.6.git >> git://git.infradead.org/users/eparis/selinux.git >> >> and there's no trace of these patches. >> >> >> Am I not looking in the right places for these patches? > > The second one is the right tree, I just forgot to push! Pushed now! MAINTAINERS says: SELINUX SECURITY MODULE T: git git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/security-testing-2.6.git Should It be changed to point to your tree? Or should a new entry be added to point to your tree? -- . ..: Lucian ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] security: remove unused security_sysctl hook 2011-02-14 20:06 ` Lucian Adrian Grijincu @ 2011-02-14 22:06 ` James Morris 0 siblings, 0 replies; 20+ messages in thread From: James Morris @ 2011-02-14 22:06 UTC (permalink / raw) To: Lucian Adrian Grijincu Cc: Eric Paris, Stephen Smalley, ebiederm, linux-kernel, linux-security-module On Mon, 14 Feb 2011, Lucian Adrian Grijincu wrote: > > MAINTAINERS says: > SELINUX SECURITY MODULE > T: git git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/security-testing-2.6.git > > Should It be changed to point to your tree? Or should a new entry be > added to point to your tree? That should probably be changed to Eric's tree. -- James Morris <jmorris@namei.org> ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] security/selinux: fix /proc/sys/ labeling 2011-02-01 16:42 ` [PATCH 1/2] " Lucian Adrian Grijincu 2011-02-01 16:44 ` [PATCH 2/2] security: remove unused security_sysctl hook Lucian Adrian Grijincu @ 2011-02-01 19:04 ` Stephen Smalley 2011-02-01 19:33 ` Eric W. Biederman 2011-02-01 19:33 ` Eric W. Biederman 3 siblings, 0 replies; 20+ messages in thread From: Stephen Smalley @ 2011-02-01 19:04 UTC (permalink / raw) To: Lucian Adrian Grijincu Cc: James Morris, Eric Paris, Eric W. Biederman, linux-kernel, linux-security-module On Tue, 2011-02-01 at 18:42 +0200, Lucian Adrian Grijincu wrote: > This fixes an old (2007) selinux regression: filesystem labeling for > /proc/sys returned > -r--r--r-- unknown /proc/sys/fs/file-nr > instead of > -r--r--r-- system_u:object_r:sysctl_fs_t:s0 /proc/sys/fs/file-nr > > Events that lead to breaking of /proc/sys/ selinux labeling: > > 1) sysctl was reimplemented to route all calls through /proc/sys/ > > commit 77b14db502cb85a031fe8fde6c85d52f3e0acb63 > [PATCH] sysctl: reimplement the sysctl proc support > > 2) proc_dir_entry was removed from ctl_table: > > commit 3fbfa98112fc3962c416452a0baf2214381030e6 > [PATCH] sysctl: remove the proc_dir_entry member for the sysctl tables > > 3) selinux still walked the proc_dir_entry tree to apply > labeling. Because ctl_tables don't have a proc_dir_entry, we did > not label /proc/sys/ inodes any more. To achieve this the /proc/sys/ > inodes were marked private and private inodes were ignored by > selinux. > > commit bbaca6c2e7ef0f663bc31be4dad7cf530f6c4962 > [PATCH] selinux: enhance selinux to always ignore private inodes > > commit 86a71dbd3e81e8870d0f0e56b87875f57e58222b > [PATCH] sysctl: hide the sysctl proc inodes from selinux > > Access control checks have been done by means of a special sysctl hook > that was called for read/write accesses to any /proc/sys/ entry. > > We don't have to do this because, instead of walking the > proc_dir_entry tree we can walk the dentry tree (as done in this > patch). With this patch: > * we don't mark /proc/sys/ inodes as private > * we don't need the sysclt security hook > * we walk the dentry tree to find the path to the inode. > > We have to strip the PID in /proc/PID/ entries that have a > proc_dir_entry because selinux does not know how to label paths like > '/1/net/rpc/nfsd.fh' (and defaults to 'proc_t' labeling). Selinux does > know of '/net/rpc/nfsd.fh' (and applies the 'sysctl_rpc_t' label). > > PID stripping from the path was done implicitly in the previous code > because the proc_dir_entry tree had the root in '/net' in the example > from above. The dentry tree has the root in '/1'. > > Signed-off-by: Eric W. Biederman <ebiederm@xmission.com> > Signed-off-by: Lucian Adrian Grijincu <lucian.grijincu@gmail.com> Acked-by: Stephen Smalley <sds@tycho.nsa.gov> > --- > fs/proc/proc_sysctl.c | 1 - > security/selinux/hooks.c | 120 +++++++--------------------------------------- > 2 files changed, 18 insertions(+), 103 deletions(-) > > diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c > index 09a1f92..fb707e0 100644 > --- a/fs/proc/proc_sysctl.c > +++ b/fs/proc/proc_sysctl.c > @@ -32,7 +32,6 @@ static struct inode *proc_sys_make_inode(struct super_block *sb, > ei->sysctl_entry = table; > > inode->i_mtime = inode->i_atime = inode->i_ctime = CURRENT_TIME; > - inode->i_flags |= S_PRIVATE; /* tell selinux to ignore this inode */ > inode->i_mode = table->mode; > if (!table->child) { > inode->i_mode |= S_IFREG; > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index e276eb4..5231b95 100644 > --- a/security/selinux/hooks.c > +++ b/security/selinux/hooks.c > @@ -43,7 +43,6 @@ > #include <linux/fdtable.h> > #include <linux/namei.h> > #include <linux/mount.h> > -#include <linux/proc_fs.h> > #include <linux/netfilter_ipv4.h> > #include <linux/netfilter_ipv6.h> > #include <linux/tty.h> > @@ -70,7 +69,6 @@ > #include <net/ipv6.h> > #include <linux/hugetlb.h> > #include <linux/personality.h> > -#include <linux/sysctl.h> > #include <linux/audit.h> > #include <linux/string.h> > #include <linux/selinux.h> > @@ -1120,39 +1118,35 @@ static inline u16 socket_type_to_security_class(int family, int type, int protoc > } > > #ifdef CONFIG_PROC_FS > -static int selinux_proc_get_sid(struct proc_dir_entry *de, > +static int selinux_proc_get_sid(struct dentry *dentry, > u16 tclass, > u32 *sid) > { > - int buflen, rc; > - char *buffer, *path, *end; > + int rc; > + char *buffer, *path; > > buffer = (char *)__get_free_page(GFP_KERNEL); > if (!buffer) > return -ENOMEM; > > - buflen = PAGE_SIZE; > - end = buffer+buflen; > - *--end = '\0'; > - buflen--; > - path = end-1; > - *path = '/'; > - while (de && de != de->parent) { > - buflen -= de->namelen + 1; > - if (buflen < 0) > - break; > - end -= de->namelen; > - memcpy(end, de->name, de->namelen); > - *--end = '/'; > - path = end; > - de = de->parent; > + path = dentry_path_raw(dentry, buffer, PAGE_SIZE); > + if (IS_ERR(path)) > + rc = PTR_ERR(path); > + else { > + /* each process gets a /proc/PID/ entry. Strip off the > + * PID part to get a valid selinux labeling. > + * e.g. /proc/1/net/rpc/nfs -> /net/rpc/nfs */ > + while (path[1] >= '0' && path[1] <= '9') { > + path[1] = '/'; > + path++; > + } > + rc = security_genfs_sid("proc", path, tclass, sid); > } > - rc = security_genfs_sid("proc", path, tclass, sid); > free_page((unsigned long)buffer); > return rc; > } > #else > -static int selinux_proc_get_sid(struct proc_dir_entry *de, > +static int selinux_proc_get_sid(struct dentry *dentry, > u16 tclass, > u32 *sid) > { > @@ -1316,10 +1310,9 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent > isec->sid = sbsec->sid; > > if ((sbsec->flags & SE_SBPROC) && !S_ISLNK(inode->i_mode)) { > - struct proc_inode *proci = PROC_I(inode); > - if (proci->pde) { > + if (opt_dentry) { > isec->sclass = inode_mode_to_security_class(inode->i_mode); > - rc = selinux_proc_get_sid(proci->pde, > + rc = selinux_proc_get_sid(opt_dentry, > isec->sclass, > &sid); > if (rc) > @@ -1862,82 +1855,6 @@ static int selinux_capable(struct task_struct *tsk, const struct cred *cred, > return task_has_capability(tsk, cred, cap, audit); > } > > -static int selinux_sysctl_get_sid(ctl_table *table, u16 tclass, u32 *sid) > -{ > - int buflen, rc; > - char *buffer, *path, *end; > - > - rc = -ENOMEM; > - buffer = (char *)__get_free_page(GFP_KERNEL); > - if (!buffer) > - goto out; > - > - buflen = PAGE_SIZE; > - end = buffer+buflen; > - *--end = '\0'; > - buflen--; > - path = end-1; > - *path = '/'; > - while (table) { > - const char *name = table->procname; > - size_t namelen = strlen(name); > - buflen -= namelen + 1; > - if (buflen < 0) > - goto out_free; > - end -= namelen; > - memcpy(end, name, namelen); > - *--end = '/'; > - path = end; > - table = table->parent; > - } > - buflen -= 4; > - if (buflen < 0) > - goto out_free; > - end -= 4; > - memcpy(end, "/sys", 4); > - path = end; > - rc = security_genfs_sid("proc", path, tclass, sid); > -out_free: > - free_page((unsigned long)buffer); > -out: > - return rc; > -} > - > -static int selinux_sysctl(ctl_table *table, int op) > -{ > - int error = 0; > - u32 av; > - u32 tsid, sid; > - int rc; > - > - sid = current_sid(); > - > - rc = selinux_sysctl_get_sid(table, (op == 0001) ? > - SECCLASS_DIR : SECCLASS_FILE, &tsid); > - if (rc) { > - /* Default to the well-defined sysctl SID. */ > - tsid = SECINITSID_SYSCTL; > - } > - > - /* The op values are "defined" in sysctl.c, thereby creating > - * a bad coupling between this module and sysctl.c */ > - if (op == 001) { > - error = avc_has_perm(sid, tsid, > - SECCLASS_DIR, DIR__SEARCH, NULL); > - } else { > - av = 0; > - if (op & 004) > - av |= FILE__READ; > - if (op & 002) > - av |= FILE__WRITE; > - if (av) > - error = avc_has_perm(sid, tsid, > - SECCLASS_FILE, av, NULL); > - } > - > - return error; > -} > - > static int selinux_quotactl(int cmds, int type, int id, struct super_block *sb) > { > const struct cred *cred = current_cred(); > @@ -5398,7 +5315,6 @@ static struct security_operations selinux_ops = { > .ptrace_traceme = selinux_ptrace_traceme, > .capget = selinux_capget, > .capset = selinux_capset, > - .sysctl = selinux_sysctl, > .capable = selinux_capable, > .quotactl = selinux_quotactl, > .quota_on = selinux_quota_on, -- Stephen Smalley National Security Agency ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] security/selinux: fix /proc/sys/ labeling 2011-02-01 16:42 ` [PATCH 1/2] " Lucian Adrian Grijincu 2011-02-01 16:44 ` [PATCH 2/2] security: remove unused security_sysctl hook Lucian Adrian Grijincu 2011-02-01 19:04 ` [PATCH 1/2] security/selinux: fix /proc/sys/ labeling Stephen Smalley @ 2011-02-01 19:33 ` Eric W. Biederman 2011-02-01 19:33 ` Eric W. Biederman 3 siblings, 0 replies; 20+ messages in thread From: Eric W. Biederman @ 2011-02-01 19:33 UTC (permalink / raw) To: Lucian Adrian Grijincu Cc: Stephen Smalley, James Morris, Eric Paris, linux-kernel, linux-security-module Lucian Adrian Grijincu <lucian.grijincu@gmail.com> writes: > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index e276eb4..5231b95 100644 > --- a/security/selinux/hooks.c > +++ b/security/selinux/hooks.c > @@ -43,7 +43,6 @@ > #include <linux/fdtable.h> > #include <linux/namei.h> > #include <linux/mount.h> > -#include <linux/proc_fs.h> > #include <linux/netfilter_ipv4.h> > #include <linux/netfilter_ipv6.h> > #include <linux/tty.h> > @@ -70,7 +69,6 @@ > #include <net/ipv6.h> > #include <linux/hugetlb.h> > #include <linux/personality.h> > -#include <linux/sysctl.h> > #include <linux/audit.h> > #include <linux/string.h> > #include <linux/selinux.h> > @@ -1120,39 +1118,35 @@ static inline u16 socket_type_to_security_class(int family, int type, int protoc > } > > #ifdef CONFIG_PROC_FS > -static int selinux_proc_get_sid(struct proc_dir_entry *de, > +static int selinux_proc_get_sid(struct dentry *dentry, > u16 tclass, > u32 *sid) > { > - int buflen, rc; > - char *buffer, *path, *end; > + int rc; > + char *buffer, *path; > > buffer = (char *)__get_free_page(GFP_KERNEL); > if (!buffer) > return -ENOMEM; > > - buflen = PAGE_SIZE; > - end = buffer+buflen; > - *--end = '\0'; > - buflen--; > - path = end-1; > - *path = '/'; > - while (de && de != de->parent) { > - buflen -= de->namelen + 1; > - if (buflen < 0) > - break; > - end -= de->namelen; > - memcpy(end, de->name, de->namelen); > - *--end = '/'; > - path = end; > - de = de->parent; > + path = dentry_path_raw(dentry, buffer, PAGE_SIZE); What kernel has a dentry_path_raw? Perhaps you mean __dentry_path? > + if (IS_ERR(path)) > + rc = PTR_ERR(path); > + else { > + /* each process gets a /proc/PID/ entry. Strip off the > + * PID part to get a valid selinux labeling. > + * e.g. /proc/1/net/rpc/nfs -> /net/rpc/nfs */ > + while (path[1] >= '0' && path[1] <= '9') { > + path[1] = '/'; > + path++; > + } > + rc = security_genfs_sid("proc", path, tclass, sid); > } > - rc = security_genfs_sid("proc", path, tclass, sid); > free_page((unsigned long)buffer); > return rc; > } > #else Eric ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] security/selinux: fix /proc/sys/ labeling 2011-02-01 16:42 ` [PATCH 1/2] " Lucian Adrian Grijincu ` (2 preceding siblings ...) 2011-02-01 19:33 ` Eric W. Biederman @ 2011-02-01 19:33 ` Eric W. Biederman 2011-02-01 19:46 ` Lucian Adrian Grijincu 3 siblings, 1 reply; 20+ messages in thread From: Eric W. Biederman @ 2011-02-01 19:33 UTC (permalink / raw) To: Lucian Adrian Grijincu Cc: Stephen Smalley, James Morris, Eric Paris, linux-kernel, linux-security-module Lucian Adrian Grijincu <lucian.grijincu@gmail.com> writes: > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index e276eb4..5231b95 100644 > --- a/security/selinux/hooks.c > +++ b/security/selinux/hooks.c > @@ -43,7 +43,6 @@ > #include <linux/fdtable.h> > #include <linux/namei.h> > #include <linux/mount.h> > -#include <linux/proc_fs.h> > #include <linux/netfilter_ipv4.h> > #include <linux/netfilter_ipv6.h> > #include <linux/tty.h> > @@ -70,7 +69,6 @@ > #include <net/ipv6.h> > #include <linux/hugetlb.h> > #include <linux/personality.h> > -#include <linux/sysctl.h> > #include <linux/audit.h> > #include <linux/string.h> > #include <linux/selinux.h> > @@ -1120,39 +1118,35 @@ static inline u16 socket_type_to_security_class(int family, int type, int protoc > } > > #ifdef CONFIG_PROC_FS > -static int selinux_proc_get_sid(struct proc_dir_entry *de, > +static int selinux_proc_get_sid(struct dentry *dentry, > u16 tclass, > u32 *sid) > { > - int buflen, rc; > - char *buffer, *path, *end; > + int rc; > + char *buffer, *path; > > buffer = (char *)__get_free_page(GFP_KERNEL); > if (!buffer) > return -ENOMEM; > > - buflen = PAGE_SIZE; > - end = buffer+buflen; > - *--end = '\0'; > - buflen--; > - path = end-1; > - *path = '/'; > - while (de && de != de->parent) { > - buflen -= de->namelen + 1; > - if (buflen < 0) > - break; > - end -= de->namelen; > - memcpy(end, de->name, de->namelen); > - *--end = '/'; > - path = end; > - de = de->parent; > + path = dentry_path_raw(dentry, buffer, PAGE_SIZE); What kernel has a dentry_path_raw? Perhaps you mean __dentry_path? > + if (IS_ERR(path)) > + rc = PTR_ERR(path); > + else { > + /* each process gets a /proc/PID/ entry. Strip off the > + * PID part to get a valid selinux labeling. > + * e.g. /proc/1/net/rpc/nfs -> /net/rpc/nfs */ > + while (path[1] >= '0' && path[1] <= '9') { > + path[1] = '/'; > + path++; > + } > + rc = security_genfs_sid("proc", path, tclass, sid); > } > - rc = security_genfs_sid("proc", path, tclass, sid); > free_page((unsigned long)buffer); > return rc; > } > #else Eric ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] security/selinux: fix /proc/sys/ labeling 2011-02-01 19:33 ` Eric W. Biederman @ 2011-02-01 19:46 ` Lucian Adrian Grijincu 2011-02-01 20:14 ` Eric W. Biederman 0 siblings, 1 reply; 20+ messages in thread From: Lucian Adrian Grijincu @ 2011-02-01 19:46 UTC (permalink / raw) To: Eric W. Biederman Cc: Stephen Smalley, James Morris, Eric Paris, linux-kernel, linux-security-module On Tue, Feb 1, 2011 at 9:33 PM, Eric W. Biederman <ebiederm@xmission.com> wrote: > What kernel has a dentry_path_raw? Perhaps you mean __dentry_path? See the function here: https://github.com/mirrors/linux-2.6/blob/70d1f365568e0cdbc9f4ab92428e1830fdb09ab0/fs/dcache.c The last patches were against 2.6.38-rc2 because the dcache layer got rewritten in 2.6.38 http://lwn.net/Articles/421784/ __dentry_path is now static (in fs/dcache.c) and does not take the necessary locks. dentry_path_raw is __dentry_path with locks ec2447c278ee973d35f38e53ca16ba7f965ae33d hostfs: simplify locking Remove dcache_lock locking from hostfs filesystem, and move it into dcache helpers. All that is required is a coherent path name. Protection from concurrent modification of the namespace after path name generation is not provided in current code, because dcache_lock is dropped before the path is used. Signed-off-by: Nick Piggin <npiggin@kernel.dk> -- . ..: Lucian ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] security/selinux: fix /proc/sys/ labeling 2011-02-01 19:46 ` Lucian Adrian Grijincu @ 2011-02-01 20:14 ` Eric W. Biederman 0 siblings, 0 replies; 20+ messages in thread From: Eric W. Biederman @ 2011-02-01 20:14 UTC (permalink / raw) To: Lucian Adrian Grijincu Cc: Stephen Smalley, James Morris, Eric Paris, linux-kernel, linux-security-module Lucian Adrian Grijincu <lucian.grijincu@gmail.com> writes: > On Tue, Feb 1, 2011 at 9:33 PM, Eric W. Biederman <ebiederm@xmission.com> wrote: >> What kernel has a dentry_path_raw? Perhaps you mean __dentry_path? > > > See the function here: > https://github.com/mirrors/linux-2.6/blob/70d1f365568e0cdbc9f4ab92428e1830fdb09ab0/fs/dcache.c > > The last patches were against 2.6.38-rc2 because the dcache layer got > rewritten in 2.6.38 http://lwn.net/Articles/421784/ > > __dentry_path is now static (in fs/dcache.c) and does not take the > necessary locks. Thanks. I thought I was looking at the latest source but it turns out I fat fingered something and my tree was still 2.6.36-rc3. Sigh. dentry_path_raw does seem reasonable. Eric ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2011-02-14 22:06 UTC | newest] Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2011-02-01 0:17 [PATCH] security/selinux: fix /proc/sys/ labeling Lucian Adrian Grijincu 2011-02-01 1:32 ` [PATCH] security: remove unused security_sysctl hook Lucian Adrian Grijincu 2011-02-01 15:02 ` [PATCH] security/selinux: fix /proc/sys/ labeling Stephen Smalley 2011-02-01 15:53 ` Lucian Adrian Grijincu 2011-02-01 15:59 ` Stephen Smalley 2011-02-01 16:32 ` Lucian Adrian Grijincu 2011-02-01 16:37 ` Stephen Smalley 2011-02-01 16:42 ` [PATCH 1/2] " Lucian Adrian Grijincu 2011-02-01 16:44 ` [PATCH 2/2] security: remove unused security_sysctl hook Lucian Adrian Grijincu 2011-02-01 19:05 ` Stephen Smalley 2011-02-01 20:06 ` Eric Paris 2011-02-14 19:33 ` Lucian Adrian Grijincu 2011-02-14 19:53 ` Eric Paris 2011-02-14 20:06 ` Lucian Adrian Grijincu 2011-02-14 22:06 ` James Morris 2011-02-01 19:04 ` [PATCH 1/2] security/selinux: fix /proc/sys/ labeling Stephen Smalley 2011-02-01 19:33 ` Eric W. Biederman 2011-02-01 19:33 ` Eric W. Biederman 2011-02-01 19:46 ` Lucian Adrian Grijincu 2011-02-01 20:14 ` Eric W. Biederman
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.