Hi,
Some supplements.
On Thu, Apr 08, 2021 at 07:31:49PM +0900, Takashi Sakamoto wrote:
> ALSA control interface allows users to add arbitrary control elements
> (called "user controls" or "user elements"), and its resource usage is
> limited just by the max number of control sets (currently 32). This
> limit, however, is quite loose: each allocation of control set may
> have 1028 elements, and each element may have up to 512 bytes (ILP32) or
> 1024 bytes (LP64) of value data. Moreover, each control set may contain
> the enum strings and TLV data, which can be up to 64kB and 128kB,
> respectively. Totally, the whole memory consumption may go over 38MB --
> it's quite large, and we'd rather like to reduce the size.
>
> OTOH, there have been other requests even to increase the max number
> of user elements; e.g. ALSA firewire stack require the more user
> controls, hence we want to raise the bar, too.
>
> For satisfying both requirements, this patch changes the management of
> user controls: instead of setting the upper limit of the number of
> user controls, we check the actual memory allocation size and set the
> upper limit of the total allocation in bytes. As long as the memory
> consumption stays below the limit, more user controls are allowed than
> the current limit 32. At the same time, we set the lower limit (8MB)
> as default than the current theoretical limit, in order to lower the
> risk of DoS.
>
> As a compromise for lowering the default limit, now the actual memory
> limit is defined as a module option, 'max_user_ctl_alloc_size', so that
> user can increase/decrease the limit if really needed, too.
>
> Co-developed-by: Takashi Iwai <tiwai@xxxxxxx>
> Reviewed-by: Takashi Sakamoto <o-takashi@xxxxxxxxxxxxx>
> Tested-by: Takashi Sakamoto <o-takashi@xxxxxxxxxxxxx>
> Signed-off-by: Takashi Sakamoto <o-takashi@xxxxxxxxxxxxx>
> ---
> v1->v2: Drop alloc_size field from user_element, calculate at private_free
> v2->v3: Rebase. Fix boundary error. Obsolete macro usage relying on modern
> compiler optimization. Change comment style by modern coding
> convention. Rename module parameter so that users get it easily.
> Patch comment improvements.
> ---
> include/sound/core.h | 2 +-
> sound/core/control.c | 75 ++++++++++++++++++++++++++++++--------------
> 2 files changed, 52 insertions(+), 25 deletions(-)
The original content of patch comes from Iwai-san[1]. I have no clear
idea to handle the case so add 'Co-developed-by' tag to the patch. If
this is not good, I apologize the lack of my understanding to the
development process in Linux kernel.
In this v3 patch, I add below changes to v2 patch:
* Rebase to current HEAD of for-next branch (884c7094a272).
* Fix boundary error.
* Original patch uses 'bigger-or-equal' to max allocation size
* Obsolete macro usage relying on modern compiler optimization
* this seems to be friendry to any static code analyzer
* Change comment style by modern coding convention
* '//' is acceptable and friendry to any static code analyzer
* Rename module parameter so that users get it easily.
* The name with enough length makes users to get it easily
* Patch comment improvements.
* Some explanations are not necessarily correct
I did test this patch by with below script, written with alsa-gobject[2].
```
#!/usr/bin/env python3
from sys import argv, exit
from re import match
import gi
gi.require_version('ALSACtl', '0.0')
from gi.repository import ALSACtl
if len(argv) < 2:
print('One argument is required for card numeric ID.')
exit(1)
card_id = int(argv[1])
card = ALSACtl.Card.new()
card.open(card_id, 0)
# Retrieve current value.
curr_cap = 0
with open('/sys/module/snd/parameters/max_user_ctl_alloc_size', 'r') as f:
buf = f.read()
curr_cap = int(buf)
print('current value of max_user_ctl_alloc_size:', curr_cap)
# Constants.
BYTES_PER_USET_ELEMENT_STRUCT = 320
BYTES_PER_ELEM_VALUE_ENUMERATED = 4
VALUE_COUNT = 128
def user_elem_size(elem_count, label_consumption, tlv_consumption):
return ((BYTES_PER_USET_ELEMENT_STRUCT + elem_count *
BYTES_PER_ELEM_VALUE_ENUMERATED * VALUE_COUNT) +
label_consumption + tlv_consumption)
def calculate_expected_iteration(elem_count, label_consumption,
tlv_consumption, curr_cap):
expected_iteration = 0
consumption = 0
while True:
allocation = user_elem_size(elem_count, label_consumption,
tlv_consumption)
if consumption + allocation > curr_cap:
break
consumption += allocation
expected_iteration += 1
return expected_iteration
def test_allocation(card, elem_count, curr_cap):
labels = (
'Opinion is the medium ',
'between knowledge and ',
'ignorance.',
'Rhythm and harmony ',
'find their way into the ',
'inward places of the soul.',
)
label_consumption = 0
for label in labels:
label_consumption += len(label) + 1
tlv_cntr = [0] * 24
tlv_consumption = len(tlv_cntr) * 4
expected_iteration = calculate_expected_iteration(
elem_count,
label_consumption,
tlv_consumption,
curr_cap)
elem_info = ALSACtl.ElemInfo.new(ALSACtl.ElemType.ENUMERATED)
elem_info.set_enum_data(labels)
access = (ALSACtl.ElemAccessFlag.READ |
ALSACtl.ElemAccessFlag.TLV_READ |
ALSACtl.ElemAccessFlag.TLV_WRITE)
elem_info.set_property('access', access)
elem_info.set_property('value-count', VALUE_COUNT)
consumption = 0
iteration = 0
added_elems = []
while True:
name = 'test-{}'.format(iteration)
elem_id = ALSACtl.ElemId.new_by_name(ALSACtl.ElemIfaceType.MIXER,
0, 0, name, 0)
try:
elem_id_list = card.add_elems(elem_id, elem_count, elem_info)
added_elems.extend(elem_id_list)
card.write_elem_tlv(elem_id_list[0], tlv_cntr)
consumption += user_elem_size(
elem_count,
label_consumption,
tlv_consumption)
iteration += 1
except Exception as e:
groups = match('ioctl\\(.+\\) ([0-9]+)\\(.+\\)', e.message)
if groups is None or int(groups[1]) != 12:
print('unexpected error',
iteration, len(added_elems), consumption, curr_cap)
elif iteration != expected_iteration:
print('unexpected iteration {} but expected {}, {}'.format(
iteration, expected_iteration, consumption))
break
print('expected_iteration: {}, iteration: {}, consumption {}'.format(
expected_iteration, iteration, consumption))
for elem_id in added_elems:
try:
card.remove_elems(elem_id)
except Exception:
pass
for i in range(1, 20):
test_allocation(card, i, curr_cap)
```
The parameter is configured to 12551 and 12552 for boundary check. As a
result:
```
current value of max_user_ctl_alloc_size: 12552
expected_iteration: 11, iteration: 11, consumption 11627
expected_iteration: 8, iteration: 8, consumption 12552
...
current value of max_user_ctl_alloc_size: 12551
expected_iteration: 11, iteration: 11, consumption 11627
expected_iteration: 7, iteration: 7, consumption 10983
...
```
It looks well.
Regards
[1] https://mailman.alsa-project.org/pipermail/alsa-devel/2021-January/179683.html
[2] https://github.com/alsa-project/alsa-gobject/
Takashi Sakamoto
