• Takashi Iwai's avatar
    ALSA: pcm: Workaround for a wrong offset in SYNC_PTR compat ioctl · 228af5a4
    Takashi Iwai authored
    Michael Forney reported an incorrect padding type that was defined in
    the commit 80fe7430 ("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: 80fe7430 ("ALSA: add new 32-bit layout for snd_pcm_mmap_status/control")
    Reported-by: default avatarMichael Forney <mforney@mforney.org>
    Reviewed-by: default avatarArnd Bergmann <arnd@arndb.de>
    Cc: <stable@vger.kernel.org>
    Cc: Rich Felker <dalias@libc.org>
    Link: https://lore.kernel.org/r/29QBMJU8DE71E.2YZSH8IHT5HMH@mforney.org
    Link: https://lore.kernel.org/r/20211010075546.23220-1-tiwai@suse.deSigned-off-by: default avatarTakashi Iwai <tiwai@suse.de>
    228af5a4
pcm_compat.c 18.7 KB