[alsa-devel] SEGFault when optininal snd_ctl_ext_callback::read_event() not set

[Takashi Iwai]

On Thu, 05 Oct 2017 16:28:33 +0200,
Wischer, Timo (ADITG/ESB) wrote:
> 
> Hi Takashi,
> 
> see attached the patch.
> >From my opinion failing with an error is the best solution (see explanation in the commit message)

Fair enough, applied now.
Thanks.


Takashi


> 
> 
> Best regards
> 
> Timo Wischer
> 
> Advanced Driver Information Technology GmbH
> Engineering Software Base (ADITG/ESB)
> Robert-Bosch-Str. 200
> 31139 Hildesheim
> Germany
> 
> Tel. +49 5121 49 6938
> Fax +49 5121 49 6999
> twischer at de.adit-jv.com
> 
> ADIT is a joint venture company of Robert Bosch GmbH/Robert Bosch Car Multimedia GmbH and DENSO Corporation
> Sitz: Hildesheim, Registergericht: Amtsgericht Hildesheim HRB 3438
> Geschäftsführung: Wilhelm Grabow, Ken Yaguchi
> 
> ________________________________________
> From: Takashi Iwai [tiwai at suse.de]
> Sent: Thursday, October 05, 2017 3:17 PM
> To: Wischer, Timo (ADITG/ESB)
> Cc: alsa-devel at alsa-project.org
> Subject: Re: [alsa-devel] SEGFault when optininal       snd_ctl_ext_callback::read_event() not set
> 
> On Thu, 05 Oct 2017 14:03:14 +0200,
> Wischer, Timo (ADITG/ESB) wrote:
> >
> > Hi all,
> >
> > snd_ctl_ext_callback::read_event() callback is mentioned as optional
> > (see http://git.alsa-project.org/?p=alsa-lib.git;a=blob;f=include/control_external.h;h=12958e70a5230de9c74029c641a395a2073c8646;hb=refs/heads/master#l239)
> >
> > but there is no NULL check and the NULL pointer will be called if the read_event function callback pointer is not set.
> > (see http://git.alsa-project.org/?p=alsa-lib.git;a=blob;f=src/control/control_ext.c;h=56552fa1aa0ef0e6383abf4029b63944a841c2c4;hb=refs/heads/master#l419)
> >
> > I think a default function has to be provided which will be called when the callback is not set or the read_event() callback should not be marked as optional.
> >
> > What is your opinion?
> 
> It should have a NULL check there, as documentation clearly states
> that it's optional.
> 
> Care to submit a fix patch?
> 
> 
> thanks,
> 
> Takashi
> From 8cc3dbc6fad0a4de82ab4b5c95a86499c99d56d8 Mon Sep 17 00:00:00 2001
> From: Timo Wischer <twischer at de.adit-jv.com>
> Date: Thu, 5 Oct 2017 16:25:23 +0200
> Subject: ctl: ext: Fail with error code if snd_ctl_ext_callback::read_event()
>  callback is not defined
> 
> The snd_ctl_ext_callback::read_event() callback is only optional
> if no poll descriptor was given via
> snd_ctl_ext_t::poll_fd
> or
> snd_ctl_ext_callback::snd_ctl_ext_poll_descriptors().
> 
> If a poll descriptor is given the
> snd_ctl_ext_callback::read_event()
> callback has also to be defined
> because there is no minigful default behavior.
> 
> This callback will be called when ever the poll() on
> the file descriptor indicates that there is an event pending.
> Therefore returning a 0 which indicates that there is no event makes no
> sense.
> 
> Signed-off-by: Timo Wischer <twischer at de.adit-jv.com>
> 
> diff --git a/src/control/control_ext.c b/src/control/control_ext.c
> index 56552fa..d7de8e8 100644
> --- a/src/control/control_ext.c
> +++ b/src/control/control_ext.c
> @@ -415,8 +415,12 @@ static int snd_ctl_ext_read(snd_ctl_t *handle, snd_ctl_event_t *event)
>  {
>  	snd_ctl_ext_t *ext = handle->private_data;
>  
> -	memset(event, 0, sizeof(*event));
> -	return ext->callback->read_event(ext, &event->data.elem.id, &event->data.elem.mask);
> +	if (ext->callback->read_event) {
> +		memset(event, 0, sizeof(*event));
> +		return ext->callback->read_event(ext, &event->data.elem.id, &event->data.elem.mask);
> +	}
> +
> +	return -EINVAL;
>  }
>  
>  static int snd_ctl_ext_poll_descriptors_count(snd_ctl_t *handle)