The user timer tu->qused counter may go to a negative value when
multiple concurrent reads are performed since both the check and the
decrement of tu->qused are done in two individual locked contexts.
This results in bogus read outs, and the endless loop in the
user-space side.

The fix is to move the decrement of the tu->qused counter into the
same spinlock context as the zero-check of the counter.

Cc: <stable@vger.kernel.org>
Signed-off-by: NTakashi Iwai <tiwai@suse.de>

When snd_pcm_add_chmap_ctls() is called to the PCM stream to which a
chmap has been already assigned, it returns as an error due to the
conflicting snd_ctl_add() result.  However, this also clears the
already assigned chmap_kctl field via pcm_chmap_ctl_private_free(),
and becomes inconsistent in the later operation.

This patch adds the check of the conflicting chmap kctl before
actually trying to allocate / assign.  The check failure is treated as
a kernel warning, as the double call of snd_pcm_add_chmap_ctls() is
basically a driver bug and having the stack trace would help
developers to figure out the bad code path.
Signed-off-by: NTakashi Iwai <tiwai@suse.de>

A switch statement looks a bit cleaner than an if statement
spread over 3 lines, as such update this to a switch.
Signed-off-by: NCharles Keepax <ckeepax@opensource.wolfsonmicro.com>
Acked-by: NVinod Koul <vinod.koul@intel.com>
Signed-off-by: NTakashi Iwai <tiwai@suse.de>

We can't return a negative error code from the poll callback the return
type is unsigned and is checked against the poll specific flags we need
to return POLLERR if we encounter an error.
Signed-off-by: NCharles Keepax <ckeepax@opensource.wolfsonmicro.com>
Acked-by: NVinod Koul <vinod.koul@intel.com>
Signed-off-by: NTakashi Iwai <tiwai@suse.de>

stream can't be NULL here as we have just taken the address of it, so no
need for the check.
Signed-off-by: NCharles Keepax <ckeepax@opensource.wolfsonmicro.com>
Acked-by: NVinod Koul <vinod.koul@intel.com>
Signed-off-by: NTakashi Iwai <tiwai@suse.de>

We have a function that returns the appropriate flags for the stream
direction, so we should use it.
Signed-off-by: NCharles Keepax <ckeepax@opensource.wolfsonmicro.com>
Acked-by: NVinod Koul <vinod.koul@intel.com>
Signed-off-by: NTakashi Iwai <tiwai@suse.de>

We can't return a negative error code from the poll callback the return
type is unsigned and is checked against the poll specific flags we need
to return POLLERR if we encounter an error.
Signed-off-by: NCharles Keepax <ckeepax@opensource.wolfsonmicro.com>
Reviewed-by: NTakashi Sakamoto <o-takashi@sakamocchi.jp>
Signed-off-by: NTakashi Iwai <tiwai@suse.de>

The stack object “r1” has a total size of 32 bytes. Its field
“event” and “val” both contain 4 bytes padding. These 8 bytes
padding bytes are sent to user without being initialized.
Signed-off-by: NKangjie Lu <kjlu@gatech.edu>
Signed-off-by: NTakashi Iwai <tiwai@suse.de>

The stack object “r1” has a total size of 32 bytes. Its field
“event” and “val” both contain 4 bytes padding. These 8 bytes
padding bytes are sent to user without being initialized.
Signed-off-by: NKangjie Lu <kjlu@gatech.edu>
Signed-off-by: NTakashi Iwai <tiwai@suse.de>

The stack object “tread” has a total size of 32 bytes. Its field
“event” and “val” both contain 4 bytes padding. These 8 bytes
padding bytes are sent to user without being initialized.
Signed-off-by: NKangjie Lu <kjlu@gatech.edu>
Signed-off-by: NTakashi Iwai <tiwai@suse.de>

dmaengine_pcm currently only supports setups where FIFO reads/writes
correspond to exactly one sample, eg 16-bit sample data is transferred
via 16-bit FIFO accesses, 32-bit data via 32-bit accesses.

This patch adds support for setups with fixed width FIFOs where
multiple samples are packed into a larger word.

For example setups with a 32-bit wide FIFO register that expect
16-bit sample transfers to be done with the left+right sample data
packed into a 32-bit word.

Support for packed transfers is controlled via the
SND_DMAENGINE_PCM_DAI_FLAG_PACK flag in snd_dmaengine_dai_dma_data.flags

If this flag is set dmaengine_pcm doesn't put any restriction on the
supported formats and sets the DMA transfer width to undefined.

