Re: [PATCH] proc: Use sane permission checks on the /proc//fd/ symlinks.

Andrew Morton
2006-03-03T09:06:10Z

With all your latest patches I get a big spew lateish in the boot:

EXT3-fs: INFO: recovery required on readonly filesystem.
EXT3-fs: write access will be enabled during recovery.
kjournald starting. Commit interval 5 seconds
EXT3-fs: recovery complete.
EXT3-fs: mounted filesystem with ordered data mode.
security: 3 users, 6 roles, 1135 types, 133 bools, 1 sens, 256 cats
security: 55 classes, 37666 rules
SELinux: Completing initialization.
SELinux: Setting up existing superblocks.
SELinux: initialized (dev sda6, type ext3), uses xattr
SELinux: initialized (dev tmpfs, type tmpfs), uses transition SIDs
SELinux: initialized (dev debugfs, type debugfs), uses genfs_contexts
SELinux: initialized (dev selinuxfs, type selinuxfs), uses genfs_contexts
SELinux: initialized (dev mqueue, type mqueue), uses transition SIDs
SELinux: initialized (dev hugetlbfs, type hugetlbfs), uses genfs_contexts
SELinux: initialized (dev devpts, type devpts), uses transition SIDs
SELinux: initialized (dev eventpollfs, type eventpollfs), uses genfs_contexts
SELinux: initialized (dev inotifyfs, type inotifyfs), uses genfs_contexts
SELinux: initialized (dev tmpfs, type tmpfs), uses transition SIDs
SELinux: initialized (dev futexfs, type futexfs), uses genfs_contexts
SELinux: initialized (dev pipefs, type pipefs), uses task SIDs
SELinux: initialized (dev sockfs, type sockfs), uses task SIDs
SELinux: initialized (dev proc, type proc), uses genfs_contexts
SELinux: initialized (dev bdev, type bdev), uses genfs_contexts
SELinux: initialized (dev rootfs, type rootfs), uses genfs_contexts
SELinux: initialized (dev sysfs, type sysfs), uses genfs_contexts
SELinux: initialized (dev usbfs, type usbfs), uses genfs_contexts
audit(1141341858.520:2): avc: denied { ptrace } for pid=372 comm="restorecon" scontext=system_u:system_r:restorecon_t:s0 tcontext=system_u:system_r:restorecon_t:s0 tclass=process
audit(1141341858.520:3): avc: denied { ptrace } for pid=372 comm="restorecon" scontext=system_u:system_r:restorecon_t:s0 tcontext=system_u:system_r:restorecon_t:s0 tclass=process
audit(1141341858.520:4): avc: denied { ptrace } for pid=372 comm="restorecon" scontext=system_u:system_r:restorecon_t:s0 tcontext=system_u:system_r:restorecon_t:s0 tclass=process
audit(1141341858.520:5): avc: denied { ptrace } for pid=372 comm="restorecon" scontext=system_u:system_r:restorecon_t:s0 tcontext=system_u:system_r:restorecon_t:s0 tclass=process
audit(1141341858.520:6): avc: denied { ptrace } for pid=372 comm="restorecon" scontext=system_u:system_r:restorecon_t:s0 tcontext=system_u:system_r:restorecon_t:s0 tclass=process
...
audit(1141370661.947:106): avc: denied { ptrace } for pid=380 comm="hwclock" scontext=system_u:system_r:hwclock_t:s0 tcontext=system_u:system_r:hwclock_t:s0 tclass=process
audit(1141370661.947:107): avc: denied { ptrace } for pid=380 comm="hwclock" scontext=system_u:system_r:hwclock_t:s0 tcontext=system_u:system_r:hwclock_t:s0 tclass=process
ICH6: IDE controller at PCI slot 0000:00:1f.1
ACPI: PCI Interrupt 0000:00:1f.1[B] -> GSI 18 (level, low) -> IRQ 18
ICH6: chipset revision 3
ICH6: not 100% native mode: will probe irqs later

Reverting just this patch prevents the above.

This is with basically unaltered FC5 as of a few days ago. The audit
patches weren't applied.

What is happening is that both `current' and get_proc_task(inode) are the
same task_struct: `restorecon' is trying to read its own proc files. But
ptrace_may_attach()->security_ptrace() is returning -EACCES.

So I bodged it in the obvious manner:

--- devel/fs/proc/base.c~proc-use-sane-permission-checks-on-the-proc-pid-fd-fix 2006-03-03 00:38:17.000000000 -0800
+++ devel-akpm/fs/proc/base.c 2006-03-03 00:43:54.000000000 -0800
@@ -521,8 +521,11 @@ static int proc_fd_access_allowed(struct
* allow access if we have the proper capability.
*/
task = get_proc_task(inode);
- if (task) {
+ if (task == current)
+ allowed = 1;
+ if (task && !allowed) {
int alive;
+
task_lock(task);
alive = !!task->mm;
task_unlock(task);

And the messages went away.

But I have a bad feeling about these /proc permission changes, Eric. I
suspect we'll be chasing a gradually decreasing frequency of weird problems
for a long time.

That task_lock() you have in proc_fd_access_allowed() looks very fishy,
btw. As soon as the lock is dropped, local `alive' becomes meaningless.
Either that, or the lock wasn't needed.

btw, it's surprising (to me) that security_ptrace(t1, t2) fails when
t1==t2?