[alsa-devel] [PATCH 1/3] alsa-lib: fixed coverity reported issues under "FORWARD_NULL" checker.
From: Sudarshan Bisht sudarshan.bisht@nokia.com
Coverity Static Analysis helps developers find hard-to-spot, yet potentially crash-causing defects early in the development phase, reducing the cost,time, and risk of software errors.
This patch has fix for situations where variable can be NULL but not been checked beforehand
--- modules/mixer/simple/sbasedl.c | 2 +- src/conf.c | 1 + src/hwdep/hwdep.c | 1 + src/pcm/pcm_hooks.c | 1 + src/pcm/pcm_simple.c | 2 +- src/rawmidi/rawmidi.c | 1 + src/rawmidi/rawmidi_virt.c | 8 +++++--- 7 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/modules/mixer/simple/sbasedl.c b/modules/mixer/simple/sbasedl.c index 0137586..494802f 100644 --- a/modules/mixer/simple/sbasedl.c +++ b/modules/mixer/simple/sbasedl.c @@ -99,7 +99,7 @@ int mixer_simple_basic_dlopen(snd_mixer_class_t *class, __error: if (initflag) free(priv); - if (h == NULL) + if (h) snd_dlclose(h); free(xlib); return -ENXIO; diff --git a/src/conf.c b/src/conf.c index 8939d62..ddefff6 100644 --- a/src/conf.c +++ b/src/conf.c @@ -3268,6 +3268,7 @@ static int snd_config_hooks_call(snd_config_t *root, snd_config_t *config, snd_c snd_config_iterator_t i, next; if (snd_config_get_type(func_conf) != SND_CONFIG_TYPE_COMPOUND) { SNDERR("Invalid type for func %s definition", str); + err = -EINVAL; goto _err; } snd_config_for_each(i, next, func_conf) { diff --git a/src/hwdep/hwdep.c b/src/hwdep/hwdep.c index b882b35..5dc791c 100644 --- a/src/hwdep/hwdep.c +++ b/src/hwdep/hwdep.c @@ -78,6 +78,7 @@ static int snd_hwdep_open_conf(snd_hwdep_t **hwdep, if (err >= 0) { if (snd_config_get_type(type_conf) != SND_CONFIG_TYPE_COMPOUND) { SNDERR("Invalid type for HWDEP type %s definition", str); + err = -EINVAL; goto _err; } snd_config_for_each(i, next, type_conf) { diff --git a/src/pcm/pcm_hooks.c b/src/pcm/pcm_hooks.c index 3a99d55..404d51e 100644 --- a/src/pcm/pcm_hooks.c +++ b/src/pcm/pcm_hooks.c @@ -385,6 +385,7 @@ static int snd_pcm_hook_add_conf(snd_pcm_t *pcm, snd_config_t *root, snd_config_ if (err >= 0) { if (snd_config_get_type(type) != SND_CONFIG_TYPE_COMPOUND) { SNDERR("Invalid type for PCM type %s definition", str); + err = -EINVAL; goto _err; } snd_config_for_each(i, next, type) { diff --git a/src/pcm/pcm_simple.c b/src/pcm/pcm_simple.c index 975f699..f943ec0 100644 --- a/src/pcm/pcm_simple.c +++ b/src/pcm/pcm_simple.c @@ -89,7 +89,7 @@ static int set_hw_params(snd_pcm_t *pcm, return err; if (periods == 1) return -EINVAL; - if (*period_time == 0) { + if (period_time) { err = INTERNAL(snd_pcm_hw_params_get_period_time)(hw_params, period_time, NULL); if (err < 0) return err; diff --git a/src/rawmidi/rawmidi.c b/src/rawmidi/rawmidi.c index b28488a..0bd6b96 100644 --- a/src/rawmidi/rawmidi.c +++ b/src/rawmidi/rawmidi.c @@ -201,6 +201,7 @@ static int snd_rawmidi_open_conf(snd_rawmidi_t **inputp, snd_rawmidi_t **outputp if (err >= 0) { if (snd_config_get_type(type_conf) != SND_CONFIG_TYPE_COMPOUND) { SNDERR("Invalid type for RAWMIDI type %s definition", str); + err = -EINVAL; goto _err; } snd_config_for_each(i, next, type_conf) { diff --git a/src/rawmidi/rawmidi_virt.c b/src/rawmidi/rawmidi_virt.c index 52b8984..e5b17e4 100644 --- a/src/rawmidi/rawmidi_virt.c +++ b/src/rawmidi/rawmidi_virt.c @@ -383,9 +383,11 @@ int snd_rawmidi_virtual_open(snd_rawmidi_t **inputp, snd_rawmidi_t **outputp, _err: if (seq_handle) snd_seq_close(seq_handle); - if (virt->midi_event) - snd_midi_event_free(virt->midi_event); - free(virt); + if (virt) { + if (virt->midi_event) + snd_midi_event_free(virt->midi_event); + free(virt); + } if (inputp) free(*inputp); if (outputp)
From: Sudarshan Bisht sudarshan.bisht@nokia.com
Coverity Static Analysis helps developers find hard-to-spot, yet potentially crash-causing defects early in the development phase, reducing the cost,time, and risk of software errors.
This patch has got fixes for many potential memory leaks.
--- modules/mixer/simple/sbase.c | 1 + modules/mixer/simple/sbasedl.c | 2 +- src/conf.c | 3 +++ src/control/control_hw.c | 4 +++- src/pcm/pcm_file.c | 4 +++- src/pcm/pcm_ladspa.c | 20 ++++++++++++++------ src/pcm/pcm_plug.c | 9 +++++++-- src/pcm/pcm_rate.c | 4 ++++ src/rawmidi/rawmidi.c | 4 +++- 9 files changed, 39 insertions(+), 12 deletions(-)
diff --git a/modules/mixer/simple/sbase.c b/modules/mixer/simple/sbase.c index 97feee8..bb2f59d 100644 --- a/modules/mixer/simple/sbase.c +++ b/modules/mixer/simple/sbase.c @@ -377,6 +377,7 @@ static int simple_event_add1(snd_mixer_class_t *class, if (ctype != SND_CTL_ELEM_TYPE_BOOLEAN) { __invalid_type: snd_mixer_selem_id_free(id); + free(hsimple); return -EINVAL; } break; diff --git a/modules/mixer/simple/sbasedl.c b/modules/mixer/simple/sbasedl.c index 0137586..494802f 100644 --- a/modules/mixer/simple/sbasedl.c +++ b/modules/mixer/simple/sbasedl.c @@ -99,7 +99,7 @@ int mixer_simple_basic_dlopen(snd_mixer_class_t *class, __error: if (initflag) free(priv); - if (h == NULL) + if (h) snd_dlclose(h); free(xlib); return -ENXIO; diff --git a/src/conf.c b/src/conf.c index 8939d62..91af6c7 100644 --- a/src/conf.c +++ b/src/conf.c @@ -638,6 +638,7 @@ static int get_char_skip_comments(input_t *input) fd = malloc(sizeof(*fd)); if (!fd) { free(str); + snd_input_close(in); return -ENOMEM; } fd->name = str; @@ -4589,6 +4590,8 @@ static int parse_args(snd_config_t *subs, const char *str, snd_config_t *defs) if (err < 0) { _err: free(val); + if (sub) + snd_config_delete(sub); return err; } free(val); diff --git a/src/control/control_hw.c b/src/control/control_hw.c index cf258b4..29cbbc6 100644 --- a/src/control/control_hw.c +++ b/src/control/control_hw.c @@ -230,8 +230,10 @@ static int snd_ctl_hw_elem_tlv(snd_ctl_t *handle, int op_flag, return -errno; } if (op_flag == 0) { - if (xtlv->tlv[1] + 2 * sizeof(unsigned int) > tlv_size) + if (xtlv->tlv[1] + 2 * sizeof(unsigned int) > tlv_size) { + free(xtlv); return -EFAULT; + } memcpy(tlv, xtlv->tlv, xtlv->tlv[1] + 2 * sizeof(unsigned int)); } free(xtlv); diff --git a/src/pcm/pcm_file.c b/src/pcm/pcm_file.c index bfa1cc8..adba7c0 100644 --- a/src/pcm/pcm_file.c +++ b/src/pcm/pcm_file.c @@ -226,7 +226,9 @@ static int snd_pcm_file_open_output_file(snd_pcm_file_t *file) file->final_fname); return -errno; } - fd = fileno(pipe); + fd = dup(fileno(pipe)); + if (fd >= 0) + fclose(pipe); } else { if (file->trunc) fd = open(file->final_fname, O_WRONLY|O_CREAT|O_TRUNC, diff --git a/src/pcm/pcm_ladspa.c b/src/pcm/pcm_ladspa.c index c413c10..41e1d38 100644 --- a/src/pcm/pcm_ladspa.c +++ b/src/pcm/pcm_ladspa.c @@ -750,8 +750,10 @@ static int snd_pcm_ladspa_allocate_memory(snd_pcm_t *pcm, snd_pcm_ladspa_t *lads if (instance->input.data == NULL || instance->input.m_data == NULL || instance->output.data == NULL || - instance->output.m_data == NULL) + instance->output.m_data == NULL) { + free(pchannels); return -ENOMEM; + } for (idx = 0; idx < instance->input.channels.size; idx++) { chn = instance->output.channels.array[idx]; if (pchannels[chn] == NULL && chn < ichannels) { @@ -761,8 +763,10 @@ static int snd_pcm_ladspa_allocate_memory(snd_pcm_t *pcm, snd_pcm_ladspa_t *lads instance->input.data[idx] = pchannels[chn]; if (instance->input.data[idx] == NULL) { instance->input.data[idx] = snd_pcm_ladspa_allocate_zero(ladspa, 0); - if (instance->input.data[idx] == NULL) - return -ENOMEM; + if (instance->input.data[idx] == NULL) { + free(pchannels); + return -ENOMEM; + } } } for (idx = 0; idx < instance->output.channels.size; idx++) { @@ -770,8 +774,10 @@ static int snd_pcm_ladspa_allocate_memory(snd_pcm_t *pcm, snd_pcm_ladspa_t *lads /* FIXME/OPTIMIZE: check if we can remove double alloc */ /* if LADSPA plugin has no broken inplace */ instance->output.data[idx] = malloc(sizeof(LADSPA_Data) * ladspa->allocated); - if (instance->output.data[idx] == NULL) - return -ENOMEM; + if (instance->output.data[idx] == NULL) { + free(pchannels); + return -ENOMEM; + } pchannels[chn] = instance->output.m_data[idx] = instance->output.data[idx]; } } @@ -793,8 +799,10 @@ static int snd_pcm_ladspa_allocate_memory(snd_pcm_t *pcm, snd_pcm_ladspa_t *lads instance->output.data[idx] = NULL; } else { instance->output.data[idx] = snd_pcm_ladspa_allocate_zero(ladspa, 1); - if (instance->output.data[idx] == NULL) + if (instance->output.data[idx] == NULL) { + free(pchannels); return -ENOMEM; + } } } } diff --git a/src/pcm/pcm_plug.c b/src/pcm/pcm_plug.c index e9d2923..e80bd3e 100644 --- a/src/pcm/pcm_plug.c +++ b/src/pcm/pcm_plug.c @@ -1298,6 +1298,7 @@ int _snd_pcm_plug_open(snd_pcm_t **pcmp, const char *name, err = snd_pcm_route_load_ttable(tt, ttable, csize, ssize, &cused, &sused, -1); if (err < 0) { snd_config_delete(sconf); + free(ttable); return err; } } @@ -1310,12 +1311,16 @@ int _snd_pcm_plug_open(snd_pcm_t **pcmp, const char *name,
err = snd_pcm_open_slave(&spcm, root, sconf, stream, mode, conf); snd_config_delete(sconf); - if (err < 0) + if (err < 0) { + free(ttable); return err; + } err = snd_pcm_plug_open(pcmp, name, sformat, schannels, srate, rate_converter, route_policy, ttable, ssize, cused, sused, spcm, 1); - if (err < 0) + if (err < 0) { + free(ttable); snd_pcm_close(spcm); + } return err; } #ifndef DOC_HIDDEN diff --git a/src/pcm/pcm_rate.c b/src/pcm/pcm_rate.c index 70e30e5..cf218ae 100644 --- a/src/pcm/pcm_rate.c +++ b/src/pcm/pcm_rate.c @@ -1392,11 +1392,13 @@ int snd_pcm_rate_open(snd_pcm_t **pcmp, const char *name, } else { SNDERR("Invalid type for rate converter"); snd_pcm_close(pcm); + free(rate); return -EINVAL; } if (err < 0) { SNDERR("Cannot find rate converter"); snd_pcm_close(pcm); + free(rate); return -ENOENT; } #else @@ -1405,6 +1407,7 @@ int snd_pcm_rate_open(snd_pcm_t **pcmp, const char *name, err = open_func(SND_PCM_RATE_PLUGIN_VERSION, &rate->obj, &rate->ops); if (err < 0) { snd_pcm_close(pcm); + free(rate); return err; } #endif @@ -1413,6 +1416,7 @@ int snd_pcm_rate_open(snd_pcm_t **pcmp, const char *name, ! rate->ops.input_frames || ! rate->ops.output_frames) { SNDERR("Inproper rate plugin %s initialization", type); snd_pcm_close(pcm); + free(rate); return err; }
diff --git a/src/rawmidi/rawmidi.c b/src/rawmidi/rawmidi.c index b28488a..25732d2 100644 --- a/src/rawmidi/rawmidi.c +++ b/src/rawmidi/rawmidi.c @@ -255,8 +255,10 @@ static int snd_rawmidi_open_conf(snd_rawmidi_t **inputp, snd_rawmidi_t **outputp snd_config_delete(type_conf); if (err >= 0) err = open_func(inputp, outputp, name, rawmidi_root, rawmidi_conf, mode); - if (err < 0) + if (err < 0) { + snd_dlclose(h); return err; + } if (inputp) { (*inputp)->dl_handle = h; h = NULL; snd_rawmidi_params_default(*inputp, ¶ms);
From: Sudarshan Bisht sudarshan.bisht@nokia.com
Coverity Static Analysis helps developers find hard-to-spot, yet potentially crash-causing defects early in the development phase, reducing the cost,time, and risk of software errors.
This patch has a fix for conditions where 1) variable is used evern after getting freed thus causing junk assignment and 2) double free of memory.
--- modules/mixer/simple/python.c | 1 + src/control/control_hw.c | 1 + src/mixer/mixer.c | 1 - 3 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/modules/mixer/simple/python.c b/modules/mixer/simple/python.c index c822c52..784d9a0 100644 --- a/modules/mixer/simple/python.c +++ b/modules/mixer/simple/python.c @@ -663,6 +663,7 @@ pymixer_attach_hctl(struct pymixer *pymixer, PyObject *args) return NULL; err = snd_mixer_attach_hctl(pymixer->mixer, hctl); if (err < 0) { + snd_hctl_close(hctl); PyErr_Format(PyExc_RuntimeError, "Cannot attach hctl: %s", snd_strerror(err)); return NULL; } diff --git a/src/control/control_hw.c b/src/control/control_hw.c index cf258b4..16c4987 100644 --- a/src/control/control_hw.c +++ b/src/control/control_hw.c @@ -414,6 +414,7 @@ int snd_ctl_hw_open(snd_ctl_t **handle, const char *name, int card, int mode) if (err < 0) { close(fd); free(hw); + return err; } ctl->ops = &snd_ctl_hw_ops; ctl->private_data = hw; diff --git a/src/mixer/mixer.c b/src/mixer/mixer.c index 85d843f..5e5789a 100644 --- a/src/mixer/mixer.c +++ b/src/mixer/mixer.c @@ -228,7 +228,6 @@ int snd_mixer_attach_hctl(snd_mixer_t *mixer, snd_hctl_t *hctl) return -ENOMEM; err = snd_hctl_nonblock(hctl, 1); if (err < 0) { - snd_hctl_close(hctl); free(slave); return err; }
At Tue, 12 Apr 2011 13:09:45 +0300, sudarshan.bisht@nokia.com wrote:
From: Sudarshan Bisht sudarshan.bisht@nokia.com
Coverity Static Analysis helps developers find hard-to-spot, yet potentially crash-causing defects early in the development phase, reducing the cost,time, and risk of software errors.
This patch has fix for situations where variable can be NULL but not been checked beforehand
Applied now (finally!). Thanks for the patch.
Takashi
modules/mixer/simple/sbasedl.c | 2 +- src/conf.c | 1 + src/hwdep/hwdep.c | 1 + src/pcm/pcm_hooks.c | 1 + src/pcm/pcm_simple.c | 2 +- src/rawmidi/rawmidi.c | 1 + src/rawmidi/rawmidi_virt.c | 8 +++++--- 7 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/modules/mixer/simple/sbasedl.c b/modules/mixer/simple/sbasedl.c index 0137586..494802f 100644 --- a/modules/mixer/simple/sbasedl.c +++ b/modules/mixer/simple/sbasedl.c @@ -99,7 +99,7 @@ int mixer_simple_basic_dlopen(snd_mixer_class_t *class, __error: if (initflag) free(priv);
- if (h == NULL)
- if (h) snd_dlclose(h); free(xlib); return -ENXIO;
diff --git a/src/conf.c b/src/conf.c index 8939d62..ddefff6 100644 --- a/src/conf.c +++ b/src/conf.c @@ -3268,6 +3268,7 @@ static int snd_config_hooks_call(snd_config_t *root, snd_config_t *config, snd_c snd_config_iterator_t i, next; if (snd_config_get_type(func_conf) != SND_CONFIG_TYPE_COMPOUND) { SNDERR("Invalid type for func %s definition", str);
err = -EINVAL; goto _err;diff --git a/src/hwdep/hwdep.c b/src/hwdep/hwdep.c index b882b35..5dc791c 100644 --- a/src/hwdep/hwdep.c +++ b/src/hwdep/hwdep.c @@ -78,6 +78,7 @@ static int snd_hwdep_open_conf(snd_hwdep_t **hwdep, if (err >= 0) { if (snd_config_get_type(type_conf) != SND_CONFIG_TYPE_COMPOUND) { SNDERR("Invalid type for HWDEP type %s definition", str);
err = -EINVAL; goto _err;diff --git a/src/pcm/pcm_hooks.c b/src/pcm/pcm_hooks.c index 3a99d55..404d51e 100644 --- a/src/pcm/pcm_hooks.c +++ b/src/pcm/pcm_hooks.c @@ -385,6 +385,7 @@ static int snd_pcm_hook_add_conf(snd_pcm_t *pcm, snd_config_t *root, snd_config_ if (err >= 0) { if (snd_config_get_type(type) != SND_CONFIG_TYPE_COMPOUND) { SNDERR("Invalid type for PCM type %s definition", str);
err = -EINVAL; goto _err;diff --git a/src/pcm/pcm_simple.c b/src/pcm/pcm_simple.c index 975f699..f943ec0 100644 --- a/src/pcm/pcm_simple.c +++ b/src/pcm/pcm_simple.c @@ -89,7 +89,7 @@ static int set_hw_params(snd_pcm_t *pcm, return err; if (periods == 1) return -EINVAL;
if (*period_time == 0) {
if (period_time) { err = INTERNAL(snd_pcm_hw_params_get_period_time)(hw_params, period_time, NULL); if (err < 0) return err;diff --git a/src/rawmidi/rawmidi.c b/src/rawmidi/rawmidi.c index b28488a..0bd6b96 100644 --- a/src/rawmidi/rawmidi.c +++ b/src/rawmidi/rawmidi.c @@ -201,6 +201,7 @@ static int snd_rawmidi_open_conf(snd_rawmidi_t **inputp, snd_rawmidi_t **outputp if (err >= 0) { if (snd_config_get_type(type_conf) != SND_CONFIG_TYPE_COMPOUND) { SNDERR("Invalid type for RAWMIDI type %s definition", str);
err = -EINVAL; goto _err;diff --git a/src/rawmidi/rawmidi_virt.c b/src/rawmidi/rawmidi_virt.c index 52b8984..e5b17e4 100644 --- a/src/rawmidi/rawmidi_virt.c +++ b/src/rawmidi/rawmidi_virt.c @@ -383,9 +383,11 @@ int snd_rawmidi_virtual_open(snd_rawmidi_t **inputp, snd_rawmidi_t **outputp, _err: if (seq_handle) snd_seq_close(seq_handle);
- if (virt->midi_event)
snd_midi_event_free(virt->midi_event);- free(virt);
- if (virt) {
if (virt->midi_event)snd_midi_event_free(virt->midi_event);free(virt);- } if (inputp) free(*inputp); if (outputp)
-- 1.7.0.4
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
participants (2)
-
sudarshan.bisht＠nokia.com -
Takashi Iwai