07 Feb, 2019
1 commit
-
commit b469e7e47c8a075cc08bcd1e85d4365134bdcdd5 upstream.
When an event is reported on a sub-directory and the parent inode has
a mark mask with FS_EVENT_ON_CHILD|FS_ISDIR, the event will be sent to
fsnotify() even if the event type is not in the parent mark mask
(e.g. FS_OPEN).Further more, if that event happened on a mount or a filesystem with
a mount/sb mark that does have that event type in their mask, the "on
child" event will be reported on the mount/sb mark. That is not
desired, because user will get a duplicate event for the same action.Note that the event reported on the victim inode is never merged with
the event reported on the parent inode, because of the check in
should_merge(): old_fsn->inode == new_fsn->inode.Fix this by looking for a match of an actual event type (i.e. not just
FS_ISDIR) in parent's inode mark mask and by not reporting an "on child"
event to group if event type is only found on mount/sb marks.[backport hint: The bug seems to have always been in fanotify, but this
patch will only apply cleanly to v4.19.y]Cc: # v4.19
Signed-off-by: Amir Goldstein
Signed-off-by: Jan Kara
[amir: backport to v4.9]
Signed-off-by: Amir Goldstein
Signed-off-by: Greg Kroah-Hartman
10 Nov, 2018
1 commit
-
[ Upstream commit 9bdda4e9cf2dcecb60a0683b10ffb8cd7e5f2f45 ]
Commit 92183a42898d ("fsnotify: fix ignore mask logic in
send_to_group()") acknoledges the use case of ignoring an event on
an inode mark, because of an ignore mask on a mount mark of the same
group (i.e. I want to get all events on this file, except for the events
that came from that mount).This change depends on correctly merging the inode marks and mount marks
group lists, so that the mount mark ignore mask would be tested in
send_to_group(). Alas, the merging of the lists did not take into
account the case where event in question is not in the mask of any of
the mount marks.To fix this, completely remove the tests for inode and mount event masks
from the lists merging code.Fixes: 92183a42898d ("fsnotify: fix ignore mask logic in send_to_group")
Signed-off-by: Amir Goldstein
Signed-off-by: Jan Kara
[amir: backport to v4.14.y]
Signed-off-by: Amir Goldstein
Signed-off-by: Sasha Levin
21 Jun, 2018
1 commit
-
[ Upstream commit 92183a42898dc400b89da35685d1814ac6acd3d8 ]
The ignore mask logic in send_to_group() does not match the logic
in fanotify_should_send_event(). In the latter, a vfsmount mark ignore
mask precedes an inode mark mask and in the former, it does not.That difference may cause events to be sent to fanotify backend for no
reason. Fix the logic in send_to_group() to match that of
fanotify_should_send_event().Signed-off-by: Amir Goldstein
Signed-off-by: Jan Kara
Signed-off-by: Sasha Levin
Signed-off-by: Greg Kroah-Hartman
24 Apr, 2018
1 commit
-
commit 54a307ba8d3cd00a3902337ffaae28f436eeb1a4 upstream.
When event on child inodes are sent to the parent inode mark and
parent inode mark was not marked with FAN_EVENT_ON_CHILD, the event
will not be delivered to the listener process. However, if the same
process also has a mount mark, the event to the parent inode will be
delivered regadless of the mount mark mask.This behavior is incorrect in the case where the mount mark mask does
not contain the specific event type. For example, the process adds
a mark on a directory with mask FAN_MODIFY (without FAN_EVENT_ON_CHILD)
and a mount mark with mask FAN_CLOSE_NOWRITE (without FAN_ONDIR).A modify event on a file inside that directory (and inside that mount)
should not create a FAN_MODIFY event, because neither of the marks
requested to get that event on the file.Fixes: 1968f5eed54c ("fanotify: use both marks when possible")
Cc: stable
Signed-off-by: Amir Goldstein
Signed-off-by: Jan Kara
Signed-off-by: Greg Kroah-Hartman
25 Feb, 2018
1 commit
-
commit b3a0066005821acdc0cdb092cb72587182ab583f upstream.
fsnotify_add_mark_locked() can fail but we do not check its return
value. This didn't matter before commit 9dd813c15b2c "fsnotify: Move
mark list head from object into dedicated structure" as none of possible
failures could happen for dnotify but after that commit -ENOMEM can be
returned. Handle this error properly in fcntl_dirnotify() as
otherwise we just hit BUG_ON(dn_mark->dn) in dnotify_free_mark().Reviewed-by: Amir Goldstein
Reported-by: syzkaller
Fixes: 9dd813c15b2c101168808d4f5941a29985758973
Signed-off-by: Jan Kara
Signed-off-by: Greg Kroah-Hartman
30 Nov, 2017
4 commits
-
commit 9a31d7ad997f55768c687974ce36b759065b49e5 upstream.
Blind increment of group's user_waits is not enough, we could be far enough
in the group's destruction that it isn't taken into account (i.e. grabbing
the mark ref afterwards doesn't guarantee that it was the ref coming from
the _group_ that was grabbed).Instead we need to check (under lock) that the mark is still attached to
the group after having obtained a ref to the mark. If not, skip it.Reviewed-by: Amir Goldstein
Signed-off-by: Miklos Szeredi
Fixes: 9385a84d7e1f ("fsnotify: Pass fsnotify_iter_info into handle_event handler")
Signed-off-by: Jan Kara
Signed-off-by: Greg Kroah-Hartman -
commit 0d6ec079d6aaa098b978d6395973bb027c752a03 upstream.
We may fail to pin one of the marks in fsnotify_prepare_user_wait() when
dropping the srcu read lock, resulting in use after free at the next
iteration.Solution is to store both marks in iter_info instead of just the one we'll
be sending the event for.Reviewed-by: Amir Goldstein
Signed-off-by: Miklos Szeredi
Fixes: 9385a84d7e1f ("fsnotify: Pass fsnotify_iter_info into handle_event handler")
Signed-off-by: Jan Kara
Signed-off-by: Greg Kroah-Hartman -
commit 24c20305c7fc8959836211cb8c50aab93ae0e54f upstream.
This patch doesn't actually fix any bug, just paves the way for fixing mark
and group pinning.Reviewed-by: Amir Goldstein
Signed-off-by: Miklos Szeredi
Signed-off-by: Jan Kara
Signed-off-by: Greg Kroah-Hartman -
commit f37650f1c7c71cf5180b43229d13b421d81e7170 upstream.
If fsnotify_prepare_user_wait() fails, we leave the event on the
notification list. Which will result in a warning in
fsnotify_destroy_event() and later use-after-free.Instead of adding a new helper to remove the event from the list in this
case, I opted to move the prepare/finish up into fanotify_handle_event().This will allow these to be moved further out into the generic code later,
and perhaps let us move to non-sleeping RCU.Reviewed-by: Amir Goldstein
Signed-off-by: Miklos Szeredi
Fixes: 05f0e38724e8 ("fanotify: Release SRCU lock when waiting for userspace response")
Signed-off-by: Jan Kara
Signed-off-by: Greg Kroah-Hartman
02 Nov, 2017
1 commit
-
Many source files in the tree are missing licensing information, which
makes it harder for compliance tools to determine the correct license.By default all files without license information are under the default
license of the kernel, which is GPL version 2.Update the files which contain no license information with the 'GPL-2.0'
SPDX license identifier. The SPDX identifier is a legally binding
shorthand, which can be used instead of the full boiler plate text.This patch is based on work done by Thomas Gleixner and Kate Stewart and
Philippe Ombredanne.How this work was done:
Patches were generated and checked against linux-4.14-rc6 for a subset of
the use cases:
- file had no licensing information it it.
- file was a */uapi/* one with no licensing information in it,
- file was a */uapi/* one with existing licensing information,Further patches will be generated in subsequent months to fix up cases
where non-standard license headers were used, and references to license
had to be inferred by heuristics based on keywords.The analysis to determine which SPDX License Identifier to be applied to
a file was done in a spreadsheet of side by side results from of the
output of two independent scanners (ScanCode & Windriver) producing SPDX
tag:value files created by Philippe Ombredanne. Philippe prepared the
base worksheet, and did an initial spot review of a few 1000 files.The 4.13 kernel was the starting point of the analysis with 60,537 files
assessed. Kate Stewart did a file by file comparison of the scanner
results in the spreadsheet to determine which SPDX license identifier(s)
to be applied to the file. She confirmed any determination that was not
immediately clear with lawyers working with the Linux Foundation.Criteria used to select files for SPDX license identifier tagging was:
- Files considered eligible had to be source code files.
- Make and config files were included as candidates if they contained >5
lines of source
- File already had some variant of a license header in it (even if
Reviewed-by: Philippe Ombredanne
Reviewed-by: Thomas Gleixner
Signed-off-by: Greg Kroah-Hartman
30 Aug, 2017
1 commit
-
Make this const as it is never modified.
Signed-off-by: Bhumika Goyal
Signed-off-by: Jan Kara
08 Jul, 2017
1 commit
-
take_dentry_name_snapshot() takes a safe snapshot of dentry name;
if the name is a short one, it gets copied into caller-supplied
structure, otherwise an extra reference to external name is grabbed
(those are never modified). In either case the pointer to stable
string is stored into the same structure.dentry must be held by the caller of take_dentry_name_snapshot(),
but may be freely dropped afterwards - the snapshot will stay
until destroyed by release_dentry_name_snapshot().Intended use:
struct name_snapshot s;take_dentry_name_snapshot(&s, dentry);
...
access s.name
...
release_dentry_name_snapshot(&s);Replaces fsnotify_oldname_...(), gets used in fsnotify to obtain the name
to pass down with event.Signed-off-by: Al Viro
25 Apr, 2017
1 commit
-
When delivering an event to userspace for a file on an NFS share,
if the file is deleted on server side before user reads the event,
user will not get the event.If the event queue contained several events, the stale event is
quietly dropped and read() returns to user with events read so far
in the buffer.If the event queue contains a single stale event or if the stale
event is a permission event, read() returns to user with the kernel
internal error code 518 (EOPENSTALE), which is not a POSIX error code.Check the internal return value -EOPENSTALE in fanotify_read(), just
the same as it is checked in path_openat() and drop the event in the
cases that it is not already dropped.This is a reproducer from Marko Rauhamaa:
Just take the example program listed under "man fanotify" ("fantest")
and follow these steps:==============================================================
NFS Server NFS Client(1) NFS Client(2)
==============================================================
# echo foo >/nfsshare/bar.txt
# cat /nfsshare/bar.txt
foo
# ./fantest /nfsshare
Press enter key to terminate.
Listening for events.
# rm -f /nfsshare/bar.txt
# cat /nfsshare/bar.txt
read: Unknown error 518
cat: /nfsshare/bar.txt: Operation not permitted
==============================================================where NFS Client (1) and (2) are two terminal sessions on a single NFS
Client machine.Reported-by: Marko Rauhamaa
Tested-by: Marko Rauhamaa
Cc:
Cc:
Signed-off-by: Amir Goldstein
Signed-off-by: Jan Kara
24 Apr, 2017
1 commit
-
We recently shifted this code around, so we're no longer holding the
lock on this path.Fixes: 755b5bc681eb ("fsnotify: Remove indirection from mark list addition")
Signed-off-by: Dan Carpenter
Signed-off-by: Jan Kara
10 Apr, 2017
26 commits
-
Pointer to ->free_mark callback unnecessarily occupies one long in each
fsnotify_mark although they are the same for all marks from one
notification group. Move the callback pointer to fsnotify_ops.Reviewed-by: Miklos Szeredi
Reviewed-by: Amir Goldstein
Signed-off-by: Jan Kara -
Currently we initialize mark->group only in fsnotify_add_mark_lock().
However we will need to access fsnotify_ops of corresponding group from
fsnotify_put_mark() so we need mark->group initialized earlier. Do that
in fsnotify_init_mark() which has a consequence that once
fsnotify_init_mark() is called on a mark, the mark has to be destroyed
by fsnotify_put_mark().Reviewed-by: Miklos Szeredi
Reviewed-by: Amir Goldstein
Signed-off-by: Jan Kara -
inode_mark.c now contains only a single function. Move it to
fs/notify/fsnotify.c and remove inode_mark.c.Reviewed-by: Miklos Szeredi
Reviewed-by: Amir Goldstein
Signed-off-by: Jan Kara -
These are very thin wrappers, just remove them. Drop
fs/notify/vfsmount_mark.c as it is empty now.Reviewed-by: Miklos Szeredi
Reviewed-by: Amir Goldstein
Signed-off-by: Jan Kara -
The function is already mostly contained in what
fsnotify_clear_marks_by_group() does. Just update that function to not
select marks when all of them should be destroyed and remove
fsnotify_detach_group_marks().Reviewed-by: Miklos Szeredi
Reviewed-by: Amir Goldstein
Signed-off-by: Jan Kara -
The _flags() suffix in the function name was more confusing than
explaining so just remove it. Also rename the argument from 'flags' to
'type' to better explain what the function expects.Reviewed-by: Miklos Szeredi
Suggested-by: Amir Goldstein
Reviewed-by: Amir Goldstein
Signed-off-by: Jan Kara -
Inline these helpers as they are very thin. We still keep them as we
don't want to expose details about how list type is determined.Reviewed-by: Miklos Szeredi
Reviewed-by: Amir Goldstein
Signed-off-by: Jan Kara -
These helpers are just very thin wrappers now. Remove them.
Reviewed-by: Miklos Szeredi
Reviewed-by: Amir Goldstein
Signed-off-by: Jan Kara -
These helpers are now only a simple assignment and just obfuscate
what is going on. Remove them.Reviewed-by: Miklos Szeredi
Reviewed-by: Amir Goldstein
Signed-off-by: Jan Kara -
When userspace task processing fanotify permission events screws up and
does not respond, fsnotify_mark_srcu SRCU is held indefinitely which
causes further hangs in the whole notification subsystem. Although we
cannot easily solve the problem of operations blocked waiting for
response from userspace, we can at least somewhat localize the damage by
dropping SRCU lock before waiting for userspace response and reacquiring
it when userspace responds.Reviewed-by: Miklos Szeredi
Reviewed-by: Amir Goldstein
Signed-off-by: Jan Kara -
Pass fsnotify_iter_info into ->handle_event() handler so that it can
release and reacquire SRCU lock via fsnotify_prepare_user_wait() and
fsnotify_finish_user_wait() functions. These functions also make sure
current marks are appropriately pinned so that iteration protected by
srcu in fsnotify() stays safe.Reviewed-by: Miklos Szeredi
Reviewed-by: Amir Goldstein
Signed-off-by: Jan Kara -
fanotify wants to drop fsnotify_mark_srcu lock when waiting for response
from userspace so that the whole notification subsystem is not blocked
during that time. This patch provides a framework for safely getting
mark reference for a mark found in the object list which pins the mark
in that list. We can then drop fsnotify_mark_srcu, wait for userspace
response and then safely continue iteration of the object list once we
reaquire fsnotify_mark_srcu.Reviewed-by: Miklos Szeredi
Reviewed-by: Amir Goldstein
Signed-off-by: Jan Kara -
Currently we queue all marks for destruction on group shutdown and then
destroy them from fsnotify_destroy_group() instead from a worker thread
which is the usual path. However worker can already be processing some
list of marks to destroy so this does not make 100% all marks are really
destroyed by the time group is shut down. This isn't a big problem as
each mark holds group reference and thus group stays partially alive
until all marks are really freed but there's no point in complicating
our lives - just wait for the delayed work to be finished instead.Reviewed-by: Miklos Szeredi
Reviewed-by: Amir Goldstein
Signed-off-by: Jan Kara -
Instead of removing mark from object list from fsnotify_detach_mark(),
remove the mark when last reference to the mark is dropped. This will
allow fanotify to wait for userspace response to event without having to
hold onto fsnotify_mark_srcu.To avoid pinning inodes by elevated refcount (and thus e.g. delaying
file deletion) while someone holds mark reference, we detach connector
from the object also from fsnotify_destroy_marks() and not only after
removing last mark from the list as it was now.Reviewed-by: Miklos Szeredi
Reviewed-by: Amir Goldstein
Signed-off-by: Jan Kara -
Currently we queue mark into a list of marks for destruction in
__fsnotify_free_mark() and keep the last mark reference dangling. After the
worker waits for SRCU period, it drops the last reference to the mark
which frees it. This scheme has the disadvantage that if we hold
reference to a mark and drop and reacquire SRCU lock, the mark can get
freed immediately which is slightly inconvenient and we will need to
avoid this in the future.Move to a scheme where queueing of mark into a list of marks for
destruction happens when the last reference to the mark is dropped. Also
drop reference to the mark held by group list already when mark is
removed from that list instead of dropping it only from the destruction
worker.Reviewed-by: Miklos Szeredi
Reviewed-by: Amir Goldstein
Signed-off-by: Jan Kara -
Dropping mark reference can result in mark being freed. Although it
should not happen in inotify_remove_from_idr() since caller should hold
another reference, just don't risk lock up just after WARN_ON
unnecessarily. Also fold do_inotify_remove_from_idr() into the single
callsite as that function really is just two lines of real code.Reviewed-by: Miklos Szeredi
Reviewed-by: Amir Goldstein
Signed-off-by: Jan Kara -
Currently we free fsnotify_mark_connector structure only when inode /
vfsmount is getting freed. This can however impose noticeable memory
overhead when marks get attached to inodes only temporarily. So free the
connector structure once the last mark is detached from the object.
Since notification infrastructure can be working with the connector
under the protection of fsnotify_mark_srcu, we have to be careful and
free the fsnotify_mark_connector only after SRCU period passes.Reviewed-by: Miklos Szeredi
Reviewed-by: Amir Goldstein
Signed-off-by: Jan Kara -
So far list of marks attached to an object (inode / vfsmount) was
protected by i_lock or mnt_root->d_lock. This dictates that the list
must be empty before the object can be destroyed although the list is
now anchored in the fsnotify_mark_connector structure. Protect the list
by a spinlock in the fsnotify_mark_connector structure to decouple
lifetime of a list of marks from a lifetime of the object. This also
simplifies the code quite a bit since we don't have to differentiate
between inode and vfsmount lists in quite a few places anymore.Reviewed-by: Miklos Szeredi
Reviewed-by: Amir Goldstein
Signed-off-by: Jan Kara -
After removing all the indirection it is clear that
hlist_del_init_rcu(&mark->obj_list);
in fsnotify_destroy_marks() is not needed as the mark gets removed from
the list shortly afterwards in fsnotify_destroy_mark() ->
fsnotify_detach_mark() -> fsnotify_detach_from_object(). Also there is
no problem with mark being visible on object list while we call
fsnotify_destroy_mark() as parallel destruction of marks from several
places is properly handled (as mentioned in the comment in
fsnotify_destroy_marks(). So just remove the list removal and also the
stale comment.Reviewed-by: Miklos Szeredi
Reviewed-by: Amir Goldstein
Signed-off-by: Jan Kara -
We lock object list lock in fsnotify_detach_from_object() twice - once
to detach mark and second time to recalculate mask. That is unnecessary
and later it will become problematic as we will free the connector as
soon as there is no mark in it. So move recalculation of fsnotify mask
into the same critical section that is detaching mark.This also removes recalculation of child dentry flags from
fsnotify_detach_from_object(). That is however fine. Those marks will
get recalculated once some event happens on a child.Reviewed-by: Miklos Szeredi
Signed-off-by: Jan Kara -
fsnotify_detach_mark() calls fsnotify_destroy_inode_mark() or
fsnotify_destroy_vfsmount_mark() to remove mark from object list. These
two functions are however very similar and differ only in the lock they
use to protect the object list of marks. Simplify the code by removing
the indirection and removing mark from the object list in a common
function.Reviewed-by: Miklos Szeredi
Reviewed-by: Amir Goldstein
Signed-off-by: Jan Kara -
Instead of passing spinlock into fsnotify_destroy_marks() determine it
directly in that function from the connector type. This will reduce code
churn when changing lock protecting list of marks.Reviewed-by: Miklos Szeredi
Reviewed-by: Amir Goldstein
Signed-off-by: Jan Kara -
Move locking of a mark list into fsnotify_find_mark(). This reduces code
churn in the following patch changing lock protecting the list.Reviewed-by: Miklos Szeredi
Reviewed-by: Amir Goldstein
Signed-off-by: Jan Kara -
Move locking of locks protecting a list of marks into
fsnotify_recalc_mask(). This reduces code churn in the following patch
which changes the lock protecting the list of marks.Reviewed-by: Miklos Szeredi
Reviewed-by: Amir Goldstein
Signed-off-by: Jan Kara -
Move fsnotify_destroy_marks() to be later in the fs/notify/mark.c. It
will need some functions that are declared after its current
declaration. No functional change.Reviewed-by: Miklos Szeredi
Reviewed-by: Amir Goldstein
Signed-off-by: Jan Kara -
Adding notification mark to object list has been currently done through
fsnotify_add_{inode|vfsmount}_mark() helpers from
fsnotify_add_mark_locked() which call fsnotify_add_mark_list(). Remove
this unnecessary indirection to simplify the code.Pushing all the locking to fsnotify_add_mark_list() also allows us to
allocate the connector structure with GFP_KERNEL mode.Reviewed-by: Miklos Szeredi
Reviewed-by: Amir Goldstein
Signed-off-by: Jan Kara