On Wed, 2007-02-07 at 13:24 -0500, Stephen Smalley wrote: > On Tue, 2007-02-06 at 14:21 -0700, Eric W. Biederman wrote: > > This time instead of generating the generating the paths from > > proc_dir_entries > > generate the labels from the names in the sysctl ctl_tables themselves. > > This > > removes an unnecessary layer of indirection, allows this to work even when > > procfs support is not compiled into the kernel, and especially allows it > > to work now that ctl_tables no longer have a proc_dir_entry field. > > Thanks, looks sane. > > > I continue passing "proc" into genfs sid although that is complete nonsense > > to allow existing selinux policies to work without modification. > > > > Signed-off-by: Eric W. Biederman <[EMAIL PROTECTED]> > > Acked-by: Stephen Smalley <[EMAIL PROTECTED]>
Hmmm...but in testing the patch, I don't seem to (consistently) reach these checks when accessing via /proc/sys. I see that you are caching the mode information and using it in some cases rather than calling the sysctl_perm function. One related but separate issue is that the /proc/sys inode labeling is also affected by the sysctl patch series. Those inodes used to be labeled by selinux_proc_get_sid (from selinux_d_instantiate), but that no longer works, so they now fall back to the superblock SID (generic proc label). That changes the inode permission checks on an attempt to access a /proc/sys node and will likely cause denials under current policy for confined domains since one wouldn't generally be writing to the generic proc label. If you always called sysctl_perm from the proc sysctl code, we could possibly dispense with inode permission checking on those inodes, e.g. marking them private. > > > --- > > security/selinux/hooks.c | 43 +++++++++++++++++++++++++++++++++++++++++-- > > 1 files changed, 41 insertions(+), 2 deletions(-) > > > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > > index 3a36057..c17a8dd 100644 > > --- a/security/selinux/hooks.c > > +++ b/security/selinux/hooks.c > > @@ -1424,6 +1424,41 @@ static int selinux_capable(struct task_struct *tsk, > > int cap) > > return task_has_capability(tsk,cap); > > } > > > > +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; > > + } > > + 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; > > @@ -1438,8 +1473,12 @@ static int selinux_sysctl(ctl_table *table, int op) > > > > tsec = current->security; > > > > - /* Use the well-defined sysctl SID. */ > > - tsid = SECINITSID_SYSCTL; > > + 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 */ -- Stephen Smalley National Security Agency - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/