28 Sep, 2022

1 commit

  • commit c3afa2a402d1ecefa59f88d55d9e765f52f75bd9 upstream.

    During the code change to add the support for devres-managed card
    instance, we put an explicit kfree(card) call at the error path in
    snd_card_new(). This is needed for the early error path before the
    card is initialized with the device, but is rather superfluous and
    causes a double-free at the error path after the card instance is
    initialized, as the destructor of the card object already contains a
    kfree() call.

    This patch fixes the double-free situation by removing the superfluous
    kfree(). Meanwhile we need to call kfree() explicitly for the early
    error path, so it's added there instead.

    Fixes: e8ad415b7a55 ("ALSA: core: Add managed card creation")
    Reported-by: Rondreis
    Cc:
    Link: https://lore.kernel.org/r/CAB7eexL1zBnB636hwS27d-LdPYZ_R1-5fJS_h=ZbCWYU=UPWJg@mail.gmail.com
    Link: https://lore.kernel.org/r/20220919123516.28222-1-tiwai@suse.de
    Signed-off-by: Takashi Iwai
    Signed-off-by: Greg Kroah-Hartman

    Takashi Iwai
     

15 Sep, 2022

1 commit

  • commit 8423f0b6d513b259fdab9c9bf4aaa6188d054c2d upstream.

    There is a small race window at snd_pcm_oss_sync() that is called from
    OSS PCM SNDCTL_DSP_SYNC ioctl; namely the function calls
    snd_pcm_oss_make_ready() at first, then takes the params_lock mutex
    for the rest. When the stream is set up again by another thread
    between them, it leads to inconsistency, and may result in unexpected
    results such as NULL dereference of OSS buffer as a fuzzer spotted
    recently.

    The fix is simply to cover snd_pcm_oss_make_ready() call into the same
    params_lock mutex with snd_pcm_oss_make_ready_locked() variant.

    Reported-and-tested-by: butt3rflyh4ck
    Reviewed-by: Jaroslav Kysela
    Cc:
    Link: https://lore.kernel.org/r/CAFcO6XN7JDM4xSXGhtusQfS2mSBcx50VJKwQpCq=WeLt57aaZA@mail.gmail.com
    Link: https://lore.kernel.org/r/20220905060714.22549-1-tiwai@suse.de
    Signed-off-by: Takashi Iwai
    Signed-off-by: Greg Kroah-Hartman

    Takashi Iwai
     

08 Sep, 2022

2 commits

  • commit 3e7e04b747adea36f349715d9f0998eeebf15d72 upstream.

    It's been reported that there is a possible data-race accessing to the
    global card_requested[] array at ALSA sequencer core, which is used
    for determining whether to call request_module() for the card or not.
    This data race itself is almost harmless, as it might end up with one
    extra request_module() call for the already loaded module at most.
    But it's still better to fix.

    This patch addresses the possible data race of card_requested[] and
    client_requested[] arrays by replacing them with bitmask.
    It's an atomic operation and can work without locks.

    Reported-by: Abhishek Shah
    Cc:
    Link: https://lore.kernel.org/r/CAEHB24_ay6YzARpA1zgCsE7=H9CSJJzux618E=Ka4h0YdKn=qA@mail.gmail.com
    Link: https://lore.kernel.org/r/20220823072717.1706-2-tiwai@suse.de
    Signed-off-by: Takashi Iwai
    Signed-off-by: Greg Kroah-Hartman

    Takashi Iwai
     
  • commit 22dec134dbfa825b963f8a1807ad19b943e46a56 upstream.

    ALSA OSS sequencer refers to a global variable max_midi_devs at
    creating a new port, storing it to its own field. Meanwhile this
    variable may be changed by other sequencer events at
    snd_seq_oss_midi_check_exit_port() in parallel, which may cause a data
    race.

    OTOH, this data race itself is almost harmless, as the access to the
    MIDI device is done via get_mdev() and it's protected with a refcount,
    hence its presence is guaranteed.

    Though, it's sill better to address the data-race from the code sanity
    POV, and this patch adds the proper spinlock for the protection.

    Reported-by: Abhishek Shah
    Cc:
    Link: https://lore.kernel.org/r/CAEHB2493pZRXs863w58QWnUTtv3HHfg85aYhLn5HJHCwxqtHQg@mail.gmail.com
    Link: https://lore.kernel.org/r/20220823072717.1706-1-tiwai@suse.de
    Signed-off-by: Takashi Iwai
    Signed-off-by: Greg Kroah-Hartman

    Takashi Iwai
     

25 Aug, 2022

