1. 02 Aug, 2016 1 commit
    • Fabian Frederick's avatar
      sysv, ipc: fix security-layer leaking · 9b24fef9
      Fabian Frederick authored
      Commit 53dad6d3 ("ipc: fix race with LSMs") updated ipc_rcu_putref()
      to receive rcu freeing function but used generic ipc_rcu_free() instead
      of msg_rcu_free() which does security cleaning.
      
      Running LTP msgsnd06 with kmemleak gives the following:
      
        cat /sys/kernel/debug/kmemleak
      
        unreferenced object 0xffff88003c0a11f8 (size 8):
          comm "msgsnd06", pid 1645, jiffies 4294672526 (age 6.549s)
          hex dump (first 8 bytes):
            1b 00 00 00 01 00 00 00                          ........
          backtrace:
            kmemleak_alloc+0x23/0x40
            kmem_cache_alloc_trace+0xe1/0x180
            selinux_msg_queue_alloc_security+0x3f/0xd0
            security_msg_queue_alloc+0x2e/0x40
            newque+0x4e/0x150
            ipcget+0x159/0x1b0
            SyS_msgget+0x39/0x40
            entry_SYSCALL_64_fastpath+0x13/0x8f
      
      Manfred Spraul suggested to fix sem.c as well and Davidlohr Bueso to
      only use ipc_rcu_free in case of security allocation failure in newary()
      
      Fixes: 53dad6d3 ("ipc: fix race with LSMs")
      Link: http://lkml.kernel.org/r/1470083552-22966-1-git-send-email-fabf@skynet.beSigned-off-by: default avatarFabian Frederick <fabf@skynet.be>
      Cc: Davidlohr Bueso <dbueso@suse.de>
      Cc: Manfred Spraul <manfred@colorfullife.com>
      Cc: <stable@vger.kernel.org>	[3.12+]
      Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
      Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
      9b24fef9
  2. 30 Sep, 2015 1 commit
    • Linus Torvalds's avatar
      Initialize msg/shm IPC objects before doing ipc_addid() · b9a53227
      Linus Torvalds authored
      As reported by Dmitry Vyukov, we really shouldn't do ipc_addid() before
      having initialized the IPC object state.  Yes, we initialize the IPC
      object in a locked state, but with all the lockless RCU lookup work,
      that IPC object lock no longer means that the state cannot be seen.
      
      We already did this for the IPC semaphore code (see commit e8577d1f:
      "ipc/sem.c: fully initialize sem_array before making it visible") but we
      clearly forgot about msg and shm.
      Reported-by: default avatarDmitry Vyukov <dvyukov@google.com>
      Cc: Manfred Spraul <manfred@colorfullife.com>
      Cc: Davidlohr Bueso <dbueso@suse.de>
      Cc: stable@vger.kernel.org
      Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
      b9a53227
  3. 01 Jul, 2015 2 commits
  4. 15 Apr, 2015 1 commit
  5. 13 Dec, 2014 1 commit
    • Manfred Spraul's avatar
      ipc/msg: increase MSGMNI, remove scaling · 0050ee05
      Manfred Spraul authored
      SysV can be abused to allocate locked kernel memory.  For most systems, a
      small limit doesn't make sense, see the discussion with regards to SHMMAX.
      
      Therefore: increase MSGMNI to the maximum supported.
      
      And: If we ignore the risk of locking too much memory, then an automatic
      scaling of MSGMNI doesn't make sense.  Therefore the logic can be removed.
      
      The code preserves auto_msgmni to avoid breaking any user space applications
      that expect that the value exists.
      
      Notes:
      1) If an administrator must limit the memory allocations, then he can set
      MSGMNI as necessary.
      
      Or he can disable sysv entirely (as e.g. done by Android).
      
      2) MSGMAX and MSGMNB are intentionally not increased, as these values are used
      to control latency vs. throughput:
      If MSGMNB is large, then msgsnd() just returns and more messages can be queued
      before a task switch to a task that calls msgrcv() is forced.
      
      [akpm@linux-foundation.org: coding-style fixes]
      Signed-off-by: default avatarManfred Spraul <manfred@colorfullife.com>
      Cc: Davidlohr Bueso <dave@stgolabs.net>
      Cc: Rafael Aquini <aquini@redhat.com>
      Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
      Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
      0050ee05
  6. 06 Jun, 2014 6 commits
  7. 16 Mar, 2014 1 commit
    • Michael Kerrisk's avatar
      ipc: Fix 2 bugs in msgrcv() MSG_COPY implementation · 4f87dac3
      Michael Kerrisk authored
      While testing and documenting the msgrcv() MSG_COPY flag that Stanislav
      Kinsbursky added in commit 4a674f34 ("ipc: introduce message queue
      copy feature" => kernel 3.8), I discovered a couple of bugs in the
      implementation.  The two bugs concern MSG_COPY interactions with other
      msgrcv() flags, namely:
      
       (A) MSG_COPY + MSG_EXCEPT
       (B) MSG_COPY + !IPC_NOWAIT
      
      The bugs are distinct (and the fix for the first one is obvious),
      however my fix for both is a single-line patch, which is why I'm
      combining them in a single mail, rather than writing two mails+patches.
      
       ===== (A) MSG_COPY + MSG_EXCEPT =====
      
      With the addition of the MSG_COPY flag, there are now two msgrcv()
      flags--MSG_COPY and MSG_EXCEPT--that modify the meaning of the 'msgtyp'
      argument in unrelated ways.  Specifying both in the same call is a
      logical error that is currently permitted, with the effect that MSG_COPY
      has priority and MSG_EXCEPT is ignored.  The call should give an error
      if both flags are specified.  The patch below implements that behavior.
      
       ===== (B) (B) MSG_COPY + !IPC_NOWAIT =====
      
      The test code that was submitted in commit 3a665531 ("selftests: IPC
      message queue copy feature test") shows MSG_COPY being used in
      conjunction with IPC_NOWAIT.  In other words, if there is no message at
      the position 'msgtyp'.  return immediately with the error in ENOMSG.
      
      What was not (fully) tested is the behavior if MSG_COPY is specified
      *without* IPC_NOWAIT, and there is an odd behavior.  If the queue
      contains less than 'msgtyp' messages, then the call blocks until the
      next message is written to the queue.  At that point, the msgrcv() call
      returns a copy of the newly added message, regardless of whether that
      message is at the ordinal position 'msgtyp'.  This is clearly bogus, and
      problematic for applications that might want to make use of the MSG_COPY
      flag.
      
      I considered the following possible solutions to this problem:
      
       (1) Force the call to block until a message *does* appear at the
           position 'msgtyp'.
      
       (2) If the MSG_COPY flag is specified, the kernel should implicitly add
           IPC_NOWAIT, so that the call fails with ENOMSG for this case.
      
       (3) If the MSG_COPY flag is specified, but IPC_NOWAIT is not, generate
           an error (probably, EINVAL is the right one).
      
      I do not know if any application would really want to have the
      functionality of solution (1), especially since an application can
      determine in advance the number of messages in the queue using msgctl()
      IPC_STAT.  Obviously, this solution would be the most work to implement.
      
      Solution (2) would have the effect of silently fixing any applications
      that tried to employ broken behavior.  However, it would mean that if we
      later decided to implement solution (1), then user-space could not
      easily detect what the kernel supports (but, since I'm somewhat doubtful
      that solution (1) is needed, I'm not sure that this is much of a
      problem).
      
      Solution (3) would have the effect of informing broken applications that
      they are doing something broken.  The downside is that this would cause
      a ABI breakage for any applications that are currently employing the
      broken behavior.  However:
      
      a) Those applications are almost certainly not getting the results they
         expect.
      b) Possibly, those applications don't even exist, because MSG_COPY is
         currently hidden behind CONFIG_CHECKPOINT_RESTORE.
      
      The upside of solution (3) is that if we later decided to implement
      solution (1), user-space could determine what the kernel supports, via
      the error return.
      
      In my view, solution (3) is mildly preferable to solution (2), and
      solution (1) could still be done later if anyone really cares.  The
      patch below implements solution (3).
      
      PS.  For anyone out there still listening, it's the usual story:
      documenting an API (and the thinking about, and the testing of the API,
      that documentation entails) is the one of the single best ways of
      finding bugs in the API, as I've learned from a lot of experience.  Best
      to do that documentation before releasing the API.
      Signed-off-by: default avatarMichael Kerrisk <mtk.manpages@gmail.com>
      Acked-by: default avatarStanislav Kinsbursky <skinsbursky@parallels.com>
      Cc: Stanislav Kinsbursky <skinsbursky@parallels.com>
      Cc: stable@vger.kernel.org
      Cc: Serge Hallyn <serge.hallyn@canonical.com>
      Cc: "Eric W. Biederman" <ebiederm@xmission.com>
      Cc: Pavel Emelyanov <xemul@parallels.com>
      Cc: Al Viro <viro@zeniv.linux.org.uk>
      Cc: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
      Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
      4f87dac3
  8. 28 Jan, 2014 3 commits
  9. 30 Sep, 2013 1 commit
  10. 24 Sep, 2013 1 commit
    • Davidlohr Bueso's avatar
      ipc: fix race with LSMs · 53dad6d3
      Davidlohr Bueso authored
      Currently, IPC mechanisms do security and auditing related checks under
      RCU.  However, since security modules can free the security structure,
      for example, through selinux_[sem,msg_queue,shm]_free_security(), we can
      race if the structure is freed before other tasks are done with it,
      creating a use-after-free condition.  Manfred illustrates this nicely,
      for instance with shared mem and selinux:
      
       -> do_shmat calls rcu_read_lock()
       -> do_shmat calls shm_object_check().
           Checks that the object is still valid - but doesn't acquire any locks.
           Then it returns.
       -> do_shmat calls security_shm_shmat (e.g. selinux_shm_shmat)
       -> selinux_shm_shmat calls ipc_has_perm()
       -> ipc_has_perm accesses ipc_perms->security
      
      shm_close()
       -> shm_close acquires rw_mutex & shm_lock
       -> shm_close calls shm_destroy
       -> shm_destroy calls security_shm_free (e.g. selinux_shm_free_security)
       -> selinux_shm_free_security calls ipc_free_security(&shp->shm_perm)
       -> ipc_free_security calls kfree(ipc_perms->security)
      
      This patch delays the freeing of the security structures after all RCU
      readers are done.  Furthermore it aligns the security life cycle with
      that of the rest of IPC - freeing them based on the reference counter.
      For situations where we need not free security, the current behavior is
      kept.  Linus states:
      
       "... the old behavior was suspect for another reason too: having the
        security blob go away from under a user sounds like it could cause
        various other problems anyway, so I think the old code was at least
        _prone_ to bugs even if it didn't have catastrophic behavior."
      
      I have tested this patch with IPC testcases from LTP on both my
      quad-core laptop and on a 64 core NUMA server.  In both cases selinux is
      enabled, and tests pass for both voluntary and forced preemption models.
      While the mentioned races are theoretical (at least no one as reported
      them), I wanted to make sure that this new logic doesn't break anything
      we weren't aware of.
      Suggested-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
      Signed-off-by: default avatarDavidlohr Bueso <davidlohr@hp.com>
      Acked-by: default avatarManfred Spraul <manfred@colorfullife.com>
      Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
      53dad6d3
  11. 11 Sep, 2013 2 commits
  12. 03 Sep, 2013 1 commit
    • Manfred Spraul's avatar
      ipc/msg.c: Fix lost wakeup in msgsnd(). · bebcb928
      Manfred Spraul authored
      The check if the queue is full and adding current to the wait queue of
      pending msgsnd() operations (ss_add()) must be atomic.
      
      Otherwise:
       - the thread that performs msgsnd() finds a full queue and decides to
         sleep.
       - the thread that performs msgrcv() first reads all messages from the
         queue and then sleeps, because the queue is empty.
       - the msgrcv() calls do not perform any wakeups, because the msgsnd()
         task has not yet called ss_add().
       - then the msgsnd()-thread first calls ss_add() and then sleeps.
      
      Net result: msgsnd() and msgrcv() both sleep forever.
      
      Observed with msgctl08 from ltp with a preemptible kernel.
      
      Fix: Call ipc_lock_object() before performing the check.
      
      The patch also moves security_msg_queue_msgsnd() under ipc_lock_object:
       - msgctl(IPC_SET) explicitely mentions that it tries to expunge any
         pending operations that are not allowed anymore with the new
         permissions.  If security_msg_queue_msgsnd() is called without locks,
         then there might be races.
       - it makes the patch much simpler.
      Reported-and-tested-by: default avatarVineet Gupta <Vineet.Gupta1@synopsys.com>
      Acked-by: default avatarRik van Riel <riel@redhat.com>
      Cc: stable@vger.kernel.org  # for 3.11
      Signed-off-by: default avatarManfred Spraul <manfred@colorfullife.com>
      Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
      bebcb928
  13. 29 Aug, 2013 1 commit
  14. 09 Jul, 2013 10 commits
  15. 01 May, 2013 6 commits
  16. 02 Apr, 2013 1 commit
  17. 08 Mar, 2013 1 commit