This means control over the constraints is now transferred to the DAI
driver and it's responsible to provide proper configuration and
check for possible corner cases that aren't handled by the ALSA core.
Signed-off-by: NMatthias Reichl <hias@horus.com>
Acked-by: NLars-Peter Clausen <lars@metafoo.de>
Tested-by: NMartin Sperl <kernel@martin.sperl.org>
Signed-off-by: NMark Brown <broonie@kernel.org>

This patch tries to address the still remaining issues in ALSA hrtimer
driver:
- Spurious use-after-free was detected in hrtimer callback
- Incorrect rescheduling due to delayed start
- WARN_ON() is triggered in hrtimer_forward() invoked in hrtimer
  callback

The first issue happens only when the new timer is scheduled even
while hrtimer is being closed.  It's related with the second and third
items; since ALSA timer core invokes hw.start callback during hrtimer
interrupt, this may result in the explicit call of hrtimer_start().

Also, the similar problem is seen for the stop; ALSA timer core
invokes hw.stop callback even in the hrtimer handler, too.  Since we
must not call the synced hrtimer_cancel() in such a context, it's just
a hrtimer_try_to_cancel() call that doesn't properly work.

Another culprit of the second and third items is the call of
hrtimer_forward_now() before snd_timer_interrupt().  The timer->stick
value may change during snd_timer_interrupt() call, but this
possibility is ignored completely.

For covering these subtle and messy issues, the following changes have
been done in this patch:
- A new flag, in_callback, is introduced in the private data to
  indicate that the hrtimer handler is being processed.
- Both start and stop callbacks skip when called from (during)
  in_callback flag.
- The hrtimer handler returns properly HRTIMER_RESTART and NORESTART
  depending on the running state now.
- The hrtimer handler reprograms the expiry properly after
  snd_timer_interrupt() call, instead of before.
- The close callback clears running flag and sets in_callback flag
  to block any further start/stop calls.
Signed-off-by: NTakashi Iwai <tiwai@suse.de>

There are no users of rtctimer left. Remove its code as this is the
in-kernel user of the legacy PC RTC driver that will hopefully be removed
at some point.
Signed-off-by: NAlexandre Belloni <alexandre.belloni@free-electrons.com>
Signed-off-by: NTakashi Iwai <tiwai@suse.de>

Currently kill_fasync() is called outside the stream lock in
snd_pcm_period_elapsed().  This is potentially racy, since the stream
may get released even during the irq handler is running.  Although
snd_pcm_release_substream() calls snd_pcm_drop(), this doesn't
guarantee that the irq handler finishes, thus the kill_fasync() call
outside the stream spin lock may be invoked after the substream is
detached, as recently reported by KASAN.

As a quick workaround, move kill_fasync() call inside the stream
lock.  The fasync is rarely used interface, so this shouldn't have a
big impact from the performance POV.

Ideally, we should implement some sync mechanism for the proper finish
of stream and irq handler.  But this oneliner should suffice for most
cases, so far.
Reported-by: NBaozeng Ding <sploving1@gmail.com>
Signed-off-by: NTakashi Iwai <tiwai@suse.de>

Treat 32 bit sample width as if it was 24 bits when generating IEC958
channel status bits. On some platforms 24 sample width is problematic
and to get full 24 bit precision a 32 bit format, using only the 24
most significant bits, may have to be used.
Signed-off-by: NJyri Sarha <jsarha@ti.com>
Reviewed-by: NTakashi Iwai <tiwai@suse.de>
Signed-off-by: NMark Brown <broonie@kernel.org>

Add IEC958 channel status helper that gets the audio properties from
snd_pcm_hw_params instead of snd_pcm_runtime. This is needed to
produce the channel status bits already in audio stream configuration
phase.
Signed-off-by: NJyri Sarha <jsarha@ti.com>
Reviewed-by: NTakashi Iwai <tiwai@suse.de>
Signed-off-by: NMark Brown <broonie@kernel.org>

