6.1-stable review patch. If anyone has any objections, please let me know. ------------------ From: Mickaël Salaün <mic@xxxxxxxxxxx> commit 26f204380a3c182e5adf1a798db0724d6111b597 upstream. 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@xxxxxxxxxxxxxxx Cc: Al Viro <viro@xxxxxxxxxxxxxxxxxx> Cc: Casey Schaufler <casey@xxxxxxxxxxxxxxxx> Cc: Christian Brauner <brauner@xxxxxxxxxx> Cc: James Morris <jmorris@xxxxxxxxx> Cc: Jann Horn <jannh@xxxxxxxxxx> Cc: Ondrej Mosnacek <omosnace@xxxxxxxxxx> Cc: Paul Moore <paul@xxxxxxxxxxxxxx> Cc: Serge E. Hallyn <serge@xxxxxxxxxx> Cc: Stephen Smalley <stephen.smalley.work@xxxxxxxxx> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Signed-off-by: Mickaël Salaün <mic@xxxxxxxxxxx> Signed-off-by: Paul Moore <paul@xxxxxxxxxxxxxx> Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> --- fs/fcntl.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) --- a/fs/fcntl.c +++ b/fs/fcntl.c @@ -85,8 +85,8 @@ static int setfl(int fd, struct file * f return error; } -static void f_modown(struct file *filp, struct pid *pid, enum pid_type type, - int force) +void __f_setown(struct file *filp, struct pid *pid, enum pid_type type, + int force) { write_lock_irq(&filp->f_owner.lock); if (force || !filp->f_owner.pid) { @@ -96,19 +96,13 @@ static void f_modown(struct file *filp, if (pid) { const struct cred *cred = current_cred(); + security_file_set_fowner(filp); filp->f_owner.uid = cred->uid; filp->f_owner.euid = cred->euid; } } write_unlock_irq(&filp->f_owner.lock); } - -void __f_setown(struct file *filp, struct pid *pid, enum pid_type type, - int force) -{ - security_file_set_fowner(filp); - f_modown(filp, pid, type, force); -} EXPORT_SYMBOL(__f_setown); int f_setown(struct file *filp, unsigned long arg, int force) @@ -144,7 +138,7 @@ EXPORT_SYMBOL(f_setown); void f_delown(struct file *filp) { - f_modown(filp, NULL, PIDTYPE_TGID, 1); + __f_setown(filp, NULL, PIDTYPE_TGID, 1); } pid_t f_getown(struct file *filp)