linux.git/ipc/msg.c, branch v3.18.111

ipc: msg, make msgrcv work with LONG_MIN

2018-01-31T13:46:13+00:00

commit 999898355e08ae3b92dfd0a08db706e0c6703d30 upstream.

When LONG_MIN is passed to msgrcv, one would expect to recieve any
message.  But convert_mode does *msgtyp = -*msgtyp and -LONG_MIN is
undefined.  In particular, with my gcc -LONG_MIN produces -LONG_MIN
again.

So handle this case properly by assigning LONG_MAX to *msgtyp if
LONG_MIN was specified as msgtyp to msgrcv.

This code:
  long msg[] = { 100, 200 };
  int m = msgget(IPC_PRIVATE, IPC_CREAT | 0644);
  msgsnd(m, &msg, sizeof(msg), 0);
  msgrcv(m, &msg, sizeof(msg), LONG_MIN, 0);

produces currently nothing:

  msgget(IPC_PRIVATE, IPC_CREAT|0644)     = 65538
  msgsnd(65538, {100, "\310\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"}, 16, 0) = 0
  msgrcv(65538, ...

Except a UBSAN warning:

  UBSAN: Undefined behaviour in ipc/msg.c:745:13
  negation of -9223372036854775808 cannot be represented in type 'long int':

With the patch, I see what I expect:

  msgget(IPC_PRIVATE, IPC_CREAT|0644)     = 0
  msgsnd(0, {100, "\310\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"}, 16, 0) = 0
  msgrcv(0, {100, "\310\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"}, 16, -9223372036854775808, 0) = 16

Link: http://lkml.kernel.org/r/20161024082633.10148-1-jslaby@suse.cz
Signed-off-by: Jiri Slaby 
Cc: Davidlohr Bueso 
Cc: Manfred Spraul 
Signed-off-by: Andrew Morton 
Signed-off-by: Linus Torvalds 
Signed-off-by: Greg Kroah-Hartman

sysv, ipc: fix security-layer leaking

2016-08-22T16:23:27+00:00

[ Upstream commit 9b24fef9f0410fb5364245d6cc2bd044cc064007 ]

Commit 53dad6d3a8e5 ("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: 53dad6d3a8e ("ipc: fix race with LSMs")
Link: http://lkml.kernel.org/r/1470083552-22966-1-git-send-email-fabf@skynet.be
Signed-off-by: Fabian Frederick 
Cc: Davidlohr Bueso 
Cc: Manfred Spraul 
Cc: 	[3.12+]
Signed-off-by: Andrew Morton 
Signed-off-by: Linus Torvalds 
Signed-off-by: Sasha Levin

Initialize msg/shm IPC objects before doing ipc_addid()

2015-10-28T02:13:50+00:00

[ Upstream commit b9a532277938798b53178d5a66af6e2915cb27cf ]

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 e8577d1f0329:
"ipc/sem.c: fully initialize sem_array before making it visible") but we
clearly forgot about msg and shm.

Reported-by: Dmitry Vyukov 
Cc: Manfred Spraul 
Cc: Davidlohr Bueso 
Cc: stable@vger.kernel.org
Signed-off-by: Linus Torvalds 
Signed-off-by: Sasha Levin

ipc,msg: document volatile r_msg

2014-06-06T23:08:15+00:00

The need for volatile is not obvious, document it.

Signed-off-by: Davidlohr Bueso 
Signed-off-by: Manfred Spraul 
Cc: Aswin Chandramouleeswaran 
Signed-off-by: Andrew Morton 
Signed-off-by: Linus Torvalds

ipc,msg: move some msgq ns code around

2014-06-06T23:08:14+00:00

Nothing big and no logical changes, just get rid of some redundant
function declarations.  Move msg_[init/exit]_ns down the end of the
file.

Signed-off-by: Davidlohr Bueso 
Signed-off-by: Manfred Spraul 
Cc: Aswin Chandramouleeswaran 
Signed-off-by: Andrew Morton 
Signed-off-by: Linus Torvalds

ipc,msg: use current->state helpers

2014-06-06T23:08:14+00:00

Call __set_current_state() instead of assigning the new state directly.

Signed-off-by: Davidlohr Bueso 
Signed-off-by: Manfred Spraul 
Cc: Aswin Chandramouleeswaran 
Signed-off-by: Andrew Morton 
Signed-off-by: Linus Torvalds

ipc, kernel: clear whitespace

2014-06-06T23:08:14+00:00

trailing whitespace

Signed-off-by: Paul McQuade 
Signed-off-by: Andrew Morton 
Signed-off-by: Linus Torvalds

ipc, kernel: use Linux headers

2014-06-06T23:08:14+00:00

Use #include  instead of 
Use #include  instead of 

Signed-off-by: Paul McQuade 
Signed-off-by: Andrew Morton 
Signed-off-by: Linus Torvalds

ipc: constify ipc_ops

2014-06-06T23:08:14+00:00

There is no need to recreate the very same ipc_ops structure on every
kernel entry for msgget/semget/shmget.  Just declare it static and be
done with it.  While at it, constify it as we don't modify the structure
at runtime.

Found in the PaX patch, written by the PaX Team.

Signed-off-by: Mathias Krause 
Cc: PaX Team 
Cc: Davidlohr Bueso 
Cc: Manfred Spraul 
Signed-off-by: Andrew Morton 
Signed-off-by: Linus Torvalds

ipc: Fix 2 bugs in msgrcv() MSG_COPY implementation

2014-03-16T17:41:04+00:00

While testing and documenting the msgrcv() MSG_COPY flag that Stanislav
Kinsbursky added in commit 4a674f34ba04 ("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 3a665531a3b7 ("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: Michael Kerrisk 
Acked-by: Stanislav Kinsbursky 
Cc: Stanislav Kinsbursky 
Cc: stable@vger.kernel.org
Cc: Serge Hallyn 
Cc: "Eric W. Biederman" 
Cc: Pavel Emelyanov 
Cc: Al Viro 
Cc: KOSAKI Motohiro 
Signed-off-by: Linus Torvalds