Skip to content

Commit 2dde263

Browse files
committed
Merge tag 'fsnotify_for_v6.13-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/jack/linux-fs
Pull fsnotify updates from Jan Kara: "A couple of smaller random fsnotify fixes" * tag 'fsnotify_for_v6.13-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/jack/linux-fs: fsnotify: Fix ordering of iput() and watched_objects decrement fsnotify: fix sending inotify event with unexpected filename fanotify: allow reporting errors on failure to open fd fsnotify, lsm: Decouple fsnotify from lsm
2 parents c01f664 + 21d1b61 commit 2dde263

File tree

8 files changed

+77
-59
lines changed

8 files changed

+77
-59
lines changed

fs/notify/fanotify/Kconfig

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ config FANOTIFY
1515
config FANOTIFY_ACCESS_PERMISSIONS
1616
bool "fanotify permissions checking"
1717
depends on FANOTIFY
18-
depends on SECURITY
1918
default n
2019
help
2120
Say Y here is you want fanotify listeners to be able to make permissions

fs/notify/fanotify/fanotify_user.c

Lines changed: 48 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -265,13 +265,6 @@ static int create_fd(struct fsnotify_group *group, const struct path *path,
265265
group->fanotify_data.f_flags | __FMODE_NONOTIFY,
266266
current_cred());
267267
if (IS_ERR(new_file)) {
268-
/*
269-
* we still send an event even if we can't open the file. this
270-
* can happen when say tasks are gone and we try to open their
271-
* /proc files or we try to open a WRONLY file like in sysfs
272-
* we just send the errno to userspace since there isn't much
273-
* else we can do.
274-
*/
275268
put_unused_fd(client_fd);
276269
client_fd = PTR_ERR(new_file);
277270
} else {
@@ -662,7 +655,7 @@ static ssize_t copy_event_to_user(struct fsnotify_group *group,
662655
unsigned int info_mode = FAN_GROUP_FLAG(group, FANOTIFY_INFO_MODES);
663656
unsigned int pidfd_mode = info_mode & FAN_REPORT_PIDFD;
664657
struct file *f = NULL, *pidfd_file = NULL;
665-
int ret, pidfd = FAN_NOPIDFD, fd = FAN_NOFD;
658+
int ret, pidfd = -ESRCH, fd = -EBADF;
666659

667660
pr_debug("%s: group=%p event=%p\n", __func__, group, event);
668661

@@ -690,10 +683,39 @@ static ssize_t copy_event_to_user(struct fsnotify_group *group,
690683
if (!FAN_GROUP_FLAG(group, FANOTIFY_UNPRIV) &&
691684
path && path->mnt && path->dentry) {
692685
fd = create_fd(group, path, &f);
693-
if (fd < 0)
694-
return fd;
686+
/*
687+
* Opening an fd from dentry can fail for several reasons.
688+
* For example, when tasks are gone and we try to open their
689+
* /proc files or we try to open a WRONLY file like in sysfs
690+
* or when trying to open a file that was deleted on the
691+
* remote network server.
692+
*
693+
* For a group with FAN_REPORT_FD_ERROR, we will send the
694+
* event with the error instead of the open fd, otherwise
695+
* Userspace may not get the error at all.
696+
* In any case, userspace will not know which file failed to
697+
* open, so add a debug print for further investigation.
698+
*/
699+
if (fd < 0) {
700+
pr_debug("fanotify: create_fd(%pd2) failed err=%d\n",
701+
path->dentry, fd);
702+
if (!FAN_GROUP_FLAG(group, FAN_REPORT_FD_ERROR)) {
703+
/*
704+
* Historically, we've handled EOPENSTALE in a
705+
* special way and silently dropped such
706+
* events. Now we have to keep it to maintain
707+
* backward compatibility...
708+
*/
709+
if (fd == -EOPENSTALE)
710+
fd = 0;
711+
return fd;
712+
}
713+
}
695714
}
696-
metadata.fd = fd;
715+
if (FAN_GROUP_FLAG(group, FAN_REPORT_FD_ERROR))
716+
metadata.fd = fd;
717+
else
718+
metadata.fd = fd >= 0 ? fd : FAN_NOFD;
697719

698720
if (pidfd_mode) {
699721
/*
@@ -708,18 +730,16 @@ static ssize_t copy_event_to_user(struct fsnotify_group *group,
708730
* The PIDTYPE_TGID check for an event->pid is performed
709731
* preemptively in an attempt to catch out cases where the event
710732
* listener reads events after the event generating process has
711-
* already terminated. Report FAN_NOPIDFD to the event listener
712-
* in those cases, with all other pidfd creation errors being
713-
* reported as FAN_EPIDFD.
733+
* already terminated. Depending on flag FAN_REPORT_FD_ERROR,
734+
* report either -ESRCH or FAN_NOPIDFD to the event listener in
735+
* those cases with all other pidfd creation errors reported as
736+
* the error code itself or as FAN_EPIDFD.
714737
*/
715-
if (metadata.pid == 0 ||
716-
!pid_has_task(event->pid, PIDTYPE_TGID)) {
717-
pidfd = FAN_NOPIDFD;
718-
} else {
738+
if (metadata.pid && pid_has_task(event->pid, PIDTYPE_TGID))
719739
pidfd = pidfd_prepare(event->pid, 0, &pidfd_file);
720-
if (pidfd < 0)
721-
pidfd = FAN_EPIDFD;
722-
}
740+
741+
if (!FAN_GROUP_FLAG(group, FAN_REPORT_FD_ERROR) && pidfd < 0)
742+
pidfd = pidfd == -ESRCH ? FAN_NOPIDFD : FAN_EPIDFD;
723743
}
724744

725745
ret = -EFAULT;
@@ -736,9 +756,6 @@ static ssize_t copy_event_to_user(struct fsnotify_group *group,
736756
buf += FAN_EVENT_METADATA_LEN;
737757
count -= FAN_EVENT_METADATA_LEN;
738758

739-
if (fanotify_is_perm_event(event->mask))
740-
FANOTIFY_PERM(event)->fd = fd;
741-
742759
if (info_mode) {
743760
ret = copy_info_records_to_user(event, info, info_mode, pidfd,
744761
buf, count);
@@ -752,15 +769,18 @@ static ssize_t copy_event_to_user(struct fsnotify_group *group,
752769
if (pidfd_file)
753770
fd_install(pidfd, pidfd_file);
754771

772+
if (fanotify_is_perm_event(event->mask))
773+
FANOTIFY_PERM(event)->fd = fd;
774+
755775
return metadata.event_len;
756776

757777
out_close_fd:
758-
if (fd != FAN_NOFD) {
778+
if (f) {
759779
put_unused_fd(fd);
760780
fput(f);
761781
}
762782

763-
if (pidfd >= 0) {
783+
if (pidfd_file) {
764784
put_unused_fd(pidfd);
765785
fput(pidfd_file);
766786
}
@@ -827,15 +847,6 @@ static ssize_t fanotify_read(struct file *file, char __user *buf,
827847
}
828848

829849
ret = copy_event_to_user(group, event, buf, count);
830-
if (unlikely(ret == -EOPENSTALE)) {
831-
/*
832-
* We cannot report events with stale fd so drop it.
833-
* Setting ret to 0 will continue the event loop and
834-
* do the right thing if there are no more events to
835-
* read (i.e. return bytes read, -EAGAIN or wait).
836-
*/
837-
ret = 0;
838-
}
839850

840851
/*
841852
* Permission events get queued to wait for response. Other
@@ -844,7 +855,7 @@ static ssize_t fanotify_read(struct file *file, char __user *buf,
844855
if (!fanotify_is_perm_event(event->mask)) {
845856
fsnotify_destroy_event(group, &event->fse);
846857
} else {
847-
if (ret <= 0) {
858+
if (ret <= 0 || FANOTIFY_PERM(event)->fd < 0) {
848859
spin_lock(&group->notification_lock);
849860
finish_permission_event(group,
850861
FANOTIFY_PERM(event), FAN_DENY, NULL);
@@ -1941,7 +1952,7 @@ static int __init fanotify_user_setup(void)
19411952
FANOTIFY_DEFAULT_MAX_USER_MARKS);
19421953

19431954
BUILD_BUG_ON(FANOTIFY_INIT_FLAGS & FANOTIFY_INTERNAL_GROUP_FLAGS);
1944-
BUILD_BUG_ON(HWEIGHT32(FANOTIFY_INIT_FLAGS) != 12);
1955+
BUILD_BUG_ON(HWEIGHT32(FANOTIFY_INIT_FLAGS) != 13);
19451956
BUILD_BUG_ON(HWEIGHT32(FANOTIFY_MARK_FLAGS) != 11);
19461957

19471958
fanotify_mark_cache = KMEM_CACHE(fanotify_mark,

fs/notify/fsnotify.c

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -333,16 +333,19 @@ static int fsnotify_handle_event(struct fsnotify_group *group, __u32 mask,
333333
if (!inode_mark)
334334
return 0;
335335

336-
if (mask & FS_EVENT_ON_CHILD) {
337-
/*
338-
* Some events can be sent on both parent dir and child marks
339-
* (e.g. FS_ATTRIB). If both parent dir and child are
340-
* watching, report the event once to parent dir with name (if
341-
* interested) and once to child without name (if interested).
342-
* The child watcher is expecting an event without a file name
343-
* and without the FS_EVENT_ON_CHILD flag.
344-
*/
345-
mask &= ~FS_EVENT_ON_CHILD;
336+
/*
337+
* Some events can be sent on both parent dir and child marks (e.g.
338+
* FS_ATTRIB). If both parent dir and child are watching, report the
339+
* event once to parent dir with name (if interested) and once to child
340+
* without name (if interested).
341+
*
342+
* In any case regardless whether the parent is watching or not, the
343+
* child watcher is expecting an event without the FS_EVENT_ON_CHILD
344+
* flag. The file name is expected if and only if this is a directory
345+
* event.
346+
*/
347+
mask &= ~FS_EVENT_ON_CHILD;
348+
if (!(mask & ALL_FSNOTIFY_DIRENT_EVENTS)) {
346349
dir = NULL;
347350
name = NULL;
348351
}

fs/notify/mark.c

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -138,8 +138,11 @@ static void fsnotify_get_sb_watched_objects(struct super_block *sb)
138138

139139
static void fsnotify_put_sb_watched_objects(struct super_block *sb)
140140
{
141-
if (atomic_long_dec_and_test(fsnotify_sb_watched_objects(sb)))
142-
wake_up_var(fsnotify_sb_watched_objects(sb));
141+
atomic_long_t *watched_objects = fsnotify_sb_watched_objects(sb);
142+
143+
/* the superblock can go away after this decrement */
144+
if (atomic_long_dec_and_test(watched_objects))
145+
wake_up_var(watched_objects);
143146
}
144147

145148
static void fsnotify_get_inode_ref(struct inode *inode)
@@ -150,8 +153,11 @@ static void fsnotify_get_inode_ref(struct inode *inode)
150153

151154
static void fsnotify_put_inode_ref(struct inode *inode)
152155
{
153-
fsnotify_put_sb_watched_objects(inode->i_sb);
156+
/* read ->i_sb before the inode can go away */
157+
struct super_block *sb = inode->i_sb;
158+
154159
iput(inode);
160+
fsnotify_put_sb_watched_objects(sb);
155161
}
156162

157163
/*

fs/open.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -929,6 +929,10 @@ static int do_dentry_open(struct file *f,
929929
if (error)
930930
goto cleanup_all;
931931

932+
error = fsnotify_open_perm(f);
933+
if (error)
934+
goto cleanup_all;
935+
932936
error = break_lease(file_inode(f), f->f_flags);
933937
if (error)
934938
goto cleanup_all;

include/linux/fanotify.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
#define FANOTIFY_ADMIN_INIT_FLAGS (FANOTIFY_PERM_CLASSES | \
3737
FAN_REPORT_TID | \
3838
FAN_REPORT_PIDFD | \
39+
FAN_REPORT_FD_ERROR | \
3940
FAN_UNLIMITED_QUEUE | \
4041
FAN_UNLIMITED_MARKS)
4142

include/uapi/linux/fanotify.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@
6060
#define FAN_REPORT_DIR_FID 0x00000400 /* Report unique directory id */
6161
#define FAN_REPORT_NAME 0x00000800 /* Report events with name */
6262
#define FAN_REPORT_TARGET_FID 0x00001000 /* Report dirent target id */
63+
#define FAN_REPORT_FD_ERROR 0x00002000 /* event->fd can report error */
6364

6465
/* Convenience macro - FAN_REPORT_NAME requires FAN_REPORT_DIR_FID */
6566
#define FAN_REPORT_DFID_NAME (FAN_REPORT_DIR_FID | FAN_REPORT_NAME)

security/security.c

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
#include <linux/kernel.h>
2020
#include <linux/kernel_read_file.h>
2121
#include <linux/lsm_hooks.h>
22-
#include <linux/fsnotify.h>
2322
#include <linux/mman.h>
2423
#include <linux/mount.h>
2524
#include <linux/personality.h>
@@ -3103,13 +3102,7 @@ int security_file_receive(struct file *file)
31033102
*/
31043103
int security_file_open(struct file *file)
31053104
{
3106-
int ret;
3107-
3108-
ret = call_int_hook(file_open, file);
3109-
if (ret)
3110-
return ret;
3111-
3112-
return fsnotify_open_perm(file);
3105+
return call_int_hook(file_open, file);
31133106
}
31143107

31153108
/**

0 commit comments

Comments
 (0)