Re: [PATCH v3] ALSA: control: Add memory consumption limit to user controls

[Takashi Iwai <tiwai@suse.de>]

On Thu, 08 Apr 2021 12:50:25 +0200,
Takashi Sakamoto wrote:
> 
> 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@suse.de>
> > Reviewed-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
> > Tested-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
> > Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
> > ---
> > 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.

It depends.  In some cases, you just carry the patch with the original
authorship (From address) and put your sign-off.  In some cases,
Co-developed-by can be used.  I don't mind much either way, so I took
your v3 patch now (with the addition of the Link URL to v2 patch).


Thanks!

Takashi