ALSA system timer backend stops the timer via del_timer() without sync
and leaves del_timer_sync() at the close instead.  This is because of
the restriction by the design of ALSA timer: namely, the stop callback
may be called from the timer handler, and calling the sync shall lead
to a hangup.  However, this also triggers a kernel BUG() when the
timer is rearmed immediately after stopping without sync:
 kernel BUG at kernel/time/timer.c:966!
 Call Trace:
  <IRQ>
  [<ffffffff8239c94e>] snd_timer_s_start+0x13e/0x1a0
  [<ffffffff8239e1f4>] snd_timer_interrupt+0x504/0xec0
  [<ffffffff8122fca0>] ? debug_check_no_locks_freed+0x290/0x290
  [<ffffffff8239ec64>] snd_timer_s_function+0xb4/0x120
  [<ffffffff81296b72>] call_timer_fn+0x162/0x520
  [<ffffffff81296add>] ? call_timer_fn+0xcd/0x520
  [<ffffffff8239ebb0>] ? snd_timer_interrupt+0xec0/0xec0
  ....

It's the place where add_timer() checks the pending timer.  It's clear
that this may happen after the immediate restart without sync in our
cases.

So, the workaround here is just to use mod_timer() instead of
add_timer().  This looks like a band-aid fix, but it's a right move,
as snd_timer_interrupt() takes care of the continuous rearm of timer.
Reported-by: NJiri Slaby <jslaby@suse.cz>
Cc: <stable@vger.kernel.org>
Signed-off-by: NTakashi Iwai <tiwai@suse.de>

'struct snd_timer_gparams' includes some members with 'unsigned long',
therefore its size differs depending on data models of architecture. As
a result, x86/x32 applications fail to execute ioctl(2) with
SNDRV_TIMER_GPARAMS command on x86_64 machine.

This commit fixes this bug by adding a pair of structure and ioctl
command for the compatibility.
Signed-off-by: NTakashi Sakamoto <o-takashi@sakamocchi.jp>
Signed-off-by: NTakashi Iwai <tiwai@suse.de>

ALSA: ctl: change return value in compatibility layer so that it's the same value in core implementation

In control compatibility layer, when no elements are found by
ELEM_READ/ELEM_WRITE ioctl commands, ENXIO is returned. On the other hand,
in core implementation, ENOENT is returned. This is not good for
ALSA ctl applications.

This commit changes the return value from the compatibility layer so
that the same value is returned.
Signed-off-by: NTakashi Sakamoto <o-takashi@sakamocchi.jp>
Signed-off-by: NTakashi Iwai <tiwai@suse.de>

The commit [d507941b: ALSA: pcm: Correct PCM BUG error message]
made the warning prefix back to "BUG:" due to its previous wrong
prefix.  But a kernel message containing "BUG:" seems taken as an Oops
message wrongly by some brain-dead daemons, and it annoys users in the
end.  Instead of teaching daemons, change the string again to a more
reasonable one.

Fixes: 507941beb1e ('ALSA: pcm: Correct PCM BUG error message')
Cc: <stable@vger.kernel.org> # v3.19+
Signed-off-by: NTakashi Iwai <tiwai@suse.de>

rawmidi devices expose the card number via IOCTLs, which allows to
find the corresponding device in sysfs.

The sequencer provides no identifing data. Chromium works around this
issue by scanning rawmidi as well as sequencer devices and matching
them by using assumtions, how the kernel register sequencer devices.

This changes adds support for exposing the card number for kernel clients
as well as the PID for user client.

The minor of the API version is changed to distinguish between the zero
initialised reserved field and card number 0.

[minor coding style fixes by tiwai]
Signed-off-by: NMartin Koegler <martin.koegler@chello.at>
Acked-by: NClemens Ladisch <clemens@ladisch.de>
Signed-off-by: NTakashi Iwai <tiwai@suse.de>

More inspection of code revealed few more typos so fix them as well
Signed-off-by: NVinod Koul <vinod.koul@intel.com>
Signed-off-by: NTakashi Iwai <tiwai@suse.de>

Found few typos while looking at code, so fix them
Signed-off-by: NVinod Koul <vinod.koul@intel.com>
Signed-off-by: NTakashi Iwai <tiwai@suse.de>

Stream states were explained in the code comments but
SNDRV_PCM_STATE_PREPARED was missed so add it
Signed-off-by: NVinod Koul <vinod.koul@intel.com>
Signed-off-by: NTakashi Iwai <tiwai@suse.de>

Allow writes in SNDRV_PCM_STATE_PREPARED state so that more
than one buffer fragment can be written from user space
before calling SNDRV_COMPRESS_START.
Signed-off-by: NEric Laurent <elaurent@google.com>
Acked-by: NVinod Koul <vinod.koul@intel.com>
Signed-off-by: NTakashi Iwai <tiwai@suse.de>

The OSS sequencer client tries to drain the pending events at
releasing.  Unfortunately, as spotted by syzkaller fuzzer, this may
lead to an unkillable process state when the event has been queued at
the far future.  Since the process being released can't be signaled
any longer, it remains and waits for the echo-back event in that far
future.

Back to history, the draining feature was implemented at the time we
misinterpreted POSIX definition for blocking file operation.
Actually, such a behavior is superfluous at release, and we should
just release the device as is instead of keeping it up forever.

This patch just removes the draining call that may block the release
for too long time unexpectedly.

BugLink: http://lkml.kernel.org/r/CACT4Y+Y4kD-aBGj37rf-xBw9bH3GMU6P+MYg4W1e-s-paVD2pg@mail.gmail.comReported-by: NDmitry Vyukov <dvyukov@google.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: NTakashi Iwai <tiwai@suse.de>

X32 ABI takes the 64bit timespec, thus the timer user status ioctl becomes
incompatible with IA32.  This results in NOTTY error when the ioctl is
issued.

Meanwhile, this struct in X32 is essentially identical with the one in
X86-64, so we can just bypassing to the existing code for this
specific compat ioctl.

Cc: <stable@vger.kernel.org> # v3.4+
Signed-off-by: NTakashi Iwai <tiwai@suse.de>

The timer user status compat ioctl returned the bogus struct used for
64bit architectures instead of the 32bit one.  This patch addresses
it to return the proper struct.

Cc: <stable@vger.kernel.org>
Signed-off-by: NTakashi Iwai <tiwai@suse.de>

Like the previous fixes for ctl and PCM, we need a fix for
incompatible X32 ABI regarding the rawmidi: namely, struct
snd_rawmidi_status has the timespec, and the size and the alignment on
X32 differ from IA32.

This patch fixes the incompatible ioctl for X32.

Cc: <stable@vger.kernel.org> # v3.4+
Signed-off-by: NTakashi Iwai <tiwai@suse.de>

Instead of open-coding, use the existing helper to copy a 32bit
timespec from/to 64bit.
Signed-off-by: NTakashi Iwai <tiwai@suse.de>

X32 ABI uses the 64bit timespec in addition to 64bit alignment of
64bit values.  This leads to incompatibilities in some PCM ioctls
involved with snd_pcm_channel_info, snd_pcm_status and
snd_pcm_sync_ptr structs.  Fix the PCM compat ABI for these ioctls
like the previous commit for ctl API.
Reported-by: NSteven Newbury <steve@snewbury.org.uk>
Cc: <stable@vger.kernel.org> # v3.4+
Signed-off-by: NTakashi Iwai <tiwai@suse.de>

The X32 ABI takes the same alignment like x86-64, and this may result
in the incompatible struct size from ia32.  Unfortunately, we hit this
in some control ABI: struct snd_ctl_elem_value differs between them
due to the position of 64bit variable array.  This ends up with the
unknown ioctl (ENOTTY) error.

The fix is to add the compat entries for the new aligned struct.
Reported-and-tested-by: NSteven Newbury <steve@snewbury.org.uk>
Cc: <stable@vger.kernel.org> # v3.4+
Signed-off-by: NTakashi Iwai <tiwai@suse.de>

Since the recent integration of kctl jack and input jack layers, we
can basically build the jack layer even without input devices.  That
is, the jack layer itself can be built with conditional to enable the
input device support or not, while the users may enable always
CONFIG_SND_JACK unconditionally.

For achieving it, this patch changes the following:
- A new Kconfig, CONFIG_SND_JACK_INPUT_DEV, was introduced to indicate
  whether the jack layer supports the input device,
- A few items in snd_jack struct and relevant codes are conditionally
  built upon CONFIG_SND_JACK_INPUT_DEV,
- The users of CONFIG_SND_JACK drop the messy dependency on
  CONFIG_INPUT.

This change also automagically fixes a potential bug in HD-audio
driver Arnd reported, where the NULL or uninitialized jack instance is
dereferenced.
Reported-by: NArnd Bergmann <arnd@arndb.de>
Acked-by: NArnd Bergmann <arnd@arndb.de>
Signed-off-by: NTakashi Iwai <tiwai@suse.de>

A non-atomic PCM stream may take snd_pcm_link_rwsem rw semaphore twice
in the same code path, e.g. one in snd_pcm_action_nonatomic() and
another in snd_pcm_stream_lock().  Usually this is OK, but when a
write lock is issued between these two read locks, the problem
happens: the write lock is blocked due to the first reade lock, and
the second read lock is also blocked by the write lock.  This
eventually deadlocks.

