sound: avoid potential crash upon naming error

[khorben]

In chn_init(), if dsp_unit2name() fails, the code sets ret to EINVAL and jumps to the out2 label, where the b and bs snd_dbuf variables may be destroyed without having been initialized in the first place.

Reported by: Coverity Scan
CID: 1545029
CID: 1545025
Sponsored by: The FreeBSD Foundation

[khorben]

Cc @christosmarg

[bsdimp]

Is there a reason we don't just initialize these to null where they are declared above?

[christosmarg]

LGTM.

@bsdimp We could also do that, I guess it's just cleaner to do it the way Pierre did and have all the declarations in the same place.

[christosmarg]

Hmmm @khorben it seems that this can also be a problem for this block as well

	CHN_INIT(c, children);
	CHN_INIT(c, children.busy);
	c->devinfo = NULL;
	c->feeder = NULL;
	c->latency = -1;
	c->timeout = 1;

out2 will check if c->devinfo is NULL, but we also initialize it after the call to dsp_unit2name().

If you want I can take care of cleaning the whole initialization in chn_init()

[khorben]

Hmmm @khorben it seems that this can also be a problem for this block as well

	CHN_INIT(c, children);
	CHN_INIT(c, children.busy);
	c->devinfo = NULL;
	c->feeder = NULL;
	c->latency = -1;
	c->timeout = 1;

out2 will check if c->devinfo is NULL, but we also initialize it after the call to dsp_unit2name().

If you want I can take care of cleaning the whole initialization in chn_init()

Unlike b and bs, c->devinfo is initialised to NULL upon allocation, and before any possible jump to out2:

1241         c = malloc(sizeof(*c), M_DEVBUF, M_WAITOK | M_ZERO);
                                                         ^^^^^^

So this looks fine to me.

[christosmarg]

@khorben I fixed this in a follow-up commit: 2db2292