]> git.itanic.dy.fi Git - linux-stable/commit
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)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Thu, 22 Mar 2018 08:37:17 +0000 (09:37 +0100)
commit92d0346b53a8deda74b6b1b8788602ec86946cf3
tree9fedf536b97f4581d722cae555ad1f05c9c2849e
parent8257ab7d6ff0fa0b8758074ad051c700d3bde216
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>
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
sound/core/seq/seq_prioq.c
sound/core/seq/seq_prioq.h
sound/core/seq/seq_queue.c