Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ASB 2023-03 & CIP st-38 update #89

Merged
merged 6 commits into from
Apr 30, 2023

Conversation

Flamefire
Copy link
Contributor

I merged cip st-38 into my fork and noticed a bug introduced by a difference in 4.4 and the used (by CIP) 4.14 kernels.

I was able to fix that by taking the missing commits from 4.14 but it was quite some work, hence proposing that here so you don't have to dive into the same rabbit hole...

I already reported that bug and the fix(es) to CIP so they may be in the next tag.

The last 2 commits are from the latest ASB

Alexander Grund and others added 6 commits March 25, 2023 11:25
Change-Id: I33ac4b5bdbc07e7139b629475d1fc350a4ec6425
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)

Change-Id: I6b39209aaf08afcbeca7c759b77bc96c67db4c77
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
commit 5a23699 upstream.

The patch "ALSA: control: code refactoring for ELEM_READ/ELEM_WRITE
operations" introduced a potential for kernel memory corruption due
to an incorrect if statement allowing non-readable controls to fall
through and call the get function. For TLV controls a driver can omit
SNDRV_CTL_ELEM_ACCESS_READ to ensure that only the TLV get function
can be called. Instead the normal get() can be invoked unexpectedly
and as the driver expects that this will only be called for controls
<= 512 bytes, potentially try to copy >512 bytes into the 512 byte
return array, so corrupting kernel memory.

The problem is an attempt to refactor the snd_ctl_elem_read function
to invert the logic so that it conditionally aborted if the control
is unreadable instead of conditionally executing. But the if statement
wasn't inverted correctly.

The correct inversion of

    if (a && !b)

is
    if (!a || b)

Fixes: becf9e5 ("ALSA: control: code refactoring for ELEM_READ/ELEM_WRITE operations")
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
…eration

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: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
commit 6c11df58fd1ac0aefcb3b227f72769272b939e56 upstream.

Verify that the fbdev or drm driver correctly adjusted the virtual
screen sizes. On failure report the failing driver and reject the screen
size change.

Signed-off-by: Helge Deller <deller@gmx.de>
Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: stable@vger.kernel.org # v5.4+
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Bug: 240019719
Signed-off-by: Todd Kjos <tkjos@google.com>
Change-Id: I70ed1cf3d0f169db8383a9e221f9f4388a69c5d6
commit 65a01e601dbba8b7a51a2677811f70f783766682 upstream.

Prevent that users set a font size which is bigger than the physical screen.
It's unlikely this may happen (because screens are usually much larger than the
fonts and each font char is limited to 32x32 pixels), but it may happen on
smaller screens/LCD displays.

Signed-off-by: Helge Deller <deller@gmx.de>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: stable@vger.kernel.org # v4.14+
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Bug: 240019719
Signed-off-by: Todd Kjos <tkjos@google.com>
Change-Id: I4ada78c5f9bbecf4c9761c8a9b2837164bf18559
@derfelot
Copy link
Member

and thanks again ;)

@derfelot derfelot merged commit 0b73b23 into whatawurst:lineage-20 Apr 30, 2023
@derfelot
Copy link
Member

looks like those three alsa commits were indeed included for st39 btw

@Flamefire Flamefire deleted the asb-23-03-los20 branch April 30, 2023 15:02
derfelot pushed a commit to derfelot/android_kernel_sony_msm8998 that referenced this pull request Jul 30, 2023
This adds test cases mostly around ARG_PTR_TO_RAW_STACK to check the
verifier behaviour.

  [...]
  whatawurst#84 raw_stack: no skb_load_bytes OK
  whatawurst#85 raw_stack: skb_load_bytes, no init OK
  whatawurst#86 raw_stack: skb_load_bytes, init OK
  whatawurst#87 raw_stack: skb_load_bytes, spilled regs around bounds OK
  whatawurst#88 raw_stack: skb_load_bytes, spilled regs corruption OK
  whatawurst#89 raw_stack: skb_load_bytes, spilled regs corruption 2 OK
  whatawurst#90 raw_stack: skb_load_bytes, spilled regs + data OK
  whatawurst#91 raw_stack: skb_load_bytes, invalid access 1 OK
  whatawurst#92 raw_stack: skb_load_bytes, invalid access 2 OK
  whatawurst#93 raw_stack: skb_load_bytes, invalid access 3 OK
  whatawurst#94 raw_stack: skb_load_bytes, invalid access 4 OK
  whatawurst#95 raw_stack: skb_load_bytes, invalid access 5 OK
  whatawurst#96 raw_stack: skb_load_bytes, invalid access 6 OK
  whatawurst#97 raw_stack: skb_load_bytes, large access OK
  Summary: 98 PASSED, 0 FAILED

Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Chatur27 <jasonbright2709@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants