This patchset removes two variable length arrays from the jack audio
backend.  The codebase has very few VLAs, and if we can get rid of
them all we can make the compiler error on new additions.  This is a
defensive measure against security bugs where an on-stack dynamic
allocation isn't correctly size-checked (e.g.  CVE-2021-3527).

The first one is fairly straightforward (although the JACK API's
requirement that (a) you don't pass it an overlong client name and
(b) that maximum length is provided by calling a function, not as a
compile time constant makes it a little less clean than it might be.

The second one avoids the dynamic allocation, but if the audio
subsystem has a compile-time upper bound on the number of
channels then we could use a fixed-size stack array rather than
the awkward "allocate a working buffer at init time" that I
have in this patch. Suggestions for improvements welcome.

Disclaimer: tested only with "make check", which doesn't actually
exercise the audio subsystem.

thanks
-- PMM

Peter Maydell (2):
  audio/jackaudio: Avoid dynamic stack allocation in qjack_client_init
  audio/jackaudio: Avoid dynamic stack allocation in qjack_process()

 audio/jackaudio.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

-- 
2.34.1

Hi Gerd; this series has been reviewed. Did you want to take it
via the audio tree? Or I can put it in via target-arm.next
if you prefer.

thanks
-- PMM

On Fri, 18 Aug 2023 at 16:58, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> This patchset removes two variable length arrays from the jack audio
> backend.  The codebase has very few VLAs, and if we can get rid of
> them all we can make the compiler error on new additions.  This is a
> defensive measure against security bugs where an on-stack dynamic
> allocation isn't correctly size-checked (e.g.  CVE-2021-3527).
>
> The first one is fairly straightforward (although the JACK API's
> requirement that (a) you don't pass it an overlong client name and
> (b) that maximum length is provided by calling a function, not as a
> compile time constant makes it a little less clean than it might be.
>
> The second one avoids the dynamic allocation, but if the audio
> subsystem has a compile-time upper bound on the number of
> channels then we could use a fixed-size stack array rather than
> the awkward "allocate a working buffer at init time" that I
> have in this patch. Suggestions for improvements welcome.
>
> Disclaimer: tested only with "make check", which doesn't actually
> exercise the audio subsystem.
>
> thanks
> -- PMM
>
> Peter Maydell (2):
>   audio/jackaudio: Avoid dynamic stack allocation in qjack_client_init
>   audio/jackaudio: Avoid dynamic stack allocation in qjack_process()
>
>  audio/jackaudio.c | 21 ++++++++++++++-------
>  1 file changed, 14 insertions(+), 7 deletions(-)

On Tue, Sep 12, 2023 at 03:19:08PM +0100, Peter Maydell wrote:
> Hi Gerd; this series has been reviewed. Did you want to take it
> via the audio tree? Or I can put it in via target-arm.next
> if you prefer.

arm tree is fine with me.

take care,
  Gerd

On Fri, Aug 18, 2023 at 7:59 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> This patchset removes two variable length arrays from the jack audio
> backend.  The codebase has very few VLAs, and if we can get rid of
> them all we can make the compiler error on new additions.  This is a
> defensive measure against security bugs where an on-stack dynamic
> allocation isn't correctly size-checked (e.g.  CVE-2021-3527).
>
> The first one is fairly straightforward (although the JACK API's
> requirement that (a) you don't pass it an overlong client name and
> (b) that maximum length is provided by calling a function, not as a
> compile time constant makes it a little less clean than it might be.
>
> The second one avoids the dynamic allocation, but if the audio
> subsystem has a compile-time upper bound on the number of
> channels then we could use a fixed-size stack array rather than
> the awkward "allocate a working buffer at init time" that I
> have in this patch. Suggestions for improvements welcome.
>
> Disclaimer: tested only with "make check", which doesn't actually
> exercise the audio subsystem.

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


-- 
Marc-André Lureau