27 Apr, 2019

1 commit


15 Jan, 2019

1 commit

  • Since the context is derived from the task parameter handed to
    __audit_free(), hand the context to audit_kill_trees() so it can be used
    to associate with a syscall record. This requires adding the context
    parameter to kill_rules() rather than using the current audit_context.

    The callers of trim_marked() and evict_chunk() still have their context.

    The EOE record was being issued prior to the pruning of the killed_tree
    list.

    Move the kill_trees call before the audit_log_exit call in
    __audit_free() and __audit_syscall_exit() so that any pruned trees
    CONFIG_CHANGE records are included with the associated syscall event by
    the user library due to the EOE record flagging the end of the event.

    See: https://github.com/linux-audit/audit-kernel/issues/50
    See: https://github.com/linux-audit/audit-kernel/issues/59

    Signed-off-by: Richard Guy Briggs
    [PM: fixed merge fuzz in kernel/audit_tree.c]
    Signed-off-by: Paul Moore

    Richard Guy Briggs
     

27 Nov, 2018

1 commit


12 Nov, 2018

14 commits

  • Variables pointing to fsnotify_mark are sometimes called 'entry' and
    sometimes 'mark'. Use 'mark' in all places.

    Reviewed-by: Richard Guy Briggs
    Signed-off-by: Jan Kara
    [PM: minor merge fuzz due to updated patches previously in the series]
    Signed-off-by: Paul Moore

    Jan Kara
     
  • Audit tree code currently associates new fsnotify mark with each new
    chunk. As chunk attached to an inode is replaced when new tag is added /
    removed, we also need to remove old fsnotify mark and add a new one on
    such occasion. This is cumbersome and makes locking rules somewhat
    difficult to follow.

    Fix these problems by allocating fsnotify mark independently of chunk
    and keeping it all the time while there is some chunk attached to an
    inode. Also add documentation about the locking rules so that things are
    easier to follow.

    Signed-off-by: Jan Kara
    Reviewed-by: Richard Guy Briggs
    [PM: minor merge fuzz due to updated patches previously in the series]
    Signed-off-by: Paul Moore

    Jan Kara
     
  • untag_chunk() has to be called with hash_lock, it drops it and
    reacquires it when returning. The unlocking of hash_lock is thus hidden
    from the callers of untag_chunk() with is rather error prone. Reorganize
    the code so that untag_chunk() is called without hash_lock, only with
    mark reference preventing the chunk from going away.

    Since this requires some more code in the caller of untag_chunk() to
    assure forward progress, factor out loop pruning tree from all chunks
    into a common helper function.

    Signed-off-by: Jan Kara
    Reviewed-by: Richard Guy Briggs
    Signed-off-by: Paul Moore

    Jan Kara
     
  • When deleting chunk from a tree, drop all unused nodes in a chunk
    instead of just the one used by the tree. This gets rid of possibly
    lingering unused nodes (created due to fallback path in untag_chunk())
    and also removes some special cases and will allow us to simplify
    locking in untag_chunk().

    Signed-off-by: Jan Kara
    Reviewed-by: Richard Guy Briggs
    Signed-off-by: Paul Moore

    Jan Kara
     
  • When removing chunk from a tree, we do shrink the chunk. This can fail
    for various reasons (due to races, ENOMEM, etc.) and in some cases we
    just bail from untag_chunk() relying on someone else to cleanup.
    Although this currently works, later we will need to add new failure
    situation which would break. Also this simplifies the code and will
    allow us to make locking around untag_chunk() less awkward.

    Signed-off-by: Jan Kara
    Reviewed-by: Richard Guy Briggs
    Signed-off-by: Paul Moore

    Jan Kara
     
  • Allocate fsnotify mark independently instead of embedding it inside
    chunk. This will allow us to just replace chunk attached to mark when
    growing / shrinking chunk instead of replacing mark attached to inode
    which is a more complex operation.

    Reviewed-by: Richard Guy Briggs
    Signed-off-by: Jan Kara
    Signed-off-by: Paul Moore

    Jan Kara
     
  • Provide a helper function audit_mark_put_chunk() for dropping mark's
    reference (which has to happen only after RCU grace period expires).
    Currently that happens only from a single place but in later patches we
    introduce more callers.

    Reviewed-by: Richard Guy Briggs
    Signed-off-by: Jan Kara
    Signed-off-by: Paul Moore

    Jan Kara
     
  • The audit_tree_group->mark_mutex is held all the time while we create
    the fsnotify mark, add it to the inode, and insert chunk into the hash.
    Hence mark cannot get detached during this time and so the check whether
    the mark is attached in insert_hash() is pointless.

    Reviewed-by: Richard Guy Briggs
    Signed-off-by: Jan Kara
    Signed-off-by: Paul Moore

    Jan Kara
     
  • Chunk replacement code is very similar for the cases where we grow or
    shrink chunk. Factor the code out into a common helper function.

    Signed-off-by: Jan Kara
    Reviewed-by: Richard Guy Briggs
    Signed-off-by: Paul Moore

    Jan Kara
     
  • Currently, the audit tree code does not make sure that when a chunk is
    inserted into the hash table, it is fully initialized. So in theory a
    user of RCU lookup could see uninitialized structure in the hash table
    and crash. Add appropriate barriers between initialization of the
    structure and its insertion into hash table.

    Reviewed-by: Richard Guy Briggs
    Signed-off-by: Jan Kara
    Signed-off-by: Paul Moore

    Jan Kara
     
  • Currently chunk hash key (which is in fact pointer to the inode) is
    derived as chunk->mark.conn->obj. It is tricky to make this dereference
    reliable for hash table lookups only under RCU as mark can get detached
    from the connector and connector gets freed independently of the
    running lookup. Thus there is a possible use after free / NULL ptr
    dereference issue:

    CPU1 CPU2
    untag_chunk()
    ...
    audit_tree_lookup()
    list_for_each_entry_rcu(p, list, hash) {
    list_del_rcu(&chunk->hash);
    fsnotify_destroy_mark(entry);
    fsnotify_put_mark(entry)
    chunk_to_key(p)
    if (!chunk->mark.connector)
    ...
    hlist_del_init_rcu(&mark->obj_list);
    if (hlist_empty(&conn->list)) {
    inode = fsnotify_detach_connector_from_object(conn);
    mark->connector = NULL;
    ...
    frees connector from workqueue
    chunk->mark.connector->obj

    This race is probably impossible to hit in practice as the race window
    on CPU1 is very narrow and CPU2 has a lot of code to execute. Still it's
    better to have this fixed. Since the inode the chunk is attached to is
    constant during chunk's lifetime it is easy to cache the key in the
    chunk itself and thus avoid these issues.

    Reviewed-by: Richard Guy Briggs
    Signed-off-by: Jan Kara
    Signed-off-by: Paul Moore

    Jan Kara
     
  • Audit tree code is replacing marks attached to inodes in non-atomic way.
    Thus fsnotify_find_mark() in tag_chunk() may find a mark that belongs to
    a chunk that is no longer valid one and will soon be destroyed. Tags
    added to such chunk will be simply lost.

    Fix the problem by making sure old mark is marked as going away (through
    fsnotify_detach_mark()) before dropping mark_mutex and thus in an atomic
    way wrt tag_chunk(). Note that this does not fix the problem completely
    as if tag_chunk() finds a mark that is going away, it fails with
    -ENOENT. But at least the failure is not silent and currently there's no
    way to search for another fsnotify mark attached to the inode. We'll fix
    this problem in later patch.

    Reviewed-by: Richard Guy Briggs
    Signed-off-by: Jan Kara
    Signed-off-by: Paul Moore

    Jan Kara
     
  • When an inode is tagged with a tree, tag_chunk() checks whether there is
    audit_tree_group mark attached to the inode and adds one if not. However
    nothing protects another tag_chunk() to add the mark between we've
    checked and try to add the fsnotify mark thus resulting in an error from
    fsnotify_add_mark() and consequently an ENOSPC error from tag_chunk().

    Fix the problem by holding mark_mutex over the whole check-insert code
    sequence.

    Reviewed-by: Richard Guy Briggs
    Signed-off-by: Jan Kara
    Signed-off-by: Paul Moore

    Jan Kara
     
  • Currently, audit_tree code uses mark->lock to protect against detaching
    of mark from an inode. In most places it however also uses
    mark->group->mark_mutex (as we need to atomically replace attached
    marks) and this provides protection against mark detaching as well. So
    just remove protection with mark->lock from audit tree code and replace
    it with mark->group->mark_mutex protection in all the places. It
    simplifies the code and gets rid of some ugly catches like calling
    fsnotify_add_mark_locked() with mark->lock held (which cannot sleep only
    because we hold a reference to another mark attached to the same inode).

    Reviewed-by: Richard Guy Briggs
    Signed-off-by: Jan Kara
    Signed-off-by: Paul Moore

    Jan Kara
     

18 Aug, 2018

1 commit

  • Pull fsnotify updates from Jan Kara:
    "fsnotify cleanups from Amir and a small inotify improvement"

    * tag 'fsnotify_for_v4.19-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/jack/linux-fs:
    inotify: Add flag IN_MASK_CREATE for inotify_add_watch()
    fanotify: factor out helpers to add/remove mark
    fsnotify: add helper to get mask from connector
    fsnotify: let connector point to an abstract object
    fsnotify: pass connp and object type to fsnotify_add_mark()
    fsnotify: use typedef fsnotify_connp_t for brevity

    Linus Torvalds
     

28 Jun, 2018

1 commit


27 Jun, 2018

1 commit

  • Make the code to attach/detach a connector to object more generic
    by letting the fsnotify connector point to an abstract fsnotify_connp_t.
    Code that needs to dereference an inode or mount object now uses the
    helpers fsnotify_conn_{inode,mount}.

    Signed-off-by: Amir Goldstein
    Signed-off-by: Jan Kara

    Amir Goldstein
     

18 May, 2018

2 commits

  • Before changing the arguments of the functions fsnotify_add_mark()
    and fsnotify_add_mark_locked(), convert most callers to use a wrapper.

    Signed-off-by: Amir Goldstein
    Signed-off-by: Jan Kara

    Amir Goldstein
     
  • inode_mark and vfsmount_mark arguments are passed to handle_event()
    operation as function arguments as well as on iter_info struct.
    The difference is that iter_info struct may contain marks that should
    not be handled and are represented as NULL arguments to inode_mark or
    vfsmount_mark.

    Instead of passing the inode_mark and vfsmount_mark arguments, add
    a report_mask member to iter_info struct to indicate which marks should
    be handled, versus marks that should only be kept alive during user
    wait.

    This change is going to be used for passing more mark types
    with handle_event() (i.e. super block marks).

    Signed-off-by: Amir Goldstein
    Signed-off-by: Jan Kara

    Amir Goldstein
     

24 Feb, 2018

1 commit

  • Evidently the __mutex_owner() function was never intended for use
    outside the core mutex code, so build a thing locking wrapper around
    the mutex code which allows us to track the mutex owner.

    One, arguably positive, side effect is that this allows us to hide
    the audit_cmd_mutex inside of kernel/audit.c behind the lock/unlock
    functions.

    Reported-by: Peter Zijlstra
    Reviewed-by: Richard Guy Briggs
    Signed-off-by: Paul Moore

    Paul Moore
     

15 Nov, 2017

1 commit

  • Pull fsnotify updates from Jan Kara:

    - fixes of use-after-tree issues when handling fanotify permission
    events from Miklos

    - refcount_t conversions from Elena

    - fixes of ENOMEM handling in dnotify and fsnotify from me

    * 'fsnotify' of git://git.kernel.org/pub/scm/linux/kernel/git/jack/linux-fs:
    fsnotify: convert fsnotify_mark.refcnt from atomic_t to refcount_t
    fanotify: clean up CONFIG_FANOTIFY_ACCESS_PERMISSIONS ifdefs
    fsnotify: clean up fsnotify()
    fanotify: fix fsnotify_prepare_user_wait() failure
    fsnotify: fix pinning group in fsnotify_prepare_user_wait()
    fsnotify: pin both inode and vfsmount mark
    fsnotify: clean up fsnotify_prepare/finish_user_wait()
    fsnotify: convert fsnotify_group.refcnt from atomic_t to refcount_t
    fsnotify: Protect bail out path of fsnotify_add_mark_locked() properly
    dnotify: Handle errors from fsnotify_add_mark_locked() in fcntl_dirnotify()

    Linus Torvalds
     

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

    Greg Kroah-Hartman
     

01 Nov, 2017

1 commit

  • atomic_t variables are currently used to implement reference
    counters with the following properties:
    - counter is initialized to 1 using atomic_set()
    - a resource is freed upon counter reaching zero
    - once counter reaches zero, its further
    increments aren't allowed
    - counter schema uses basic atomic operations
    (set, inc, inc_not_zero, dec_and_test, etc.)

    Such atomic variables should be converted to a newly provided
    refcount_t type and API that prevents accidental counter overflows
    and underflows. This is important since overflows and underflows
    can lead to use-after-free situation and be exploitable.

    The variable fsnotify_mark.refcnt is used as pure reference counter.
    Convert it to refcount_t and fix up the operations.

    Suggested-by: Kees Cook
    Reviewed-by: David Windsor
    Reviewed-by: Hans Liljestrand
    Signed-off-by: Elena Reshetova
    Signed-off-by: Jan Kara

    Elena Reshetova
     

04 May, 2017

1 commit

  • Pull fsnotify updates from Jan Kara:
    "The branch contains mainly a rework of fsnotify infrastructure fixing
    a shortcoming that we have waited for response to fanotify permission
    events with SRCU read lock held and when the process consuming events
    was slow to respond the kernel has stalled.

    It also contains several cleanups of unnecessary indirections in
    fsnotify framework and a bugfix from Amir fixing leakage of kernel
    internal errno to userspace"

    * 'fsnotify' of git://git.kernel.org/pub/scm/linux/kernel/git/jack/linux-fs: (37 commits)
    fanotify: don't expose EOPENSTALE to userspace
    fsnotify: remove a stray unlock
    fsnotify: Move ->free_mark callback to fsnotify_ops
    fsnotify: Add group pointer in fsnotify_init_mark()
    fsnotify: Drop inode_mark.c
    fsnotify: Remove fsnotify_find_{inode|vfsmount}_mark()
    fsnotify: Remove fsnotify_detach_group_marks()
    fsnotify: Rename fsnotify_clear_marks_by_group_flags()
    fsnotify: Inline fsnotify_clear_{inode|vfsmount}_mark_group()
    fsnotify: Remove fsnotify_recalc_{inode|vfsmount}_mask()
    fsnotify: Remove fsnotify_set_mark_{,ignored_}mask_locked()
    fanotify: Release SRCU lock when waiting for userspace response
    fsnotify: Pass fsnotify_iter_info into handle_event handler
    fsnotify: Provide framework for dropping SRCU lock in ->handle_event
    fsnotify: Remove special handling of mark destruction on group shutdown
    fsnotify: Detach mark from object list when last reference is dropped
    fsnotify: Move queueing of mark for destruction into fsnotify_put_mark()
    inotify: Do not drop mark reference under idr_lock
    fsnotify: Free fsnotify_mark_connector when there is no mark attached
    fsnotify: Lock object list with connector lock
    ...

    Linus Torvalds
     

02 May, 2017

1 commit

  • refcount_t type and corresponding API should be
    used instead of atomic_t when the variable is used as
    a reference counter. This allows to avoid accidental
    refcounter overflows that might lead to use-after-free
    situations.

    Signed-off-by: Elena Reshetova
    Signed-off-by: Hans Liljestrand
    Signed-off-by: Kees Cook
    Signed-off-by: David Windsor
    [PM: fix subject line, add #include]
    Signed-off-by: Paul Moore

    Elena Reshetova
     

10 Apr, 2017

7 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

    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

    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

    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

    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

    Jan Kara
     
  • Move pointer to inode / vfsmount from mark itself to the
    fsnotify_mark_connector structure. This is another step on the path
    towards decoupling inode / vfsmount lifetime from notification mark
    lifetime.

    Reviewed-by: Miklos Szeredi
    Reviewed-by: Amir Goldstein
    Signed-off-by: Jan Kara

    Jan Kara
     
  • Currently audit code uses checking of mark->inode to verify whether mark
    is still alive. Switch that to checking mark flags as that is more
    logical and current way will become unreliable in future.

    Reviewed-by: Miklos Szeredi
    Signed-off-by: Jan Kara

    Jan Kara
     

05 Apr, 2017

1 commit

  • Audit tree currently uses inode pointer as a key into the hash table.
    Getting that from notification mark will be somewhat more difficult with
    coming fsnotify changes. So abstract getting of hash key from the audit
    chunk and inode so that we can change the method to obtain a key easily.

    Reviewed-by: Miklos Szeredi
    CC: Paul Moore
    Acked-by: Paul Moore
    Signed-off-by: Jan Kara

    Jan Kara
     

06 Jan, 2017

1 commit

  • Pull audit fixes from Paul Moore:
    "Two small fixes relating to audit's use of fsnotify.

    The first patch plugs a leak and the second fixes some lock
    shenanigans. The patches are small and I banged on this for an
    afternoon with our testsuite and didn't see anything odd"

    * 'stable-4.10' of git://git.infradead.org/users/pcmoore/audit:
    audit: Fix sleep in atomic
    fsnotify: Remove fsnotify_duplicate_mark()

    Linus Torvalds
     

04 Jan, 2017

1 commit

  • Audit tree code was happily adding new notification marks while holding
    spinlocks. Since fsnotify_add_mark() acquires group->mark_mutex this can
    lead to sleeping while holding a spinlock, deadlocks due to lock
    inversion, and probably other fun. Fix the problem by acquiring
    group->mark_mutex earlier.

    CC: Paul Moore
    Signed-off-by: Jan Kara
    Signed-off-by: Paul Moore

    Jan Kara
     

24 Dec, 2016

1 commit

  • There are only two calls sites of fsnotify_duplicate_mark(). Those are
    in kernel/audit_tree.c and both are bogus. Vfsmount pointer is unused
    for audit tree, inode pointer and group gets set in
    fsnotify_add_mark_locked() later anyway, mask and free_mark are already
    set in alloc_chunk(). In fact, calling fsnotify_duplicate_mark() is
    actively harmful because following fsnotify_add_mark_locked() will leak
    group reference by overwriting the group pointer. So just remove the two
    calls to fsnotify_duplicate_mark() and the function.

    Signed-off-by: Jan Kara
    [PM: line wrapping to fit in 80 chars]
    Signed-off-by: Paul Moore

    Jan Kara
     

18 Dec, 2016

1 commit

  • Pull more vfs updates from Al Viro:
    "In this pile:

    - autofs-namespace series
    - dedupe stuff
    - more struct path constification"

    * 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs: (40 commits)
    ocfs2: implement the VFS clone_range, copy_range, and dedupe_range features
    ocfs2: charge quota for reflinked blocks
    ocfs2: fix bad pointer cast
    ocfs2: always unlock when completing dio writes
    ocfs2: don't eat io errors during _dio_end_io_write
    ocfs2: budget for extent tree splits when adding refcount flag
    ocfs2: prohibit refcounted swapfiles
    ocfs2: add newlines to some error messages
    ocfs2: convert inode refcount test to a helper
    simple_write_end(): don't zero in short copy into uptodate
    exofs: don't mess with simple_write_{begin,end}
    9p: saner ->write_end() on failing copy into non-uptodate page
    fix gfs2_stuffed_write_end() on short copies
    fix ceph_write_end()
    nfs_write_end(): fix handling of short copies
    vfs: refactor clone/dedupe_file_range common functions
    fs: try to clone files first in vfs_copy_file_range
    vfs: misc struct path constification
    namespace.c: constify struct path passed to a bunch of primitives
    quota: constify struct path in quota_on
    ...

    Linus Torvalds