Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

fs: Fix file_set_fowner LSM hook inconsistencies

The fcntl's F_SETOWN command sets the process that handle SIGIO/SIGURG
for the related file descriptor. Before this change, the
file_set_fowner LSM hook was always called, ignoring the VFS logic which
may not actually change the process that handles SIGIO (e.g. TUN, TTY,
dnotify), nor update the related UID/EUID.

Moreover, because security_file_set_fowner() was called without lock
(e.g. f_owner.lock), concurrent F_SETOWN commands could result to a race
condition and inconsistent LSM states (e.g. SELinux's fown_sid) compared
to struct fown_struct's UID/EUID.

This change makes sure the LSM states are always in sync with the VFS
state by moving the security_file_set_fowner() call close to the
UID/EUID updates and using the same f_owner.lock .

Rename f_modown() to __f_setown() to simplify code.

Cc: stable@vger.kernel.org
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Casey Schaufler <casey@schaufler-ca.com>
Cc: Christian Brauner <brauner@kernel.org>
Cc: James Morris <jmorris@namei.org>
Cc: Jann Horn <jannh@google.com>
Cc: Ondrej Mosnacek <omosnace@redhat.com>
Cc: Paul Moore <paul@paul-moore.com>
Cc: Serge E. Hallyn <serge@hallyn.com>
Cc: Stephen Smalley <stephen.smalley.work@gmail.com>
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Signed-off-by: Paul Moore <paul@paul-moore.com>

authored by

Mickaël Salaün and committed by
Paul Moore
26f20438 ce4a6059

+4 -10
+4 -10
fs/fcntl.c
··· 87 87 return error; 88 88 } 89 89 90 - static void f_modown(struct file *filp, struct pid *pid, enum pid_type type, 91 - int force) 90 + void __f_setown(struct file *filp, struct pid *pid, enum pid_type type, 91 + int force) 92 92 { 93 93 write_lock_irq(&filp->f_owner.lock); 94 94 if (force || !filp->f_owner.pid) { ··· 98 98 99 99 if (pid) { 100 100 const struct cred *cred = current_cred(); 101 + security_file_set_fowner(filp); 101 102 filp->f_owner.uid = cred->uid; 102 103 filp->f_owner.euid = cred->euid; 103 104 } 104 105 } 105 106 write_unlock_irq(&filp->f_owner.lock); 106 - } 107 - 108 - void __f_setown(struct file *filp, struct pid *pid, enum pid_type type, 109 - int force) 110 - { 111 - security_file_set_fowner(filp); 112 - f_modown(filp, pid, type, force); 113 107 } 114 108 EXPORT_SYMBOL(__f_setown); 115 109 ··· 140 146 141 147 void f_delown(struct file *filp) 142 148 { 143 - f_modown(filp, NULL, PIDTYPE_TGID, 1); 149 + __f_setown(filp, NULL, PIDTYPE_TGID, 1); 144 150 } 145 151 146 152 pid_t f_getown(struct file *filp)