1. 27 Nov, 2017 1 commit
  2. 22 Nov, 2017 1 commit
    • Takashi Iwai's avatar
      ALSA: hda - Fix yet remaining issue with vmaster 0dB initialization · d6c0615f
      Takashi Iwai authored
      The previous fix for addressing the breakage in vmaster slave
      initialization, commit a91d6612 ("ALSA: hda - Fix incorrect TLV
      callback check introduced during set_fs() removal"), introduced a new
      helper to process over each slave kctl.  However, this helper passes
      only the original kctl, not the virtual slave kctl.  As a result,
      HD-audio driver (which is the only user so far) couldn't initialize
      the slave correctly because it's trying to update the value directly
      with the original kctl, not with the mapped kctl.
      
      This patch fixes the situation again by passing both the mapped slaved
      and original slave kctls to the function.  Luckily there is a single
      caller as of now, so changing the call signature is no big matter.
      
      Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=197959
      Fixes: a91d6612 ("ALSA: hda - Fix incorrect TLV callback check introduced during set_fs() removal")
      Cc: <stable@vger.kernel.org>
      Signed-off-by: default avatarTakashi Iwai <tiwai@suse.de>
      d6c0615f
  3. 21 Nov, 2017 2 commits
    • Takashi Iwai's avatar
      ALSA: timer: Remove kernel warning at compat ioctl error paths · 3d4e8303
      Takashi Iwai authored
      Some timer compat ioctls have NULL checks of timer instance with
      snd_BUG_ON() that bring up WARN_ON() when the debug option is set.
      Actually the condition can be met in the normal situation and it's
      confusing and bad to spew kernel warnings with stack trace there.
      Let's remove snd_BUG_ON() invocation and replace with the simple
      checks.  Also, correct the error code to EBADFD to follow the native
      ioctl error handling.
      Reported-by: default avatarsyzbot <syzkaller@googlegroups.com>
      Cc: <stable@vger.kernel.org>
      Signed-off-by: default avatarTakashi Iwai <tiwai@suse.de>
      3d4e8303
    • Henrik Eriksson's avatar
      ALSA: pcm: update tstamp only if audio_tstamp changed · 20e3f985
      Henrik Eriksson authored
      commit 3179f620 ("ALSA: core: add .get_time_info") had a side effect
      of changing the behaviour of the PCM runtime tstamp.  Prior to this
      change tstamp was not updated by snd_pcm_update_hw_ptr0() unless the
      hw_ptr had moved, after this change tstamp was always updated.
      
      For an application using alsa-lib, doing snd_pcm_readi() followed by
      snd_pcm_status() to estimate the age of the read samples by subtracting
      status->avail * [sample rate] from status->tstamp this change degraded
      the accuracy of the estimate on devices where the pcm hw does not
      provide a granular hw_ptr, e.g., devices using
      soc-generic-dmaengine-pcm.c and a dma-engine with residue_granularity
      DMA_RESIDUE_GRANULARITY_DESCRIPTOR.  The accuracy of the estimate
      depended on the latency between the PCM hw completing a period and the
      driver called snd_pcm_period_elapsed() to notify ALSA core, typically
      determined by interrupt handling latency.  After the change the accuracy
      of the estimate depended on the latency between the PCM hw completing a
      period and the application calling snd_pcm_status(), determined by the
      scheduling of the application process.  The maximum error of the
      estimate is one period length in both cases, but the error average and
      variance is smaller when it depends on interrupt latency.
      
      Instead of always updating tstamp, update it only if audio_tstamp
      changed.
      
      Fixes: 3179f620 ("ALSA: core: add .get_time_info")
      Suggested-by: default avatarPierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
      Signed-off-by: default avatarHenrik Eriksson <henrik.eriksson@axis.com>
      Cc: <stable@vger.kernel.org>
      Signed-off-by: default avatarTakashi Iwai <tiwai@suse.de>
      20e3f985
  4. 07 Nov, 2017 1 commit
    • Takashi Iwai's avatar
      ALSA: seq: Fix OSS sysex delivery in OSS emulation · 132d358b
      Takashi Iwai authored
      The SYSEX event delivery in OSS sequencer emulation assumed that the
      event is encoded in the variable-length data with the straight
      buffering.  This was the normal behavior in the past, but during the
      development, the chained buffers were introduced for carrying more
      data, while the OSS code was left intact.  As a result, when a SYSEX
      event with the chained buffer data is passed to OSS sequencer port,
      it may end up with the wrong memory access, as if it were having a too
      large buffer.
      
      This patch addresses the bug, by applying the buffer data expansion by
      the generic snd_seq_dump_var_event() helper function.
      Reported-by: default avatarsyzbot <syzkaller@googlegroups.com>
      Reported-by: default avatarMark Salyzyn <salyzyn@android.com>
      Cc: <stable@vger.kernel.org>
      Signed-off-by: default avatarTakashi Iwai <tiwai@suse.de>
      132d358b
  5. 06 Nov, 2017 1 commit
    • Takashi Iwai's avatar
      ALSA: timer: Limit max instances per timer · 9b7d869e
      Takashi Iwai authored
      Currently we allow unlimited number of timer instances, and it may
      bring the system hogging way too much CPU when too many timer
      instances are opened and processed concurrently.  This may end up with
      a soft-lockup report as triggered by syzkaller, especially when
      hrtimer backend is deployed.
      
      Since such insane number of instances aren't demanded by the normal
      use case of ALSA sequencer and it merely  opens a risk only for abuse,
      this patch introduces the upper limit for the number of instances per
      timer backend.  As default, it's set to 1000, but for the fine-grained
      timer like hrtimer, it's set to 100.
      
      Reported-by: syzbot
      Tested-by: default avatarJérôme Glisse <jglisse@redhat.com>
      Cc: <stable@vger.kernel.org>
      Signed-off-by: default avatarTakashi Iwai <tiwai@suse.de>
      9b7d869e
  6. 02 Nov, 2017 1 commit
    • Greg Kroah-Hartman's avatar
      License cleanup: add SPDX GPL-2.0 license identifier to files with no license · b2441318
      Greg Kroah-Hartman authored
      Many source files in the tree are missing licensing information, which
      makes it harder for compliance tools to determine the correct license.
      
      By default all files without license information are under the default
      license of the kernel, which is GPL version 2.
      
      Update the files which contain no license information with the 'GPL-2.0'
      SPDX license identifier.  The SPDX identifier is a legally binding
      shorthand, which can be used instead of the full boiler plate text.
      
      This patch is based on work done by Thomas Gleixner and Kate Stewart and
      Philippe Ombredanne.
      
      How this work was done:
      
      Patches were generated and checked against linux-4.14-rc6 for a subset of
      the use cases:
       - file had no licensing information it it.
       - file was a */uapi/* one with no licensing information in it,
       - file was a */uapi/* one with existing licensing information,
      
      Further patches will be generated in subsequent months to fix up cases
      where non-standard license headers were used, and references to license
      had to be inferred by heuristics based on keywords.
      
      The analysis to determine which SPDX License Identifier to be applied to
      a file was done in a spreadsheet of side by side results from of the
      output of two independent scanners (ScanCode & Windriver) producing SPDX
      tag:value files created by Philippe Ombredanne.  Philippe prepared the
      base worksheet, and did an initial spot review of a few 1000 files.
      
      The 4.13 kernel was the starting point of the analysis with 60,537 files
      assessed.  Kate Stewart did a file by file comparison of the scanner
      results in the spreadsheet to determine which SPDX license identifier(s)
      to be applied to the file. She confirmed any determination that was not
      immediately clear with lawyers working with the Linux Foundation.
      
      Criteria used to select files for SPDX license identifier tagging was:
       - Files considered eligible had to be source code files.
       - Make and config files were included as candidates if they contained >5
         lines of source
       - File already had some variant of a license header in it (even if <5
         lines).
      
      All documentation files were explicitly excluded.
      
      The following heuristics were used to determine which SPDX license
      identifiers to apply.
      
       - when both scanners couldn't find any license traces, file was
         considered to have no license information in it, and the top level
         COPYING file license applied.
      
         For non */uapi/* files that summary was:
      
         SPDX license identifier                            # files
         ---------------------------------------------------|-------
         GPL-2.0                                              11139
      
         and resulted in the first patch in this series.
      
         If that file was a */uapi/* path one, it was "GPL-2.0 WITH
         Linux-syscall-note" otherwise it was "GPL-2.0".  Results of that was:
      
         SPDX license identifier                            # files
         ---------------------------------------------------|-------
         GPL-2.0 WITH Linux-syscall-note                        930
      
         and resulted in the second patch in this series.
      
       - if a file had some form of licensing information in it, and was one
         of the */uapi/* ones, it was denoted with the Linux-syscall-note if
         any GPL family license was found in the file or had no licensing in
         it (per prior point).  Results summary:
      
         SPDX license identifier                            # files
         ---------------------------------------------------|------
         GPL-2.0 WITH Linux-syscall-note                       270
         GPL-2.0+ WITH Linux-syscall-note                      169
         ((GPL-2.0 WITH Linux-syscall-note) OR BSD-2-Clause)    21
         ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause)    17
         LGPL-2.1+ WITH Linux-syscall-note                      15
         GPL-1.0+ WITH Linux-syscall-note                       14
         ((GPL-2.0+ WITH Linux-syscall-note) OR BSD-3-Clause)    5
         LGPL-2.0+ WITH Linux-syscall-note                       4
         LGPL-2.1 WITH Linux-syscall-note                        3
         ((GPL-2.0 WITH Linux-syscall-note) OR MIT)              3
         ((GPL-2.0 WITH Linux-syscall-note) AND MIT)             1
      
         and that resulted in the third patch in this series.
      
       - when the two scanners agreed on the detected license(s), that became
         the concluded license(s).
      
       - when there was disagreement between the two scanners (one detected a
         license but the other didn't, or they both detected different
         licenses) a manual inspection of the file occurred.
      
       - In most cases a manual inspection of the information in the file
         resulted in a clear resolution of the license that should apply (and
         which scanner probably needed to revisit its heuristics).
      
       - When it was not immediately clear, the license identifier was
         confirmed with lawyers working with the Linux Foundation.
      
       - If there was any question as to the appropriate license identifier,
         the file was flagged for further research and to be revisited later
         in time.
      
      In total, over 70 hours of logged manual review was done on the
      spreadsheet to determine the SPDX license identifiers to apply to the
      source files by Kate, Philippe, Thomas and, in some cases, confirmation
      by lawyers working with the Linux Foundation.
      
      Kate also obtained a third independent scan of the 4.13 code base from
      FOSSology, and compared selected files where the other two scanners
      disagreed against that SPDX file, to see if there was new insights.  The
      Windriver scanner is based on an older version of FOSSology in part, so
      they are related.
      
      Thomas did random spot checks in about 500 files from the spreadsheets
      for the uapi headers and agreed with SPDX license identifier in the
      files he inspected. For the non-uapi files Thomas did random spot checks
      in about 15000 files.
      
      In initial set of patches against 4.14-rc6, 3 files were found to have
      copy/paste license identifier errors, and have been fixed to reflect the
      correct identifier.
      
      Additionally Philippe spent 10 hours this week doing a detailed manual
      inspection and review of the 12,461 patched files from the initial patch
      version early this week with:
       - a full scancode scan run, collecting the matched texts, detected
         license ids and scores
       - reviewing anything where there was a license detected (about 500+
         files) to ensure that the applied SPDX license was correct
       - reviewing anything where there was no detection but the patch license
         was not GPL-2.0 WITH Linux-syscall-note to ensure that the applied
         SPDX license was correct
      
      This produced a worksheet with 20 files needing minor correction.  This
      worksheet was then exported into 3 different .csv files for the
      different types of files to be modified.
      
      These .csv files were then reviewed by Greg.  Thomas wrote a script to
      parse the csv files and add the proper SPDX tag to the file, in the
      format that the file expected.  This script was further refined by Greg
      based on the output to detect more types of files automatically and to
      distinguish between header and source .c files (which need different
      comment types.)  Finally Greg ran the script using the .csv files to
      generate the patches.
      Reviewed-by: default avatarKate Stewart <kstewart@linuxfoundation.org>
      Reviewed-by: default avatarPhilippe Ombredanne <pombredanne@nexb.com>
      Reviewed-by: default avatarThomas Gleixner <tglx@linutronix.de>
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
      b2441318
  7. 31 Oct, 2017 2 commits
  8. 30 Oct, 2017 1 commit
  9. 18 Oct, 2017 4 commits
    • Takashi Iwai's avatar
      ALSA: hda - Fix incorrect TLV callback check introduced during set_fs() removal · a91d6612
      Takashi Iwai authored
      The commit 99b5c5bb ("ALSA: hda - Remove the use of set_fs()")
      converted the get_kctl_0dB_offset() call for killing set_fs() usage in
      HD-audio codec code.  The conversion assumed that the TLV callback
      used in HD-audio code is only snd_hda_mixer_amp() and applies the TLV
      calculation locally.
      
      Although this assumption is correct, and all slave kctls are actually
      with that callback, the current code is still utterly buggy; it
      doesn't hit this condition and falls back to the next check.  It's
      because the function gets called after adding slave kctls to vmaster.
      By assigning a slave kctl, the slave kctl object is faked inside
      vmaster code, and the whole kctl ops are overridden.  Thus the
      callback op points to a different value from what we've assumed.
      
      More badly, as reported by the KERNEXEC and UDEREF features of PaX,
      the code flow turns into the unexpected pitfall.  The next fallback
      check is SNDRV_CTL_ELEM_ACCESS_TLV_READ access bit, and this always
      hits for each kctl with TLV.  Then it evaluates the callback function
      pointer wrongly as if it were a TLV array.  Although currently its
      side-effect is fairly limited, this incorrect reference may lead to an
      unpleasant result.
      
      For addressing the regression, this patch introduces a new helper to
      vmaster code, snd_ctl_apply_vmaster_slaves().  This works similarly
      like the existing map_slaves() in hda_codec.c: it loops over the slave
      list of the given master, and applies the given function to each
      slave.  Then the initializer function receives the right kctl object
      and we can compare the correct pointer instead of the faked one.
      
      Also, for catching the similar breakage in future, give an error
      message when the unexpected TLV callback is found and bail out
      immediately.
      
      Fixes: 99b5c5bb ("ALSA: hda - Remove the use of set_fs()")
      Reported-by: default avatarPaX Team <pageexec@freemail.hu>
      Cc: <stable@vger.kernel.org> # v4.13
      Signed-off-by: default avatarTakashi Iwai <tiwai@suse.de>
      a91d6612
    • Takashi Iwai's avatar
      ALSA: pcm: Forcibly stop at disconnect callback · 6ca73de7
      Takashi Iwai authored
      So far we assumed that each driver implements the hotplug PCM handling
      properly, e.g. dealing with the pending PCM stream at disconnect
      callback.  But most codes don't care, and it eventually leaves the PCM
      stream inconsistent state when an abrupt disconnection like sysfs
      unbind happens.
      
      This patch is simple but a big-hammer solution: invoke snd_pcm_stop()
      at the common PCM disconnect callback always when the stream is
      running.
      Tested-by: default avatarKuninori Morimoto <kuninori.morimoto.gx@renesas.com>
      Signed-off-by: default avatarTakashi Iwai <tiwai@suse.de>
      6ca73de7
    • Takashi Iwai's avatar
      ALSA: pcm: Don't call register and disconnect callbacks for internal PCM · 8b645e4a
      Takashi Iwai authored
      The internal PCM (aka DPCM backend PCM) doesn't need any registration
      procedure, thus currently we bail out immediately at dev_register
      callback.  Similarly, its counterpart, dev_disconnect callback, is
      superfluous for the internal PCM.  For simplifying and avoiding the
      conflicting disconnect call for internal PCM objects, this patch drops
      dev_register and dev_disconnect callbacks for the internal ops.
      
      The only uncertain thing by this action is whether skipping the PCM
      state change to SNDRV_PCM_STATE_DISCONNECT for the internal PCM is
      mandatory.  Looking through the current implementations, this doesn't
      look so, hence dropping the whole dev_disconnect would make more
      sense.
      Tested-by: default avatarKuninori Morimoto <kuninori.morimoto.gx@renesas.com>
      Signed-off-by: default avatarTakashi Iwai <tiwai@suse.de>
      8b645e4a
    • Ben Hutchings's avatar
      ALSA: seq: Enable 'use' locking in all configurations · 8009d506
      Ben Hutchings authored
      The 'use' locking macros are no-ops if neither SMP or SND_DEBUG is
      enabled.  This might once have been OK in non-preemptible
      configurations, but even in that case snd_seq_read() may sleep while
      relying on a 'use' lock.  So always use the proper implementations.
      
      Cc: stable@vger.kernel.org
      Signed-off-by: default avatarBen Hutchings <ben.hutchings@codethink.co.uk>
      Signed-off-by: default avatarTakashi Iwai <tiwai@suse.de>
      8009d506
  10. 16 Oct, 2017 1 commit
  11. 11 Oct, 2017 2 commits
    • Takashi Iwai's avatar
      ALSA: add snd_card_disconnect_sync() · c44027c8
      Takashi Iwai authored
      In case of user unbind ALSA driver during playing back / capturing,
      each driver needs to stop and remove it correctly. One note here is
      that we can't cancel from remove function in such case, because
      unbind operation doesn't check return value from remove function.
      So, we *must* stop and remove in this case.
      
      For this purpose, we need to sync (= wait) until the all top-level
      operations are canceled at remove function.
      For example, snd_card_free() processes the disconnection procedure at
      first, then waits for the completion. That's how the hot-unplug works
      safely. It's implemented, at least, in the top-level driver removal.
      
      Now for the lower level driver, we need a similar strategy. Notify to
      the toplevel for hot-unplug (disconnect in ALSA), and sync with the
      stop operation, then continue the rest of its own remove procedure.
      
      This patch adds snd_card_disconnect_sync(), and driver can use it from
      remove function.
      
      Note: the "lower level" driver here refers to a middle layer driver
      (e.g. ASoC components) that can be unbound freely during operation.
      Most of legacy ALSA helper drivers don't have such a problem because
      they can't be unbound.
      
      Note#2: snd_card_disconnect_sync() merely calls snd_card_disconnect()
      and syncs with closing all pending files.  It takes only the files
      opened by user-space into account, and doesn't care about object
      refcounts.  (The latter is handled by snd_card_free() completion call,
      BTW.)  Also, the function doesn't free resources by itself.
      Tested-by: default avatarKuninori Morimoto <kuninori.morimoto.gx@renesas.com>
      Signed-off-by: default avatarTakashi Iwai <tiwai@suse.de>
      c44027c8
    • Takashi Iwai's avatar
      ALSA: seq: Fix use-after-free at creating a port · 71105998
      Takashi Iwai authored
      There is a potential race window opened at creating and deleting a
      port via ioctl, as spotted by fuzzing.  snd_seq_create_port() creates
      a port object and returns its pointer, but it doesn't take the
      refcount, thus it can be deleted immediately by another thread.
      Meanwhile, snd_seq_ioctl_create_port() still calls the function
      snd_seq_system_client_ev_port_start() with the created port object
      that is being deleted, and this triggers use-after-free like:
      
       BUG: KASAN: use-after-free in snd_seq_ioctl_create_port+0x504/0x630 [snd_seq] at addr ffff8801f2241cb1
       =============================================================================
       BUG kmalloc-512 (Tainted: G    B          ): kasan: bad access detected
       -----------------------------------------------------------------------------
       INFO: Allocated in snd_seq_create_port+0x94/0x9b0 [snd_seq] age=1 cpu=3 pid=4511
       	___slab_alloc+0x425/0x460
       	__slab_alloc+0x20/0x40
        	kmem_cache_alloc_trace+0x150/0x190
      	snd_seq_create_port+0x94/0x9b0 [snd_seq]
      	snd_seq_ioctl_create_port+0xd1/0x630 [snd_seq]
       	snd_seq_do_ioctl+0x11c/0x190 [snd_seq]
       	snd_seq_ioctl+0x40/0x80 [snd_seq]
       	do_vfs_ioctl+0x54b/0xda0
       	SyS_ioctl+0x79/0x90
       	entry_SYSCALL_64_fastpath+0x16/0x75
       INFO: Freed in port_delete+0x136/0x1a0 [snd_seq] age=1 cpu=2 pid=4717
       	__slab_free+0x204/0x310
       	kfree+0x15f/0x180
       	port_delete+0x136/0x1a0 [snd_seq]
       	snd_seq_delete_port+0x235/0x350 [snd_seq]
       	snd_seq_ioctl_delete_port+0xc8/0x180 [snd_seq]
       	snd_seq_do_ioctl+0x11c/0x190 [snd_seq]
       	snd_seq_ioctl+0x40/0x80 [snd_seq]
       	do_vfs_ioctl+0x54b/0xda0
       	SyS_ioctl+0x79/0x90
       	entry_SYSCALL_64_fastpath+0x16/0x75
       Call Trace:
        [<ffffffff81b03781>] dump_stack+0x63/0x82
        [<ffffffff81531b3b>] print_trailer+0xfb/0x160
        [<ffffffff81536db4>] object_err+0x34/0x40
        [<ffffffff815392d3>] kasan_report.part.2+0x223/0x520
        [<ffffffffa07aadf4>] ? snd_seq_ioctl_create_port+0x504/0x630 [snd_seq]
        [<ffffffff815395fe>] __asan_report_load1_noabort+0x2e/0x30
        [<ffffffffa07aadf4>] snd_seq_ioctl_create_port+0x504/0x630 [snd_seq]
        [<ffffffffa07aa8f0>] ? snd_seq_ioctl_delete_port+0x180/0x180 [snd_seq]
        [<ffffffff8136be50>] ? taskstats_exit+0xbc0/0xbc0
        [<ffffffffa07abc5c>] snd_seq_do_ioctl+0x11c/0x190 [snd_seq]
        [<ffffffffa07abd10>] snd_seq_ioctl+0x40/0x80 [snd_seq]
        [<ffffffff8136d433>] ? acct_account_cputime+0x63/0x80
        [<ffffffff815b515b>] do_vfs_ioctl+0x54b/0xda0
        .....
      
      We may fix this in a few different ways, and in this patch, it's fixed
      simply by taking the refcount properly at snd_seq_create_port() and
      letting the caller unref the object after use.  Also, there is another
      potential use-after-free by sprintf() call in snd_seq_create_port(),
      and this is moved inside the lock.
      
      This fix covers CVE-2017-15265.
      Reported-and-tested-by: default avatarMichael23 Yu <ycqzsy@gmail.com>
      Suggested-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
      Cc: <stable@vger.kernel.org>
      Signed-off-by: default avatarTakashi Iwai <tiwai@suse.de>
      71105998
  12. 10 Oct, 2017 1 commit
    • Takashi Iwai's avatar
      ALSA: seq: Add sanity check for user-space pointer delivery · 19b592da
      Takashi Iwai authored
      The sequencer event may contain a user-space pointer with its
      SNDRV_SEQ_EXT_USRPTR bit, and we assure that its delivery is limited
      with non-atomic mode.  Otherwise the copy_from_user() may hit the
      fault and cause a problem.  Although the core code doesn't set such a
      flag (only set at snd_seq_write()), any wild driver may set it
      mistakenly and lead to an unexpected crash.
      
      This patch adds a sanity check of such events at the delivery core
      code to filter out the invalid invocation in the atomic mode.
      Signed-off-by: default avatarTakashi Iwai <tiwai@suse.de>
      19b592da
  13. 09 Oct, 2017 1 commit
    • Takashi Iwai's avatar
      ALSA: seq: Fix copy_from_user() call inside lock · 5803b023
      Takashi Iwai authored
      The event handler in the virmidi sequencer code takes a read-lock for
      the linked list traverse, while it's calling snd_seq_dump_var_event()
      in the loop.  The latter function may expand the user-space data
      depending on the event type.  It eventually invokes copy_from_user(),
      which might be a potential dead-lock.
      
      The sequencer core guarantees that the user-space data is passed only
      with atomic=0 argument, but snd_virmidi_dev_receive_event() ignores it
      and always takes read-lock().  For avoiding the problem above, this
      patch introduces rwsem for non-atomic case, while keeping rwlock for
      atomic case.
      
      Also while we're at it: the superfluous irq flags is dropped in
      snd_virmidi_input_open().
      Reported-by: default avatarJia-Ju Bai <baijiaju1990@163.com>
      Cc: <stable@vger.kernel.org>
      Signed-off-by: default avatarTakashi Iwai <tiwai@suse.de>
      5803b023
  14. 05 Oct, 2017 1 commit
  15. 22 Sep, 2017 1 commit
  16. 19 Sep, 2017 1 commit
  17. 18 Sep, 2017 2 commits
  18. 12 Sep, 2017 1 commit
    • Takashi Iwai's avatar
      ALSA: seq: Cancel pending autoload work at unbinding device · fc27fe7e
      Takashi Iwai authored
      ALSA sequencer core has a mechanism to load the enumerated devices
      automatically, and it's performed in an off-load work.  This seems
      causing some race when a sequencer is removed while the pending
      autoload work is running.  As syzkaller spotted, it may lead to some
      use-after-free:
        BUG: KASAN: use-after-free in snd_rawmidi_dev_seq_free+0x69/0x70
        sound/core/rawmidi.c:1617
        Write of size 8 at addr ffff88006c611d90 by task kworker/2:1/567
      
        CPU: 2 PID: 567 Comm: kworker/2:1 Not tainted 4.13.0+ #29
        Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
        Workqueue: events autoload_drivers
        Call Trace:
         __dump_stack lib/dump_stack.c:16 [inline]
         dump_stack+0x192/0x22c lib/dump_stack.c:52
         print_address_description+0x78/0x280 mm/kasan/report.c:252
         kasan_report_error mm/kasan/report.c:351 [inline]
         kasan_report+0x230/0x340 mm/kasan/report.c:409
         __asan_report_store8_noabort+0x1c/0x20 mm/kasan/report.c:435
         snd_rawmidi_dev_seq_free+0x69/0x70 sound/core/rawmidi.c:1617
         snd_seq_dev_release+0x4f/0x70 sound/core/seq_device.c:192
         device_release+0x13f/0x210 drivers/base/core.c:814
         kobject_cleanup lib/kobject.c:648 [inline]
         kobject_release lib/kobject.c:677 [inline]
         kref_put include/linux/kref.h:70 [inline]
         kobject_put+0x145/0x240 lib/kobject.c:694
         put_device+0x25/0x30 drivers/base/core.c:1799
         klist_devices_put+0x36/0x40 drivers/base/bus.c:827
         klist_next+0x264/0x4a0 lib/klist.c:403
         next_device drivers/base/bus.c:270 [inline]
         bus_for_each_dev+0x17e/0x210 drivers/base/bus.c:312
         autoload_drivers+0x3b/0x50 sound/core/seq_device.c:117
         process_one_work+0x9fb/0x1570 kernel/workqueue.c:2097
         worker_thread+0x1e4/0x1350 kernel/workqueue.c:2231
         kthread+0x324/0x3f0 kernel/kthread.c:231
         ret_from_fork+0x25/0x30 arch/x86/entry/entry_64.S:425
      
      The fix is simply to assure canceling the autoload work at removing
      the device.
      Reported-by: default avatarAndrey Konovalov <andreyknvl@google.com>
      Tested-by: default avatarAndrey Konovalov <andreyknvl@google.com>
      Cc: <stable@vger.kernel.org>
      Signed-off-by: default avatarTakashi Iwai <tiwai@suse.de>
      fc27fe7e
  19. 07 Sep, 2017 1 commit
  20. 30 Aug, 2017 3 commits
    • Takashi Iwai's avatar
      ALSA: pcm: Unify ioctl functions for playback and capture streams · 67616fed
      Takashi Iwai authored
      Some ioctl functions are implemented individually for both playback
      and capture streams although most of the codes are identical with just
      a few different stream-specific function calls.  This patch unifies
      these places, removes the superfluous trivial check and flattens the
      call paths as a cleanup.  Meanwhile, for better readability, some
      codes (e.g. xfer ioctls or forward/rewind ioctls) are factored out as
      functions.
      Signed-off-by: default avatarTakashi Iwai <tiwai@suse.de>
      67616fed
    • Takashi Iwai's avatar
      ALSA: Get rid of card power_lock · 7d8e8292
      Takashi Iwai authored
      Currently we're taking power_lock at each card component for assuring
      the power-up sequence, but it doesn't help anything in the
      implementation at the moment: it just serializes unnecessarily the
      callers, but it doesn't protect about the power state change itself.
      It used to have some usefulness in the early days where we managed the
      PM manually.  But now the suspend/resume core procedure is beyond our
      hands, and power_lock lost its meaning.
      
      This patch drops the power_lock from allover the places.
      There shouldn't be any issues by this change, as it's no helper
      regarding the power state change.  Rather we'll get better performance
      by removing the serialization; which is the only slight concern of any
      behavior change, but it can't be a showstopper, after all.
      Signed-off-by: default avatarTakashi Iwai <tiwai@suse.de>
      7d8e8292
    • Takashi Iwai's avatar
      ALSA: pcm: Fix power lock unbalance via OSS emulation · bcab3a6e
      Takashi Iwai authored
      PCM OSS emulation issues the drain ioctl without power lock.  It used
      to work in the earlier kernels as the power lock was taken inside
      snd_pcm_drain() itself.  But since 68b4acd3 ("ALSA: pcm: Apply
      power lock globally to common ioctls"), the power lock is taken
      outside the function.  Due to that change, the call via OSS emulation
      leads to the unbalanced power lock, thus it deadlocks.
      
      As a quick fix, just take the power lock before snd_pcm_drain() call
      for OSS emulation path.  A better cleanup will follow later.
      
      Fixes: 68b4acd3 ("ALSA: pcm: Apply power lock globally to common ioctls")
      Reported-and-tested-by: default avatarMarkus Trippelsdorf <markus@trippelsdorf.de>
      Signed-off-by: default avatarTakashi Iwai <tiwai@suse.de>
      bcab3a6e
  21. 24 Aug, 2017 4 commits
    • Takashi Iwai's avatar
      ALSA: pcm: Correct broken procfs set up · e6b4c525
      Takashi Iwai authored
      The commit c8da9be4 ("ALSA: pcm: Adjust nine function calls
      together with a variable assignment") contained a badly incorrect
      conversion, a "status" PCM procfs creation was replaced with the next
      one.  Luckily, this could be spotted easily by the kernel runtime
      warning.
      
      Fixes: c8da9be4 ("ALSA: pcm: Adjust nine function calls together...")
      Reported-by: default avatarFabio Estevam <fabio.estevam@nxp.com>
      Tested-by: default avatarFabio Estevam <fabio.estevam@nxp.com>
      Signed-off-by: default avatarTakashi Iwai <tiwai@suse.de>
      e6b4c525
    • Takashi Sakamoto's avatar
      ALSA: control: TLV data is unavailable at initial state of user-defined element set · b8e2204b
      Takashi Sakamoto authored
      For user-defined element set, in its initial state, TLV data is not
      registered. It's firstly available when any application register it by
      an additional operation. However, in current implementation, it's available
      in its initial state. As a result, applications get -ENXIO to read it.
      
      This commit controls its readability to manage info flags properly. In an
      initial state, elements don't have SND_CTL_ELEM_ACCESS_TLV_READ flag. Once
      TLV write operation is executed, they get the flag.
      Signed-off-by: default avatarTakashi Sakamoto <o-takashi@sakamocchi.jp>
      Signed-off-by: default avatarTakashi Iwai <tiwai@suse.de>
      b8e2204b
    • Takashi Sakamoto's avatar
      ALSA: control: queue TLV event for a set of user-defined element · da428828
      Takashi Sakamoto authored
      In a design of user-defined element set, applications allow to change TLV
      data on the set. This operation doesn't only affects to a target element,
      but also to elements in the set.
      
      This commit generates TLV event for all of elements in the set when the TLV
      data is changed.
      Signed-off-by: default avatarTakashi Sakamoto <o-takashi@sakamocchi.jp>
      Signed-off-by: default avatarTakashi Iwai <tiwai@suse.de>
      da428828
    • Takashi Sakamoto's avatar
      ALSA: control: delegate TLV eventing to each driver · fb8027eb
      Takashi Sakamoto authored
      In a design of ALSA control core, a set of elements is represented by
      'struct snd_kcontrol' to share common attributes. The set of elements
      shares TLV (Type-Length-Value) data, too.
      
      On the other hand, in ALSA control interface/protocol for applications,
      a TLV operation is committed to an element. Totally, the operation can
      have sub-effect to the other elements in the set. For example, TLV_WRITE
      operation is expected to change TLV data, which returns to applications.
      Applications attempt to change the TLV data per element, but in the above
      design, they can effect to elements in the same set.
      
      As a default, ALSA control core has no implementation except for TLV_READ
      operation. Thus, the above design looks to have no issue. However, in
      kernel APIs of ALSA control component, developers can program a handler
      for any request of the TLV operation. Therefore, for elements in a set
      which has the handler, applications can commit TLV_WRITE and TLV_COMMAND
      requests.
      
      For the above scenario, ALSA control core assist notification. When the
      handler returns positive value, the core queueing an event for a requested
      element. However, this includes design defects that the event is not
      queued for the other element in a set. Actually, developers can program
      the handlers to keep per-element TLV data, but it depends on each driver.
      
      As of v4.13-rc6, there's no driver in tree to utilize the notification,
      except for user-defined element set. This commit delegates the notification
      into each driver to prevent developers from the design defects.
      Signed-off-by: default avatarTakashi Sakamoto <o-takashi@sakamocchi.jp>
      Signed-off-by: default avatarTakashi Iwai <tiwai@suse.de>
      fb8027eb
  22. 23 Aug, 2017 4 commits
  23. 22 Aug, 2017 1 commit
    • Takashi Iwai's avatar
      ALSA: core: Fix unexpected error at replacing user TLV · 88c54cdf
      Takashi Iwai authored
      When user tries to replace the user-defined control TLV, the kernel
      checks the change of its content via memcmp().  The problem is that
      the kernel passes the return value from memcmp() as is.  memcmp()
      gives a non-zero negative value depending on the comparison result,
      and this shall be recognized as an error code.
      
      The patch covers that corner-case, return 1 properly for the changed
      TLV.
      
      Fixes: 8aa9b586 ("[ALSA] Control API - more robust TLV implementation")
      Cc: <stable@vger.kernel.org>
      Signed-off-by: default avatarTakashi Iwai <tiwai@suse.de>
      88c54cdf
  24. 20 Aug, 2017 2 commits
    • Takashi Sakamoto's avatar
      ALSA: control: use counting semaphore as write lock for ELEM_WRITE operation · 5bbb1ab5
      Takashi Sakamoto authored
      In ALSA control interface, applications can execute two types of request
      for value of members on each element; ELEM_READ and ELEM_WRITE. In ALSA
      control core, these two requests are handled within read lock of a
      counting semaphore, therefore several processes can run to execute these
      two requests at the same time. This has an issue because ELEM_WRITE
      requests have an effect to change state of the target element. Concurrent
      access should be controlled for each of ELEM_READ/ELEM_WRITE case.
      
      This commit uses the counting semaphore as write lock for ELEM_WRITE
      requests, while use it as read lock for ELEM_READ requests. The state of
      a target element is maintained exclusively between ELEM_WRITE/ELEM_READ
      operations.
      
      There's a concern. If the counting semaphore is acquired for read lock
      in implementations of 'struct snd_kcontrol.put()' in each driver, this
      commit shall cause dead lock. As of v4.13-rc5, 'snd-mixer-oss.ko',
      'snd-emu10k1.ko' and 'snd-soc-sst-atom-hifi2-platform.ko' includes codes
      for read locks, but these are not in a call graph from
      'struct snd_kcontrol.put(). Therefore, this commit is safe.
      
      In current implementation, the same solution is applied for the other
      operations to element; e.g. ELEM_LOCK and ELEM_UNLOCK. There's another
      discussion about an overhead to maintain concurrent access to an element
      during operating the other elements on the same card instance, because the
      lock primitive is originally implemented to maintain a list of elements on
      the card instance. There's a substantial difference between
      per-element-list lock and per-element lock.
      
      Here, let me investigate another idea to add per-element lock to maintain
      the concurrent accesses with inquiry/change requests to an element. It's
      not so frequent for applications to operate members on elements, while
      adding a new lock primitive to structure increases memory footprint for
      all of element sets somehow. Experimentally, inquiry operation is more
      frequent than change operation and usage of counting semaphore for the
      inquiry operation brings no blocking to the other inquiry operations. Thus
      the overhead is not so critical for usual applications. For the above
      reasons, in this commit, the per-element lock is not introduced.
      Signed-off-by: default avatarTakashi Sakamoto <o-takashi@sakamocchi.jp>
      Signed-off-by: default avatarTakashi Iwai <tiwai@suse.de>
      5bbb1ab5
    • Takashi Sakamoto's avatar
      ALSA: control: code refactoring for ELEM_READ/ELEM_WRITE operations · becf9e5d
      Takashi Sakamoto authored
      ALSA control core handles ELEM_READ/ELEM_WRITE requests within lock
      acquisition of a counting semaphore. The lock is acquired in helper
      functions in the end of call path before calling implementations of each
      driver.
      
      ioctl(2) with SNDRV_CTL_ELEM_READ
      ...
      ->snd_ctl_ioctl()
        ->snd_ctl_elem_read_user()
          ->snd_ctl_elem_read()
            ->down_read(controls_rwsem)
            ->snd_ctl_find_id()
            ->struct snd_kcontrol.get()
            ->up_read(controls_rwsem)
      
      ioctl(2) with SNDRV_CTL_ELEM_WRITE
      ...
      ->snd_ctl_ioctl()
        ->snd_ctl_elem_write_user()
          ->snd_ctl_elem_write()
            ->down_read(controls_rwsem)
            ->snd_ctl_find_id()
            ->struct snd_kcontrol.put()
            ->up_read(controls_rwsem)
      
      This commit moves the lock acquisition to middle of the call graph to
      simplify the helper functions. As a result:
      
      ioctl(2) with SNDRV_CTL_ELEM_READ
      ...
      ->snd_ctl_ioctl()
        ->snd_ctl_elem_read_user()
          ->down_read(controls_rwsem)
          ->snd_ctl_elem_read()
            ->snd_ctl_find_id()
            ->struct snd_kcontrol.get()
          ->up_read(controls_rwsem)
      
      ioctl(2) with SNDRV_CTL_ELEM_WRITE
      ...
      ->snd_ctl_ioctl()
        ->snd_ctl_elem_write_user()
          ->down_read(controls_rwsem)
          ->snd_ctl_elem_write()
            ->snd_ctl_find_id()
            ->struct snd_kcontrol.put()
          ->up_read(controls_rwsem)
      Signed-off-by: default avatarTakashi Sakamoto <o-takashi@sakamocchi.jp>
      Signed-off-by: default avatarTakashi Iwai <tiwai@suse.de>
      becf9e5d