4 commits

  • [ Upstream commit 4a971e84a7ae10a38d875cd2d4e487c8d1682ca3 ]

    For avoiding the potential deadlock via kill_fasync() call, use the
    new fasync helpers to defer the invocation from the control API. Note
    that it's merely a workaround.

    Another note: although we haven't received reports about the deadlock
    with the control API, the deadlock is still potentially possible, and
    it's better to align the behavior with other core APIs (PCM and
    timer); so let's move altogether.

    Link: https://lore.kernel.org/r/20220728125945.29533-5-tiwai@suse.de
    Signed-off-by: Takashi Iwai
    Signed-off-by: Sasha Levin

    Takashi Iwai
     
  • [ Upstream commit 95cc637c1afd83fb7dd3d7c8a53710488f4caf9c ]

    For avoiding the potential deadlock via kill_fasync() call, use the
    new fasync helpers to defer the invocation from PCI API. Note that
    it's merely a workaround.

    Reported-by: syzbot+1ee0910eca9c94f71f25@syzkaller.appspotmail.com
    Reported-by: syzbot+49b10793b867871ee26f@syzkaller.appspotmail.com
    Reported-by: syzbot+8285e973a41b5aa68902@syzkaller.appspotmail.com
    Link: https://lore.kernel.org/r/20220728125945.29533-3-tiwai@suse.de
    Signed-off-by: Takashi Iwai
    Signed-off-by: Sasha Levin

    Takashi Iwai
     
  • [ Upstream commit ef34a0ae7a2654bc9e58675e36898217fb2799d8 ]

    Currently the call of kill_fasync() from an interrupt handler might
    lead to potential spin deadlocks, as spotted by syzkaller.
    Unfortunately, it's not so trivial to fix this lock chain as it's
    involved with the tasklist_lock that is touched in allover places.

    As a temporary workaround, this patch provides the way to defer the
    async signal notification in a work. The new helper functions,
    snd_fasync_helper() and snd_kill_faync() are replacements for
    fasync_helper() and kill_fasync(), respectively. In addition,
    snd_fasync_free() needs to be called at the destructor of the relevant
    file object.

    Link: https://lore.kernel.org/r/20220728125945.29533-2-tiwai@suse.de
    Signed-off-by: Takashi Iwai
    Signed-off-by: Sasha Levin

    Takashi Iwai
     
  • commit 9be080edcca330be4af06b19916c35227891e8bc upstream.

    When using callback there was a flow of

    ret = -EINVAL
    if (callback) {
    offset = callback();
    goto out;
    }
    ...
    offset = some other value in case of no callback;
    ret = offset;
    out:
    return ret;

    which causes the snd_info_entry_llseek() to return -EINVAL when there is
    callback handler. Fix this by setting "ret" directly to callback return
    value before jumping to "out".

    Fixes: 73029e0ff18d ("ALSA: info - Implement common llseek for binary mode")
    Signed-off-by: Amadeusz Sławiński
    Cc:
    Link: https://lore.kernel.org/r/20220817124924.3974577-1-amadeuszx.slawinski@linux.intel.com
    Signed-off-by: Takashi Iwai
    Signed-off-by: Greg Kroah-Hartman

    Amadeusz Sławiński
     

09 Jun, 2022

2 commits

  • [ Upstream commit 011b559be832194f992f73d6c0d5485f5925a10b ]

    Pointer substream is being dereferenced on the assignment of pointer card
    before substream is being null checked with the macro PCM_RUNTIME_CHECK.
    Although PCM_RUNTIME_CHECK calls BUG_ON, it still is useful to perform the
    the pointer check before card is assigned.

    Fixes: d4cfb30fce03 ("ALSA: pcm: Set per-card upper limit of PCM buffer allocations")
    Signed-off-by: Colin Ian King
    Link: https://lore.kernel.org/r/20220424205945.1372247-1-colin.i.king@gmail.com
    Signed-off-by: Takashi Iwai
    Signed-off-by: Sasha Levin

    Colin Ian King
     
  • [ Upstream commit 1b6a6fc5280e97559287b61eade2d4b363e836f2 ]

    It is possible when using ASoC that input_dev is unregistered while
    calling snd_jack_report, which causes NULL pointer dereference.
    In order to prevent this serialize access to input_dev using mutex lock.

    Signed-off-by: Amadeusz Sławiński
    Reviewed-by: Cezary Rojewski
    Link: https://lore.kernel.org/r/20220412091628.3056922-1-amadeuszx.slawinski@linux.intel.com
    Signed-off-by: Takashi Iwai
    Signed-off-by: Sasha Levin

    Amadeusz Sławiński
     

20 Apr, 2022

