lkml.org 
[lkml]   [2015]   [May]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    Subject[ 48/48] sound/oss: fix deadlock in sequencer_ioctl(SNDCTL_SEQ_OUTOFBAND)
    2.6.32-longterm review patch.  If anyone has any objections, please let me know.

    ------------------

    From: Alexey Khoroshilov <khoroshilov@ispras.ru>

    A deadlock can be initiated by userspace via ioctl(SNDCTL_SEQ_OUTOFBAND)
    on /dev/sequencer with TMR_ECHO midi event.

    In this case the control flow is:
    sound_ioctl()
    -> case SND_DEV_SEQ:
    case SND_DEV_SEQ2:
    sequencer_ioctl()
    -> case SNDCTL_SEQ_OUTOFBAND:
    spin_lock_irqsave(&lock,flags);
    play_event();
    -> case EV_TIMING:
    seq_timing_event()
    -> case TMR_ECHO:
    seq_copy_to_input()
    -> spin_lock_irqsave(&lock,flags);

    It seems that spin_lock_irqsave() around play_event() is not necessary,
    because the only other call location in seq_startplay() makes the call
    without acquiring spinlock.

    So, the patch just removes spinlocks around play_event().
    By the way, it removes unreachable code in seq_timing_event(),
    since (seq_mode == SEQ_2) case is handled in the beginning.

    Compile tested only.

    Found by Linux Driver Verification project (linuxtesting.org).

    Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
    Signed-off-by: Takashi Iwai <tiwai@suse.de>
    (cherry picked from commit bc26d4d06e337ade069f33d3f4377593b24e6e36)
    Signed-off-by: Willy Tarreau <w@1wt.eu>
    ---
    sound/oss/sequencer.c | 12 ++----------
    1 file changed, 2 insertions(+), 10 deletions(-)

    diff --git a/sound/oss/sequencer.c b/sound/oss/sequencer.c
    index 5cb171d..7d32997 100644
    --- a/sound/oss/sequencer.c
    +++ b/sound/oss/sequencer.c
    @@ -677,13 +677,8 @@ static int seq_timing_event(unsigned char *event_rec)
    break;

    case TMR_ECHO:
    - if (seq_mode == SEQ_2)
    - seq_copy_to_input(event_rec, 8);
    - else
    - {
    - parm = (parm << 8 | SEQ_ECHO);
    - seq_copy_to_input((unsigned char *) &parm, 4);
    - }
    + parm = (parm << 8 | SEQ_ECHO);
    + seq_copy_to_input((unsigned char *) &parm, 4);
    break;

    default:;
    @@ -1326,7 +1321,6 @@ int sequencer_ioctl(int dev, struct file *file, unsigned int cmd, void __user *a
    int mode = translate_mode(file);
    struct synth_info inf;
    struct seq_event_rec event_rec;
    - unsigned long flags;
    int __user *p = arg;

    orig_dev = dev = dev >> 4;
    @@ -1481,9 +1475,7 @@ int sequencer_ioctl(int dev, struct file *file, unsigned int cmd, void __user *a
    case SNDCTL_SEQ_OUTOFBAND:
    if (copy_from_user(&event_rec, arg, sizeof(event_rec)))
    return -EFAULT;
    - spin_lock_irqsave(&lock,flags);
    play_event(event_rec.arr);
    - spin_unlock_irqrestore(&lock,flags);
    return 0;

    case SNDCTL_MIDI_INFO:
    --
    1.7.12.2.21.g234cd45.dirty




    \
     
     \ /
      Last update: 2015-05-15 10:41    [W:2.096 / U:0.048 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site