1. 16 6月, 2017 1 次提交
  2. 14 6月, 2017 1 次提交
    • P
      audit: fix a race condition with the auditd tracking code · c81be52a
      Paul Moore 提交于
      Originally reported by Adam and Dusty, it appears we have a small
      race window in kauditd_thread(), as documented in the Fedora BZ:
      
       * https://bugzilla.redhat.com/show_bug.cgi?id=1459326#c35
      
       "This issue is partly due to the read-copy nature of RCU, and
        partly due to how we sync the auditd_connection state across
        kauditd_thread and the audit control channel.  The kauditd_thread
        thread is always running so it can service the record queues and
        emit the multicast messages, if it happens to be just past the
        "main_queue" label, but before the "if (sk == NULL || ...)"
        if-statement which calls auditd_reset() when the new auditd
        connection is registered it could end up resetting the auditd
        connection, regardless of if it is valid or not.  This is a rather
        small window and the variable nature of multi-core scheduling
        explains why this is proving rather difficult to reproduce."
      
      The fix is to have functions only call auditd_reset() when they
      believe that the kernel/auditd connection is still valid, e.g.
      non-NULL, and to have these callers pass their local copy of the
      auditd_connection pointer to auditd_reset() where it can be compared
      with the current connection state before resetting.  If the caller
      has a stale state tracking pointer then the reset is ignored.
      
      We also make a small change to kauditd_thread() so that if the
      kernel/auditd connection is dead we skip the retry queue and send the
      records straight to the hold queue.  This is necessary as we used to
      rely on auditd_reset() to occasionally purge the retry queue but we
      are going to be calling the reset function much less now and we want
      to make sure the retry queue doesn't grow unbounded.
      Reported-by: NAdam Williamson <awilliam@redhat.com>
      Reported-by: NDusty Mabe <dustymabe@redhat.com>
      Reviewed-by: NRichard Guy Briggs <rgb@redhat.com>
      Signed-off-by: NPaul Moore <paul@paul-moore.com>
      c81be52a
  3. 24 5月, 2017 1 次提交
    • R
      audit: unswing cap_* fields in PATH records · 4b3e4ed6
      Richard Guy Briggs 提交于
      The cap_* fields swing in and out of PATH records.
      If no capabilities are set, the cap_* fields are completely missing and when
      one of the cap_fi or cap_fp values is empty, that field is omitted.
      
      Original:
      type=PATH msg=audit(04/20/2017 12:17:11.222:193) : item=1 name=/lib64/ld-linux-x86-64.so.2 inode=787694 dev=08:03 mode=file,755 ouid=root ogid=root rdev=00:00 obj=system_u:object_r:ld_so_t:s0 nametype=NORMAL
      type=PATH msg=audit(04/20/2017 12:17:11.222:193) : item=0 name=/home/sleep inode=1319469 dev=08:03 mode=file,suid,755 ouid=root ogid=root rdev=00:00 obj=system_u:object_r:bin_t:s0 nametype=NORMAL cap_fp=sys_admin cap_fe=1 cap_fver=2
      
      Normalize the PATH record by always printing all 4 cap_* fields.
      
      Fixed:
      type=PATH msg=audit(04/20/2017 13:01:31.679:201) : item=1 name=/lib64/ld-linux-x86-64.so.2 inode=787694 dev=08:03 mode=file,755 ouid=root ogid=root rdev=00:00 obj=system_u:object_r:ld_so_t:s0 nametype=NORMAL cap_fp=none cap_fi=none cap_fe=0 cap_fver=0
      type=PATH msg=audit(04/20/2017 13:01:31.679:201) : item=0 name=/home/sleep inode=1319469 dev=08:03 mode=file,suid,755 ouid=root ogid=root rdev=00:00 obj=system_u:object_r:bin_t:s0 nametype=NORMAL cap_fp=sys_admin cap_fi=none cap_fe=1 cap_fver=2
      
      See: https://github.com/linux-audit/audit-kernel/issues/42Signed-off-by: NRichard Guy Briggs <rgb@redhat.com>
      Signed-off-by: NPaul Moore <paul@paul-moore.com>
      4b3e4ed6
  4. 02 5月, 2017 6 次提交
  5. 14 4月, 2017 1 次提交
  6. 10 4月, 2017 1 次提交
    • P
      audit: make sure we don't let the retry queue grow without bounds · 264d5096
      Paul Moore 提交于
      The retry queue is intended to provide a temporary buffer in the case
      of transient errors when communicating with auditd, it is not meant
      as a long life queue, that functionality is provided by the hold
      queue.
      
      This patch fixes a problem identified by Seth where the retry queue
      could grow uncontrollably if an auditd instance did not connect to
      the kernel to drain the queues.  This commit fixes this by doing the
      following:
      
      * Make sure we always call auditd_reset() if we decide the connection
      with audit is really dead.  There were some cases in
      kauditd_hold_skb() where we did not reset the connection, this patch
      relocates the reset calls to kauditd_thread() so all the error
      conditions are caught and the connection reset.  As a side effect,
      this means we could move auditd_reset() and get rid of the forward
      definition at the top of kernel/audit.c.
      
      * We never checked the status of the auditd connection when
      processing the main audit queue which meant that the retry queue
      could grow unchecked.  This patch adds a call to auditd_reset()
      after the main queue has been processed if auditd is not connected,
      the auditd_reset() call will make sure the retry and hold queues are
      correctly managed/flushed so that the retry queue remains reasonable.
      
      Cc: <stable@vger.kernel.org> # 4.10.x-: 5b52330bReported-by: NSeth Forshee <seth.forshee@canonical.com>
      Signed-off-by: NPaul Moore <paul@paul-moore.com>
      264d5096
  7. 21 3月, 2017 1 次提交
    • P
      audit: fix auditd/kernel connection state tracking · 5b52330b
      Paul Moore 提交于
      What started as a rather straightforward race condition reported by
      Dmitry using the syzkaller fuzzer ended up revealing some major
      problems with how the audit subsystem managed its netlink sockets and
      its connection with the userspace audit daemon.  Fixing this properly
      had quite the cascading effect and what we are left with is this rather
      large and complicated patch.  My initial goal was to try and decompose
      this patch into multiple smaller patches, but the way these changes
      are intertwined makes it difficult to split these changes into
      meaningful pieces that don't break or somehow make things worse for
      the intermediate states.
      
      The patch makes a number of changes, but the most significant are
      highlighted below:
      
      * The auditd tracking variables, e.g. audit_sock, are now gone and
      replaced by a RCU/spin_lock protected variable auditd_conn which is
      a structure containing all of the auditd tracking information.
      
      * We no longer track the auditd sock directly, instead we track it
      via the network namespace in which it resides and we use the audit
      socket associated with that namespace.  In spirit, this is what the
      code was trying to do prior to this patch (at least I think that is
      what the original authors intended), but it was done rather poorly
      and added a layer of obfuscation that only masked the underlying
      problems.
      
      * Big backlog queue cleanup, again.  In v4.10 we made some pretty big
      changes to how the audit backlog queues work, here we haven't changed
      the queue design so much as cleaned up the implementation.  Brought
      about by the locking changes, we've simplified kauditd_thread() quite
      a bit by consolidating the queue handling into a new helper function,
      kauditd_send_queue(), which allows us to eliminate a lot of very
      similar code and makes the looping logic in kauditd_thread() clearer.
      
      * All netlink messages sent to auditd are now sent via
      auditd_send_unicast_skb().  Other than just making sense, this makes
      the lock handling easier.
      
      * Change the audit_log_start() sleep behavior so that we never sleep
      on auditd events (unchanged) or if the caller is holding the
      audit_cmd_mutex (changed).  Previously we didn't sleep if the caller
      was auditd or if the message type fell between a certain range; the
      type check was a poor effort of doing what the cmd_mutex check now
      does.  Richard Guy Briggs originally proposed not sleeping the
      cmd_mutex owner several years ago but his patch wasn't acceptable
      at the time.  At least the idea lives on here.
      
      * A problem with the lost record counter has been resolved.  Steve
      Grubb and I both happened to notice this problem and according to
      some quick testing by Steve, this problem goes back quite some time.
      It's largely a harmless problem, although it may have left some
      careful sysadmins quite puzzled.
      
      Cc: <stable@vger.kernel.org> # 4.10.x-
      Reported-by: NDmitry Vyukov <dvyukov@google.com>
      Signed-off-by: NPaul Moore <paul@paul-moore.com>
      5b52330b
  8. 19 1月, 2017 1 次提交
  9. 15 12月, 2016 11 次提交
  10. 06 12月, 2016 1 次提交
  11. 02 12月, 2016 1 次提交
  12. 18 11月, 2016 1 次提交
    • A
      netns: make struct pernet_operations::id unsigned int · c7d03a00
      Alexey Dobriyan 提交于
      Make struct pernet_operations::id unsigned.
      
      There are 2 reasons to do so:
      
      1)
      This field is really an index into an zero based array and
      thus is unsigned entity. Using negative value is out-of-bound
      access by definition.
      
      2)
      On x86_64 unsigned 32-bit data which are mixed with pointers
      via array indexing or offsets added or subtracted to pointers
      are preffered to signed 32-bit data.
      
      "int" being used as an array index needs to be sign-extended
      to 64-bit before being used.
      
      	void f(long *p, int i)
      	{
      		g(p[i]);
      	}
      
        roughly translates to
      
      	movsx	rsi, esi
      	mov	rdi, [rsi+...]
      	call 	g
      
      MOVSX is 3 byte instruction which isn't necessary if the variable is
      unsigned because x86_64 is zero extending by default.
      
      Now, there is net_generic() function which, you guessed it right, uses
      "int" as an array index:
      
      	static inline void *net_generic(const struct net *net, int id)
      	{
      		...
      		ptr = ng->ptr[id - 1];
      		...
      	}
      
      And this function is used a lot, so those sign extensions add up.
      
      Patch snipes ~1730 bytes on allyesconfig kernel (without all junk
      messing with code generation):
      
      	add/remove: 0/0 grow/shrink: 70/598 up/down: 396/-2126 (-1730)
      
      Unfortunately some functions actually grow bigger.
      This is a semmingly random artefact of code generation with register
      allocator being used differently. gcc decides that some variable
      needs to live in new r8+ registers and every access now requires REX
      prefix. Or it is shifted into r12, so [r12+0] addressing mode has to be
      used which is longer than [r8]
      
      However, overall balance is in negative direction:
      
      	add/remove: 0/0 grow/shrink: 70/598 up/down: 396/-2126 (-1730)
      	function                                     old     new   delta
      	nfsd4_lock                                  3886    3959     +73
      	tipc_link_build_proto_msg                   1096    1140     +44
      	mac80211_hwsim_new_radio                    2776    2808     +32
      	tipc_mon_rcv                                1032    1058     +26
      	svcauth_gss_legacy_init                     1413    1429     +16
      	tipc_bcbase_select_primary                   379     392     +13
      	nfsd4_exchange_id                           1247    1260     +13
      	nfsd4_setclientid_confirm                    782     793     +11
      		...
      	put_client_renew_locked                      494     480     -14
      	ip_set_sockfn_get                            730     716     -14
      	geneve_sock_add                              829     813     -16
      	nfsd4_sequence_done                          721     703     -18
      	nlmclnt_lookup_host                          708     686     -22
      	nfsd4_lockt                                 1085    1063     -22
      	nfs_get_client                              1077    1050     -27
      	tcf_bpf_init                                1106    1076     -30
      	nfsd4_encode_fattr                          5997    5930     -67
      	Total: Before=154856051, After=154854321, chg -0.00%
      Signed-off-by: NAlexey Dobriyan <adobriyan@gmail.com>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      c7d03a00
  13. 31 8月, 2016 1 次提交
  14. 29 6月, 2016 1 次提交
  15. 27 6月, 2016 1 次提交
  16. 27 4月, 2016 1 次提交
  17. 05 4月, 2016 1 次提交
  18. 28 1月, 2016 2 次提交
  19. 26 1月, 2016 2 次提交
    • R
      audit: log failed attempts to change audit_pid configuration · 935c9e7f
      Richard Guy Briggs 提交于
      Failed attempts to change the audit_pid configuration are not presently
      logged.  One case is an attempt to starve an old auditd by starting up
      a new auditd when the old one is still alive and active.  The other
      case is an attempt to orphan a new auditd when an old auditd shuts
      down.
      
      Log both as AUDIT_CONFIG_CHANGE messages with failure result.
      Signed-off-by: NRichard Guy Briggs <rgb@redhat.com>
      Signed-off-by: NPaul Moore <pmoore@redhat.com>
      935c9e7f
    • R
      audit: stop an old auditd being starved out by a new auditd · 133e1e5a
      Richard Guy Briggs 提交于
      Nothing prevents a new auditd starting up and replacing a valid
      audit_pid when an old auditd is still running, effectively starving out
      the old auditd since audit_pid no longer points to the old valid
      auditd.
      
      If no message to auditd has been attempted since auditd died
      unnaturally or got killed, audit_pid will still indicate it is alive.
      There isn't an easy way to detect if an old auditd is still running on
      the existing audit_pid other than attempting to send a message to see
      if it fails.  An -ECONNREFUSED almost certainly means it disappeared
      and can be replaced.  Other errors are not so straightforward and may
      indicate transient problems that will resolve themselves and the old
      auditd will recover.  Yet others will likely need manual intervention
      for which a new auditd will not solve the problem.
      
      Send a new message type (AUDIT_REPLACE) to the old auditd containing a
      u32 with the PID of the new auditd.  If the audit replace message
      succeeds (or doesn't fail with certainty), fail to register the new
      auditd and return an error (-EEXIST).
      
      This is expected to make the patch preventing an old auditd orphaning a
      new auditd redundant.
      
      V3: Switch audit message type from 1000 to 1300 block.
      Signed-off-by: NRichard Guy Briggs <rgb@redhat.com>
      Signed-off-by: NPaul Moore <pmoore@redhat.com>
      133e1e5a
  20. 13 1月, 2016 4 次提交