ALSA: seq: Fix possible UAF in snd_seq_check_queue()
authorTakashi Iwai <tiwai@suse.de>
Fri, 9 Mar 2018 20:58:28 +0000 (21:58 +0100)
committerBen Hutchings <ben@decadent.org.uk>
Thu, 31 May 2018 23:30:21 +0000 (00:30 +0100)
commitdeeaca310dc1d79b70384810c47b64b550f6bef7
treeb1f66e84d58b1c8c68ca5560dcfd88fa6bd164e8
parent47e0a4def4d2e1dfcef1fcf1b78706c24584351b
ALSA: seq: Fix possible UAF in snd_seq_check_queue()

commit d0f833065221cbfcbadf19fd4102bcfa9330006a upstream.

Although we've covered the races between concurrent write() and
ioctl() in the previous patch series, there is still a possible UAF in
the following scenario:

A: user client closed B: timer irq
  -> snd_seq_release()   -> snd_seq_timer_interrupt()
    -> snd_seq_free_client()     -> snd_seq_check_queue()
      -> cell = snd_seq_prioq_cell_peek()
      -> snd_seq_prioq_leave()
         .... removing all cells
      -> snd_seq_pool_done()
         .... vfree()
      -> snd_seq_compare_tick_time(cell)
         ... Oops

So the problem is that a cell is peeked and accessed without any
protection until it's retrieved from the queue again via
snd_seq_prioq_cell_out().

This patch tries to address it, also cleans up the code by a slight
refactoring.  snd_seq_prioq_cell_out() now receives an extra pointer
argument.  When it's non-NULL, the function checks the event timestamp
with the given pointer.  The caller needs to pass the right reference
either to snd_seq_tick or snd_seq_realtime depending on the event
timestamp type.

A good news is that the above change allows us to remove the
snd_seq_prioq_cell_peek(), too, thus the patch actually reduces the
code size.

Reviewed-by: Nicolai Stange <nstange@suse.de>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
[bwh: Backported to 3.2: Deleted function had different log message]
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
sound/core/seq/seq_prioq.c
sound/core/seq/seq_prioq.h
sound/core/seq/seq_queue.c