[alsa-devel] [PATCH] ASoC: Intel: Add NULL checks for the stream pointer

[Jie, Yang]

> -----Original Message-----
> From: Mark Brown [mailto:broonie at kernel.org]
> Sent: Wednesday, January 07, 2015 1:22 AM
> To: Jie, Yang
> Cc: alsa-devel at alsa-project.org; Girdwood, Liam R
> Subject: Re: [PATCH] ASoC: Intel: Add NULL checks for the stream pointer
> 
> On Tue, Jan 06, 2015 at 11:04:07PM +0800, Jie Yang wrote:
> 
> > We should not send IPC stream commands to FW when the stream is NULL,
> > dereference the NULL pointer may also occur without precheck.
> > Here add NULL pointer checks for these stream APIs.
> 
> > @@ -1420,6 +1423,9 @@ int sst_hsw_stream_commit(struct sst_hsw *hsw,
> struct sst_hsw_stream *stream)
> >  	u32 header;
> >  	int ret;
> >
> > +	if (stream->commited)
> > +		return 0;
> > +
> >  	trace_ipc_request("stream alloc", stream->host_id);
> >
> >  	header = IPC_GLB_TYPE(IPC_GLB_ALLOCATE_STREAM);
> 
> This is a bit worrying, it means that we will silently ignore any attempts to pass
> in a NULL stream.  Sometimes that's an OK thing to do (like in deallocation
> paths) but this seems to more generally silently ignore errors which means
> we're less likely to spot other coding issues.
> Race conditions worry me especially - trying to use something before we've
> quite allocated it for example.
> 
> I'd be much happier if this warned in cases where it wasn't totally obvious that
> attempting to use a NULL pointer is sensible.
[Keyon] thanks for review, Mark. So you meant we should add some warning log here, right?


~Keyon