2 commits

  • commit 2f7a26abb8241a0208c68d22815aa247c5ddacab upstream.

    Syzbot reports "KASAN: null-ptr-deref Write in
    snd_pcm_format_set_silence".[1]

    It is due to missing validation of the "silence" field of struct
    "pcm_format_data" in "pcm_formats" array.

    Add a test for valid "pat" and, if it is not so, return -EINVAL.

    [1] https://lore.kernel.org/lkml/000000000000d188ef05dc2c7279@google.com/

    Reported-and-tested-by: syzbot+205eb15961852c2c5974@syzkaller.appspotmail.com
    Signed-off-by: Fabio M. De Francesco
    Cc:
    Link: https://lore.kernel.org/r/20220409012655.9399-1-fmdefrancesco@gmail.com
    Signed-off-by: Takashi Iwai
    Signed-off-by: Greg Kroah-Hartman

    Fabio M. De Francesco
     
  • commit fee2b871d8d6389c9b4bdf9346a99ccc1c98c9b8 upstream.

    This is a small helper function to handle the error path more easily
    when an error happens during the probe for the device with the
    device-managed card. Since devres releases in the reverser order of
    the creations, usually snd_card_free() gets called at the last in the
    probe error path unless it already reached snd_card_register() calls.
    Due to this nature, when a driver expects the resource releases in
    card->private_free, this might be called too lately.

    As a workaround, one should call the probe like:

    static int __some_probe(...) { // do real probe.... }

    static int some_probe(...)
    {
    return snd_card_free_on_error(dev, __some_probe(dev, ...));
    }

    so that the snd_card_free() is called explicitly at the beginning of
    the error path from the probe.

    This function will be used in the upcoming fixes to address the
    regressions by devres usages.

    Fixes: e8ad415b7a55 ("ALSA: core: Add managed card creation")
    Cc:
    Link: https://lore.kernel.org/r/20220412093141.8008-2-tiwai@suse.de
    Signed-off-by: Takashi Iwai
    Signed-off-by: Greg Kroah-Hartman

    Takashi Iwai
     

08 Apr, 2022

1 commit

  • commit bc55cfd5718c7c23e5524582e9fa70b4d10f2433 upstream.

    syzbot caught a potential deadlock between the PCM
    runtime->buffer_mutex and the mm->mmap_lock. It was brought by the
    recent fix to cover the racy read/write and other ioctls, and in that
    commit, I overlooked a (hopefully only) corner case that may take the
    revert lock, namely, the OSS mmap. The OSS mmap operation
    exceptionally allows to re-configure the parameters inside the OSS
    mmap syscall, where mm->mmap_mutex is already held. Meanwhile, the
    copy_from/to_user calls at read/write operations also take the
    mm->mmap_lock internally, hence it may lead to a AB/BA deadlock.

    A similar problem was already seen in the past and we fixed it with a
    refcount (in commit b248371628aa). The former fix covered only the
    call paths with OSS read/write and OSS ioctls, while we need to cover
    the concurrent access via both ALSA and OSS APIs now.

    This patch addresses the problem above by replacing the buffer_mutex
    lock in the read/write operations with a refcount similar as we've
    used for OSS. The new field, runtime->buffer_accessing, keeps the
    number of concurrent read/write operations. Unlike the former
    buffer_mutex protection, this protects only around the
    copy_from/to_user() calls; the other codes are basically protected by
    the PCM stream lock. The refcount can be a negative, meaning blocked
    by the ioctls. If a negative value is seen, the read/write aborts
    with -EBUSY. In the ioctl side, OTOH, they check this refcount, too,
    and set to a negative value for blocking unless it's already being
    accessed.

    Reported-by: syzbot+6e5c88838328e99c7e1c@syzkaller.appspotmail.com
    Fixes: dca947d4d26d ("ALSA: pcm: Fix races among concurrent read/write and buffer changes")
    Cc:
    Link: https://lore.kernel.org/r/000000000000381a0d05db622a81@google.com
    Link: https://lore.kernel.org/r/20220330120903.4738-1-tiwai@suse.de
    Signed-off-by: Takashi Iwai
    Signed-off-by: Greg Kroah-Hartman

    Takashi Iwai
     

28 Mar, 2022

6 commits

  • commit 1f68915b2efd0d6bfd6e124aa63c94b3c69f127c upstream.

    snd_pcm_reset() is a non-atomic operation, and it's allowed to run
    during the PCM stream running. It implies that the manipulation of
    hw_ptr and other parameters might be racy.

    This patch adds the PCM stream lock at appropriate places in
    snd_pcm_*_reset() actions for covering that.

    Cc:
    Reviewed-by: Jaroslav Kysela
    Link: https://lore.kernel.org/r/20220322171325.4355-1-tiwai@suse.de
    Signed-off-by: Takashi Iwai
    Signed-off-by: Greg Kroah-Hartman

    Takashi Iwai
     
  • commit 69534c48ba8ce552ce383b3dfdb271ffe51820c3 upstream.

    We have no protection against concurrent PCM buffer preallocation
    changes via proc files, and it may potentially lead to UAF or some
    weird problem. This patch applies the PCM open_mutex to the proc
    write operation for avoiding the racy proc writes and the PCM stream
    open (and further operations).

    Cc:
    Reviewed-by: Jaroslav Kysela
    Link: https://lore.kernel.org/r/20220322170720.3529-5-tiwai@suse.de
    Signed-off-by: Takashi Iwai
    Signed-off-by: Greg Kroah-Hartman

    Takashi Iwai
     
  • commit 3c3201f8c7bb77eb53b08a3ca8d9a4ddc500b4c0 upstream.

    Like the previous fixes to hw_params and hw_free ioctl races, we need
    to paper over the concurrent prepare ioctl calls against hw_params and
    hw_free, too.

    This patch implements the locking with the existing
    runtime->buffer_mutex for prepare ioctls. Unlike the previous case
    for snd_pcm_hw_hw_params() and snd_pcm_hw_free(), snd_pcm_prepare() is
    performed to the linked streams, hence the lock can't be applied
    simply on the top. For tracking the lock in each linked substream, we
    modify snd_pcm_action_group() slightly and apply the buffer_mutex for
    the case stream_lock=false (formerly there was no lock applied)
    there.

    Cc:
    Reviewed-by: Jaroslav Kysela
    Link: https://lore.kernel.org/r/20220322170720.3529-4-tiwai@suse.de
    Signed-off-by: Takashi Iwai
    Signed-off-by: Greg Kroah-Hartman

    Takashi Iwai
     
  • commit dca947d4d26dbf925a64a6cfb2ddbc035e831a3d upstream.

    In the current PCM design, the read/write syscalls (as well as the
    equivalent ioctls) are allowed before the PCM stream is running, that
    is, at PCM PREPARED state. Meanwhile, we also allow to re-issue
    hw_params and hw_free ioctl calls at the PREPARED state that may
    change or free the buffers, too. The problem is that there is no
    protection against those mix-ups.

    This patch applies the previously introduced runtime->buffer_mutex to
    the read/write operations so that the concurrent hw_params or hw_free
    call can no longer interfere during the operation. The mutex is
    unlocked before scheduling, so we don't take it too long.

    Cc:
    Reviewed-by: Jaroslav Kysela
    Link: https://lore.kernel.org/r/20220322170720.3529-3-tiwai@suse.de
    Signed-off-by: Takashi Iwai
    Signed-off-by: Greg Kroah-Hartman

    Takashi Iwai
     
  • commit 92ee3c60ec9fe64404dc035e7c41277d74aa26cb upstream.

    Currently we have neither proper check nor protection against the
    concurrent calls of PCM hw_params and hw_free ioctls, which may result
    in a UAF. Since the existing PCM stream lock can't be used for
    protecting the whole ioctl operations, we need a new mutex to protect
    those racy calls.

    This patch introduced a new mutex, runtime->buffer_mutex, and applies
    it to both hw_params and hw_free ioctl code paths. Along with it, the
    both functions are slightly modified (the mmap_count check is moved
    into the state-check block) for code simplicity.

    Reported-by: Hu Jiahui
    Cc:
    Reviewed-by: Jaroslav Kysela
    Link: https://lore.kernel.org/r/20220322170720.3529-2-tiwai@suse.de
    Signed-off-by: Takashi Iwai
    Signed-off-by: Greg Kroah-Hartman

    Takashi Iwai
     
  • commit efb6402c3c4a7c26d97c92d70186424097b6e366 upstream.

    We've got syzbot reports hitting INT_MAX overflow at vmalloc()
    allocation that is called from snd_pcm_plug_alloc(). Although we
    apply the restrictions to input parameters, it's based only on the
    hw_params of the underlying PCM device. Since the PCM OSS layer
    allocates a temporary buffer for the data conversion, the size may
    become unexpectedly large when more channels or higher rates is given;
    in the reported case, it went over INT_MAX, hence it hits WARN_ON().

    This patch is an attempt to avoid such an overflow and an allocation
    for too large buffers. First off, it adds the limit of 1MB as the
    upper bound for period bytes. This must be large enough for all use
    cases, and we really don't want to handle a larger temporary buffer
    than this size. The size check is performed at two places, where the
    original period bytes is calculated and where the plugin buffer size
    is calculated.

    In addition, the driver uses array_size() and array3_size() for
    multiplications to catch overflows for the converted period size and
    buffer bytes.

    Reported-by: syzbot+72732c532ac1454eeee9@syzkaller.appspotmail.com
    Suggested-by: Linus Torvalds
    Cc:
    Link: https://lore.kernel.org/r/00000000000085b1b305da5a66f3@google.com
    Link: https://lore.kernel.org/r/20220318082036.29699-1-tiwai@suse.de
    Signed-off-by: Takashi Iwai
    Signed-off-by: Greg Kroah-Hartman

    Takashi Iwai
     

27 Jan, 2022

5 commits

  • [ Upstream commit 6fadb494a638d8b8a55864ecc6ac58194f03f327 ]

    Currently ALSA sequencer core tries to process the queued events as
    much as possible when they become dispatchable. If applications try
    to queue too massive events to be processed at the very same timing,
    the sequencer core would still try to process such all events, either
    in the interrupt context or via some notifier; in either away, it
    might be a cause of RCU stall or such problems.

    As a potential workaround for those problems, this patch adds the
    upper limit of the amount of events to be processed. The remaining
    events are processed in the next batch, so they won't be lost.

    For the time being, it's limited up to 1000 events per queue, which
    should be high enough for any normal usages.

    Reported-by: Zqiang
    Reported-by: syzbot+bb950e68b400ab4f65f8@syzkaller.appspotmail.com
    Link: https://lore.kernel.org/r/20211102033222.3849-1-qiang.zhang1211@gmail.com
    Link: https://lore.kernel.org/r/20211207165146.2888-1-tiwai@suse.de
    Signed-off-by: Takashi Iwai
    Signed-off-by: Sasha Levin

    Takashi Iwai
     
  • [ Upstream commit 8e7daf318d97f25e18b2fc7eb5909e34cd903575 ]

    Fix compile error when OSS_DEBUG is enabled:
    sound/core/oss/pcm_oss.c: In function 'snd_pcm_oss_set_trigger':
    sound/core/oss/pcm_oss.c:2055:10: error: 'substream' undeclared (first
    use in this function); did you mean 'csubstream'?
    pcm_dbg(substream->pcm, "pcm_oss: trigger = 0x%x\n", trigger);
    ^

    Fixes: 61efcee8608c ("ALSA: oss: Use standard printk helpers")
    Signed-off-by: Bixuan Cui
    Link: https://lore.kernel.org/r/1638349134-110369-1-git-send-email-cuibixuan@linux.alibaba.com
    Signed-off-by: Takashi Iwai
    Signed-off-by: Sasha Levin

    Bixuan Cui
     
  • [ Upstream commit 5471e9762e1af4b7df057a96bfd46cc250979b88 ]

    snd_ctl_remove() has to be called with card->controls_rwsem held (when
    called after the card instantiation). This patch add the missing
    rwsem calls around it.

    Fixes: a8ff48cb7083 ("ALSA: pcm: Free chmap at PCM free callback, too")
    Link: https://lore.kernel.org/r/20211116071314.15065-2-tiwai@suse.de
    Signed-off-by: Takashi Iwai
    Signed-off-by: Sasha Levin

    Takashi Iwai
     
  • [ Upstream commit 06764dc931848c3a9bc01a63bbf76a605408bb54 ]

    snd_ctl_remove() has to be called with card->controls_rwsem held (when
    called after the card instantiation). This patch add the missing
    rwsem calls around it.

    Fixes: 9058cbe1eed2 ("ALSA: jack: implement kctl creating for jack devices")
    Link: https://lore.kernel.org/r/20211116071314.15065-1-tiwai@suse.de
    Signed-off-by: Takashi Iwai
    Signed-off-by: Sasha Levin

    Takashi Iwai
     
  • commit 5576c4f24c56722a2d9fb9c447d896e5b312078b upstream.

    Some weird devices set the codec SSID vendor ID 0, and
    snd_pci_quirk_lookup_id() loop aborts at the point although it should
    still try matching with the SSID device ID. This resulted in a
    missing quirk for some old Macs.

    Fix the loop termination condition to check both subvendor and
    subdevice.

    Fixes: 73355ddd8775 ("ALSA: hda: Code refactoring snd_hda_pick_fixup()")
    Cc:
    BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=215495
    Link: https://lore.kernel.org/r/20220116082838.19382-1-tiwai@suse.de
    Signed-off-by: Takashi Iwai
    Signed-off-by: Greg Kroah-Hartman

    Takashi Iwai
     

29 Dec, 2021

2 commits

  • commit 39a8fc4971a00d22536aeb7d446ee4a97810611b upstream.

    The user_pversion was uninitialized for the user space file structure
    in the open function, because the file private structure use
    kmalloc for the allocation.

    The kernel ALSA sequencer code clears the file structure, so no additional
    fixes are required.

    Cc: stable@kernel.org
    Cc: broonie@kernel.org
    BugLink: https://github.com/alsa-project/alsa-lib/issues/178
    Fixes: 09d23174402d ("ALSA: rawmidi: introduce SNDRV_RAWMIDI_IOCTL_USER_PVERSION")
    Reported-by: syzbot+88412ee8811832b00dbe@syzkaller.appspotmail.com
    Signed-off-by: Jaroslav Kysela
    Link: https://lore.kernel.org/r/20211218123925.2583847-1-perex@perex.cz
    Signed-off-by: Takashi Iwai
    Signed-off-by: Greg Kroah-Hartman

    Jaroslav Kysela
     
  • commit c01c1db1dc632edafb0dff32d40daf4f9c1a4e19 upstream.

    kstrdup() can return NULL, it is better to check the return value of it.

    Signed-off-by: Xiaoke Wang
    Cc:
    Link: https://lore.kernel.org/r/tencent_094816F3522E0DC704056C789352EBBF0606@qq.com
    Signed-off-by: Takashi Iwai
    Signed-off-by: Greg Kroah-Hartman

    Xiaoke Wang
     

14 Dec, 2021

4 commits

  • commit 6665bb30a6b1a4a853d52557c05482ee50e71391 upstream.

    A couple of calls in snd_pcm_oss_change_params_locked() ignore the
    possible errors. Catch those errors and abort the operation for
    avoiding further problems.

    Cc:
    Link: https://lore.kernel.org/r/20211201073606.11660-4-tiwai@suse.de
    Signed-off-by: Takashi Iwai
    Signed-off-by: Greg Kroah-Hartman

    Takashi Iwai
     
  • commit 8839c8c0f77ab8fc0463f4ab8b37fca3f70677c2 upstream.

    Set the practical limit to the period size (the fragment shift in OSS)
    instead of a full 31bit; a too large value could lead to the exhaust
    of memory as we allocate temporary buffers of the period size, too.

    As of this patch, we set to 16MB limit, which should cover all use
    cases.

    Reported-by: syzbot+bb348e9f9a954d42746f@syzkaller.appspotmail.com
    Reported-by: Bixuan Cui
    Cc:
    Link: https://lore.kernel.org/r/1638270978-42412-1-git-send-email-cuibixuan@linux.alibaba.com
    Link: https://lore.kernel.org/r/20211201073606.11660-3-tiwai@suse.de
    Signed-off-by: Takashi Iwai
    Signed-off-by: Greg Kroah-Hartman

    Takashi Iwai
     
  • commit 9d2479c960875ca1239bcb899f386970c13d9cfe upstream.

    The period size calculation in OSS layer may receive a negative value
    as an error, but the code there assumes only the positive values and
    handle them with size_t. Due to that, a too big value may be passed
    to the lower layers.

    This patch changes the code to handle with ssize_t and adds the proper
    error checks appropriately.

    Reported-by: syzbot+bb348e9f9a954d42746f@syzkaller.appspotmail.com
    Reported-by: Bixuan Cui
    Cc:
    Link: https://lore.kernel.org/r/1638270978-42412-1-git-send-email-cuibixuan@linux.alibaba.com
    Link: https://lore.kernel.org/r/20211201073606.11660-2-tiwai@suse.de
    Signed-off-by: Takashi Iwai
    Signed-off-by: Greg Kroah-Hartman

    Takashi Iwai
     
  • commit b6409dd6bdc03aa178bbff0d80db2a30d29b63ac upstream.

    When control_compat.c:copy_ctl_value_to_user() is used, by
    ctl_elem_read_user() & ctl_elem_write_user(), it must also copy back the
    snd_ctl_elem_id value that may have been updated (filled in) by the call
    to snd_ctl_elem_read/snd_ctl_elem_write().

    This matches the functionality provided by snd_ctl_elem_read_user() and
    snd_ctl_elem_write_user(), via snd_ctl_build_ioff().

    Without this, and without making additional calls to snd_ctl_info()
    which are unnecessary when using the non-compat calls, a userspace
    application will not know the numid value for the element and
    consequently will not be able to use the poll/read interface on the
    control file to determine which elements have updates.

    Signed-off-by: Alan Young
    Cc:
    Link: https://lore.kernel.org/r/20211202150607.543389-1-consult.awy@gmail.com
    Signed-off-by: Takashi Iwai
    Signed-off-by: Greg Kroah-Hartman

    Alan Young
     

25 Nov, 2021

1 commit

  • [ Upstream commit 3c05f1477e62ea5a0a8797ba6a545b1dc751fb31 ]

    On m68k, compiling drivers under SND_ISA causes build errors:

    ../sound/core/isadma.c: In function 'snd_dma_program':
    ../sound/core/isadma.c:33:17: error: implicit declaration of function 'claim_dma_lock' [-Werror=implicit-function-declaration]
    33 | flags = claim_dma_lock();
    | ^~~~~~~~~~~~~~
    ../sound/core/isadma.c:41:9: error: implicit declaration of function 'release_dma_lock' [-Werror=implicit-function-declaration]
    41 | release_dma_lock(flags);
    | ^~~~~~~~~~~~~~~~

    ../sound/isa/sb/sb16_main.c: In function 'snd_sb16_playback_prepare':
    ../sound/isa/sb/sb16_main.c:253:72: error: 'DMA_AUTOINIT' undeclared (first use in this function)
    253 | snd_dma_program(dma, runtime->dma_addr, size, DMA_MODE_WRITE | DMA_AUTOINIT);
    | ^~~~~~~~~~~~
    ../sound/isa/sb/sb16_main.c:253:72: note: each undeclared identifier is reported only once for each function it appears in
    ../sound/isa/sb/sb16_main.c: In function 'snd_sb16_capture_prepare':
    ../sound/isa/sb/sb16_main.c:322:71: error: 'DMA_AUTOINIT' undeclared (first use in this function)
    322 | snd_dma_program(dma, runtime->dma_addr, size, DMA_MODE_READ | DMA_AUTOINIT);
    | ^~~~~~~~~~~~

    and more...

    Signed-off-by: Randy Dunlap
    Cc: Jaroslav Kysela
    Cc: Takashi Iwai
    Cc: alsa-devel@alsa-project.org
    Cc: linux-m68k@lists.linux-m68k.org
    Cc: Geert Uytterhoeven
    Link: https://lore.kernel.org/r/20211016062602.3588-1-rdunlap@infradead.org
    Signed-off-by: Takashi Iwai
    Signed-off-by: Sasha Levin

    Randy Dunlap
     

19 Nov, 2021

6 commits

  • [ Upstream commit dce9446192439eaac81c21f517325fb473735e53 ]

    Although we've covered all calls with NULL dma buffer pointer, so far,
    there may be still some else in the wild. For catching such a case
    more easily, add a WARN_ON_ONCE() in snd_dma_get_ops().

    Fixes: 37af81c5998f ("ALSA: core: Abstract memory alloc helpers")
    Link: https://lore.kernel.org/r/20211105102103.28148-1-tiwai@suse.de
    Signed-off-by: Takashi Iwai
    Signed-off-by: Sasha Levin

    Takashi Iwai
     
  • commit ffdd98277f0a1d15a67a74ae09bee713df4c0dbc upstream.

    Like the previous fix (commit c0317c0e8709 "ALSA: timer: Fix
    use-after-free problem"), we have to unlink slave timer instances
    immediately at snd_timer_stop(), too. Otherwise it may leave a stale
    entry in the list if the slave instance is freed before actually
    running.

    Cc:
    Link: https://lore.kernel.org/r/20211105091517.21733-1-tiwai@suse.de
    Signed-off-by: Takashi Iwai
    Signed-off-by: Greg Kroah-Hartman

    Takashi Iwai
     
  • commit c0317c0e87094f5b5782b6fdef5ae0a4b150496c upstream.

    When the timer instance was add into ack_list but was not currently in
    process, the user could stop it via snd_timer_stop1() without delete it
    from the ack_list. Then the user could free the timer instance and when
    it was actually processed UAF occurred.

    This issue could be reproduced via testcase snd_timer01 in ltp - running
    several instances of that testcase at the same time.

    What I actually met was that the ack_list of the timer broken and the
    kernel went into deadloop with irqoff. That could be detected by
    hardlockup detector on board or when we run it on qemu, we could use gdb
    to dump the ack_list when the console has no response.

    To fix this issue, we delete the timer instance from ack_list and
    active_list unconditionally in snd_timer_stop1().

    Signed-off-by: Wang Wensheng
    Suggested-by: Takashi Iwai
    Cc:
    Link: https://lore.kernel.org/r/20211103033517.80531-1-wangwensheng4@huawei.com
    Signed-off-by: Takashi Iwai
    Signed-off-by: Greg Kroah-Hartman

    Wang Wensheng
     
  • commit 8e537d5dec34cac746dd6abf6a83e5de3aa471fc upstream.

    The recent refactoring of mmap handling caused Oops on some devices
    that don't use the standard memory allocations. This patch addresses
    it by allowing snd_dma_buffer_mmap() helper to receive the NULL
    pointer dmab argument (and return an error appropriately).

    Fixes: a202bd1ad86d ("ALSA: core: Move mmap handler into memalloc ops")
    Cc:
    Link: https://lore.kernel.org/r/20211107163911.13534-1-tiwai@suse.de
    Signed-off-by: Takashi Iwai
    Signed-off-by: Greg Kroah-Hartman

    Takashi Iwai
     
  • commit 3ab7992018455ac63c33e9b3eaa7264e293e40f4 upstream.

    In commit 411cef6adfb3 ("ALSA: mixer: oss: Fix racy access to slots")
    added mutex protection in snd_mixer_oss_set_volume(). Second
    mutex_lock() in same function looks like typo, fix it.

    Reported-by: syzbot+ace149a75a9a0a399ac7@syzkaller.appspotmail.com
    Fixes: 411cef6adfb3 ("ALSA: mixer: oss: Fix racy access to slots")
    Cc:
    Signed-off-by: Pavel Skripkin
    Link: https://lore.kernel.org/r/20211024140315.16704-1-paskripkin@gmail.com
    Signed-off-by: Takashi Iwai
    Signed-off-by: Greg Kroah-Hartman

    Pavel Skripkin
     
  • commit 411cef6adfb38a5bb6bd9af3941b28198e7fb680 upstream.

    The OSS mixer can reassign the mapping slots dynamically via proc
    file. Although the addition and deletion of those slots are protected
    by mixer->reg_mutex, the access to slots aren't, hence this may cause
    UAF when the slots in use are deleted concurrently.

    This patch applies the mixer->reg_mutex in all appropriate code paths
    (i.e. the ioctl functions) that may access slots.

    Reported-by: syzbot+9988f17cf72a1045a189@syzkaller.appspotmail.com
    Reviewed-by: Jaroslav Kysela
    Cc:
    Link: https://lore.kernel.org/r/00000000000036adc005ceca9175@google.com
    Link: https://lore.kernel.org/r/20211020164846.922-1-tiwai@suse.de
    Signed-off-by: Takashi Iwai
    Signed-off-by: Greg Kroah-Hartman

    Takashi Iwai
     

12 Oct, 2021

1 commit

  • Michael Forney reported an incorrect padding type that was defined in
    the commit 80fe7430c708 ("ALSA: add new 32-bit layout for
    snd_pcm_mmap_status/control") for PCM control mmap data.
    His analysis is correct, and this caused the misplacements of PCM
    control data on 32bit arch and 32bit compat mode.

    The bug is that the __pad2 definition in __snd_pcm_mmap_control64
    struct was wrongly with __pad_before_uframe, which should have been
    __pad_after_uframe instead. This struct is used in SYNC_PTR ioctl and
    control mmap. Basically this bug leads to two problems:

    - The offset of avail_min field becomes wrong, it's placed right after
    appl_ptr without padding on little-endian

    - When appl_ptr and avail_min are read as 64bit values in kernel side,
    the values become either zero or corrupted (mixed up)

    One good news is that, because both user-space and kernel
    misunderstand the wrong offset, at least, 32bit application running on
    32bit kernel works as is. Also, 64bit applications are unaffected
    because the padding size is zero. The remaining problem is the 32bit
    compat mode; as mentioned in the above, avail_min is placed right
    after appl_ptr on little-endian archs, 64bit kernel reads bogus values
    for appl_ptr updates, which may lead to streaming bugs like jumping,
    XRUN or whatever unexpected.
    (However, we haven't heard any serious bug reports due to this over
    years, so practically seen, it's fairly safe to assume that the impact
    by this bug is limited.)

    Ideally speaking, we should correct the wrong mmap status control
    definition. But this would cause again incompatibility with the
    existing binaries, and fixing it (e.g. by renumbering ioctls) would be
    really messy.

    So, as of this patch, we only correct the behavior of 32bit compat
    mode and keep the rest as is. Namely, the SYNC_PTR ioctl is now
    handled differently in compat mode to read/write the 32bit values at
    the right offsets. The control mmap of 32bit apps on 64bit kernels
    has been already disabled (which is likely rather an overlook, but
    this worked fine at this time :), so covering SYNC_PTR ioctl should
    suffice as a fallback.

    Fixes: 80fe7430c708 ("ALSA: add new 32-bit layout for snd_pcm_mmap_status/control")
    Reported-by: Michael Forney
    Reviewed-by: Arnd Bergmann
    Cc:
    Cc: Rich Felker
    Link: https://lore.kernel.org/r/29QBMJU8DE71E.2YZSH8IHT5HMH@mforney.org
    Link: https://lore.kernel.org/r/20211010075546.23220-1-tiwai@suse.de
    Signed-off-by: Takashi Iwai

    Takashi Iwai
     

30 Sep, 2021

1 commit

  • John Keeping reported and posted a patch for a potential UAF in
    rawmidi sequencer destruction: the snd_rawmidi_dev_seq_free() may be
    called after the associated rawmidi object got already freed.
    After a deeper look, it turned out that the bug is rather the
    incorrect private_free call order for a snd_seq_device. The
    snd_seq_device private_free gets called at the release callback of the
    sequencer device object, while this was rather expected to be executed
    at the snd_device call chains that runs at the beginning of the whole
    card-free procedure. It's been broken since the rewrite of
    sequencer-device binding (although it hasn't surfaced because the
    sequencer device release happens usually right along with the card
    device release).

    This patch corrects the private_free call to be done in the right
    place, at snd_seq_device_dev_free().

    Fixes: 7c37ae5c625a ("ALSA: seq: Rewrite sequencer device binding with standard bus")
    Reported-and-tested-by: John Keeping
    Cc:
    Link: https://lore.kernel.org/r/20210930114114.8645-1-tiwai@suse.de
    Signed-off-by: Takashi Iwai

    Takashi Iwai
     

23 Sep, 2021

1 commit

  • The new framing mode causes the user space regression, because
    the alsa-lib code does not initialize the reserved space in
    the params structure when the device is opened.

    This change adds SNDRV_RAWMIDI_IOCTL_USER_PVERSION like we
    do for the PCM interface for the protocol acknowledgment.

    Cc: David Henningsson
    Cc:
    Fixes: 08fdced60ca0 ("ALSA: rawmidi: Add framing mode")
    BugLink: https://github.com/alsa-project/alsa-lib/issues/178
    Signed-off-by: Jaroslav Kysela
    Link: https://lore.kernel.org/r/20210920171850.154186-1-perex@perex.cz
    Signed-off-by: Takashi Iwai

    Jaroslav Kysela