The reason is the way rwsem manages waiters; it's queued like FIFO, so
even if the writer itself doesn't take the lock yet, it blocks all the
waiters (including reads) queued after it.

As a workaround, in this patch, we replace the standard down_write()
with an spinning loop.  This is far from optimal, but it's good
enough, as the spinning time is supposed to be relatively short for
normal PCM operations, and the code paths requiring the write lock
aren't called so often.
Reported-by: NVinod Koul <vinod.koul@intel.com>
Tested-by: NRamesh Babu <ramesh.babu@intel.com>
Cc: <stable@vger.kernel.org> # v3.18+
Signed-off-by: NTakashi Iwai <tiwai@suse.de>

The commit [7f0973e9: ALSA: seq: Fix lockdep warnings due to
double mutex locks] split the management of two linked lists (source
and destination) into two individual calls for avoiding the AB/BA
deadlock.  However, this may leave the possible double deletion of one
of two lists when the counterpart is being deleted concurrently.
It ends up with a list corruption, as revealed by syzkaller fuzzer.

This patch fixes it by checking the list emptiness and skipping the
deletion and the following process.

BugLink: http://lkml.kernel.org/r/CACT4Y+bay9qsrz6dQu31EcGaH9XwfW7o3oBzSQUG9fMszoh=Sg@mail.gmail.com
Fixes: 7f0973e9 ('ALSA: seq: Fix lockdep warnings due to 'double mutex locks)
Reported-by: NDmitry Vyukov <dvyukov@google.com>
Tested-by: NDmitry Vyukov <dvyukov@google.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: NTakashi Iwai <tiwai@suse.de>

When multiple concurrent writes happen on the ALSA sequencer device
right after the open, it may try to allocate vmalloc buffer for each
write and leak some of them.  It's because the presence check and the
assignment of the buffer is done outside the spinlock for the pool.

The fix is to move the check and the assignment into the spinlock.

(The current implementation is suboptimal, as there can be multiple
 unnecessary vmallocs because the allocation is done before the check
 in the spinlock.  But the pool size is already checked beforehand, so
 this isn't a big problem; that is, the only possible path is the
 multiple writes before any pool assignment, and practically seen, the
 current coverage should be "good enough".)

The issue was triggered by syzkaller fuzzer.

BugLink: http://lkml.kernel.org/r/CACT4Y+bSzazpXNvtAr=WXaL8hptqjHwqEyFA+VN2AWEx=aurkg@mail.gmail.comReported-by: NDmitry Vyukov <dvyukov@google.com>
Tested-by: NDmitry Vyukov <dvyukov@google.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: NTakashi Iwai <tiwai@suse.de>

snd_timer_notify1() is called outside the spinlock and it retakes the
lock after the unlock.  This is rather racy, and it's safer to move
snd_timer_notify() call inside the main spinlock.

The patch also contains a slight refactoring / cleanup of the code.
Now all start/stop/continue/pause look more symmetric and a bit better
readable.
Signed-off-by: NTakashi Iwai <tiwai@suse.de>

In order to make the open/close more robust, widen the register_mutex
protection over the whole snd_timer_close() function.  Also, the close
procedure is slightly shuffled to be in the safer order, as well as a
few code refactoring.
Signed-off-by: NTakashi Iwai <tiwai@suse.de>

snd_timer_user_read() has a potential race among parallel reads, as
qhead and qused are updated outside the critical section due to
copy_to_user() calls.  Move them into the critical section, and also
sanitize the relevant code a bit.

Cc: <stable@vger.kernel.org>
Signed-off-by: NTakashi Iwai <tiwai@suse.de>

A slave timer element also unlinks at snd_timer_stop() but it takes
only slave_active_lock.  When a slave is assigned to a master,
however, this may become a race against the master's interrupt
handling, eventually resulting in a list corruption.  The actual bug
could be seen with a syzkaller fuzzer test case in BugLink below.

As a fix, we need to take timeri->timer->lock when timer isn't NULL,
i.e. assigned to a master, while the assignment to a master itself is
protected by slave_active_lock.

BugLink: http://lkml.kernel.org/r/CACT4Y+Y_Bm+7epAb=8Wi=AaWd+DYS7qawX52qxdCfOfY49vozQ@mail.gmail.com
Cc: <stable@vger.kernel.org>
Signed-off-by: NTakashi Iwai <tiwai@suse.de>