Skip to content

Commit 26f2043

Browse files
l0kodpcmoore
authored andcommitted
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: [email protected] Cc: Al Viro <[email protected]> Cc: Casey Schaufler <[email protected]> Cc: Christian Brauner <[email protected]> Cc: James Morris <[email protected]> Cc: Jann Horn <[email protected]> Cc: Ondrej Mosnacek <[email protected]> Cc: Paul Moore <[email protected]> Cc: Serge E. Hallyn <[email protected]> Cc: Stephen Smalley <[email protected]> Fixes: 1da177e ("Linux-2.6.12-rc2") Signed-off-by: Mickaël Salaün <[email protected]> Signed-off-by: Paul Moore <[email protected]>
1 parent ce4a605 commit 26f2043

File tree

1 file changed

+4
-10
lines changed

1 file changed

+4
-10
lines changed

fs/fcntl.c

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -87,8 +87,8 @@ static int setfl(int fd, struct file * filp, unsigned int arg)
8787
return error;
8888
}
8989

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)
9292
{
9393
write_lock_irq(&filp->f_owner.lock);
9494
if (force || !filp->f_owner.pid) {
@@ -98,19 +98,13 @@ static void f_modown(struct file *filp, struct pid *pid, enum pid_type type,
9898

9999
if (pid) {
100100
const struct cred *cred = current_cred();
101+
security_file_set_fowner(filp);
101102
filp->f_owner.uid = cred->uid;
102103
filp->f_owner.euid = cred->euid;
103104
}
104105
}
105106
write_unlock_irq(&filp->f_owner.lock);
106107
}
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-
}
114108
EXPORT_SYMBOL(__f_setown);
115109

116110
int f_setown(struct file *filp, int who, int force)
@@ -146,7 +140,7 @@ EXPORT_SYMBOL(f_setown);
146140

147141
void f_delown(struct file *filp)
148142
{
149-
f_modown(filp, NULL, PIDTYPE_TGID, 1);
143+
__f_setown(filp, NULL, PIDTYPE_TGID, 1);
150144
}
151145

152146
pid_t f_getown(struct file *filp)

0 commit comments

Comments
 (0)