[alsa-devel] [PATCH 00/10] Yet another small covery bug fixes
Hi,
here is yet another series of fixes for bugs reported by coverity. Mostly driver-specific bugs, no serious ones, as far as I look over them.
Takashi
Spotted by coverity CID 115196.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/drivers/opl3/opl3_midi.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/sound/drivers/opl3/opl3_midi.c b/sound/drivers/opl3/opl3_midi.c index 0c796bcbc0a3..6c6d09a51f42 100644 --- a/sound/drivers/opl3/opl3_midi.c +++ b/sound/drivers/opl3/opl3_midi.c @@ -390,6 +390,11 @@ void snd_opl3_note_on(void *p, int note, int vel, struct snd_midi_channel *chan) voice = snd_opl3_oss_map[chan->number]; }
+ if (voice < 0) { + spin_unlock_irqrestore(&opl3->voice_lock, flags); + return; + } + if (voice < MAX_OPL2_VOICES) { /* Left register block for voices 0 .. 8 */ reg_side = OPL3_LEFT;
When nopcm=1 is set, some initializations based on hrtimer resolution might be bogus because the driver checks the resolution only when nopcm=0. Simply get the resolution always at first for fixing the bug.
Spotted by coverity CID 139740.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/drivers/pcsp/pcsp.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/sound/drivers/pcsp/pcsp.c b/sound/drivers/pcsp/pcsp.c index 1c19cd7ad26e..f664bae3b9b0 100644 --- a/sound/drivers/pcsp/pcsp.c +++ b/sound/drivers/pcsp/pcsp.c @@ -46,8 +46,9 @@ static int snd_pcsp_create(struct snd_card *card) int err; int div, min_div, order;
+ hrtimer_get_res(CLOCK_MONOTONIC, &tp); + if (!nopcm) { - hrtimer_get_res(CLOCK_MONOTONIC, &tp); if (tp.tv_sec || tp.tv_nsec > PCSP_MAX_PERIOD_NS) { printk(KERN_ERR "PCSP: Timer resolution is not sufficient " "(%linS)\n", tp.tv_nsec);
When no proper id string is given, the driver tries to fall back to copy the proc_root name string via strcpy(), but this might overflow the fixed string size. Let's use strlcpy().
Spotted by coverity CID 139008.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/core/init.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/core/init.c b/sound/core/init.c index 01a89383a062..1351f22f651c 100644 --- a/sound/core/init.c +++ b/sound/core/init.c @@ -597,7 +597,7 @@ static void snd_card_set_id_no_lock(struct snd_card *card, const char *src, /* last resort... */ snd_printk(KERN_ERR "unable to set card id (%s)\n", id); if (card->proc_root->name) - strcpy(card->id, card->proc_root->name); + strlcpy(card->id, card->proc_root->name, sizeof(card->id)); }
/**
We tend to make stupid mistakes with strncpy(). Let's take a safer one, strlcpy().
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/arm/pxa2xx-ac97.c | 2 +- sound/i2c/other/ak4xxx-adda.c | 2 +- sound/pci/cs5535audio/cs5535audio_olpc.c | 4 ++-- sound/pci/ice1712/psc724.c | 4 ++-- sound/pci/ice1712/wm8776.c | 2 +- sound/pci/rme9652/hdspm.c | 2 +- 6 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/sound/arm/pxa2xx-ac97.c b/sound/arm/pxa2xx-ac97.c index 5066a3768b28..9a2ac1e0f77a 100644 --- a/sound/arm/pxa2xx-ac97.c +++ b/sound/arm/pxa2xx-ac97.c @@ -185,7 +185,7 @@ static int pxa2xx_ac97_probe(struct platform_device *dev) goto err;
card->dev = &dev->dev; - strncpy(card->driver, dev->dev.driver->name, sizeof(card->driver)); + strlcpy(card->driver, dev->dev.driver->name, sizeof(card->driver));
ret = pxa2xx_pcm_new(card, &pxa2xx_ac97_pcm_client, &pxa2xx_ac97_pcm); if (ret) diff --git a/sound/i2c/other/ak4xxx-adda.c b/sound/i2c/other/ak4xxx-adda.c index ed726d1569e8..f3735e64791c 100644 --- a/sound/i2c/other/ak4xxx-adda.c +++ b/sound/i2c/other/ak4xxx-adda.c @@ -583,7 +583,7 @@ static int ak4xxx_capture_source_info(struct snd_kcontrol *kcontrol, if (idx >= num_names) return -EINVAL; input_names = ak->adc_info[mixer_ch].input_names; - strncpy(uinfo->value.enumerated.name, input_names[idx], + strlcpy(uinfo->value.enumerated.name, input_names[idx], sizeof(uinfo->value.enumerated.name)); return 0; } diff --git a/sound/pci/cs5535audio/cs5535audio_olpc.c b/sound/pci/cs5535audio/cs5535audio_olpc.c index da1cb9c4c76c..e6a44507d557 100644 --- a/sound/pci/cs5535audio/cs5535audio_olpc.c +++ b/sound/pci/cs5535audio/cs5535audio_olpc.c @@ -161,13 +161,13 @@ int olpc_quirks(struct snd_card *card, struct snd_ac97 *ac97) /* drop the original AD1888 HPF control */ memset(&elem, 0, sizeof(elem)); elem.iface = SNDRV_CTL_ELEM_IFACE_MIXER; - strncpy(elem.name, "High Pass Filter Enable", sizeof(elem.name)); + strlcpy(elem.name, "High Pass Filter Enable", sizeof(elem.name)); snd_ctl_remove_id(card, &elem);
/* drop the original V_REFOUT control */ memset(&elem, 0, sizeof(elem)); elem.iface = SNDRV_CTL_ELEM_IFACE_MIXER; - strncpy(elem.name, "V_REFOUT Enable", sizeof(elem.name)); + strlcpy(elem.name, "V_REFOUT Enable", sizeof(elem.name)); snd_ctl_remove_id(card, &elem);
/* add the OLPC-specific controls */ diff --git a/sound/pci/ice1712/psc724.c b/sound/pci/ice1712/psc724.c index 302ac6ddd545..4019cf27d117 100644 --- a/sound/pci/ice1712/psc724.c +++ b/sound/pci/ice1712/psc724.c @@ -203,12 +203,12 @@ static void psc724_set_jack_state(struct snd_ice1712 *ice, bool hp_connected) /* notify about master speaker mute change */ memset(&elem_id, 0, sizeof(elem_id)); elem_id.iface = SNDRV_CTL_ELEM_IFACE_MIXER; - strncpy(elem_id.name, "Master Speakers Playback Switch", + strlcpy(elem_id.name, "Master Speakers Playback Switch", sizeof(elem_id.name)); kctl = snd_ctl_find_id(ice->card, &elem_id); snd_ctl_notify(ice->card, SNDRV_CTL_EVENT_MASK_VALUE, &kctl->id); /* and headphone mute change */ - strncpy(elem_id.name, spec->wm8776.ctl[WM8776_CTL_HP_SW].name, + strlcpy(elem_id.name, spec->wm8776.ctl[WM8776_CTL_HP_SW].name, sizeof(elem_id.name)); kctl = snd_ctl_find_id(ice->card, &elem_id); snd_ctl_notify(ice->card, SNDRV_CTL_EVENT_MASK_VALUE, &kctl->id); diff --git a/sound/pci/ice1712/wm8776.c b/sound/pci/ice1712/wm8776.c index a3c05fe5daf9..5227cb08247f 100644 --- a/sound/pci/ice1712/wm8776.c +++ b/sound/pci/ice1712/wm8776.c @@ -52,7 +52,7 @@ static void snd_wm8776_activate_ctl(struct snd_wm8776 *wm, unsigned int index_offset;
memset(&elem_id, 0, sizeof(elem_id)); - strncpy(elem_id.name, ctl_name, sizeof(elem_id.name)); + strlcpy(elem_id.name, ctl_name, sizeof(elem_id.name)); elem_id.iface = SNDRV_CTL_ELEM_IFACE_MIXER; kctl = snd_ctl_find_id(card, &elem_id); if (!kctl) diff --git a/sound/pci/rme9652/hdspm.c b/sound/pci/rme9652/hdspm.c index 2907e68150cb..e98dc008de0b 100644 --- a/sound/pci/rme9652/hdspm.c +++ b/sound/pci/rme9652/hdspm.c @@ -6400,7 +6400,7 @@ static int snd_hdspm_hwdep_ioctl(struct snd_hwdep *hw, struct file *file, memset(&hdspm_version, 0, sizeof(hdspm_version));
hdspm_version.card_type = hdspm->io_type; - strncpy(hdspm_version.cardname, hdspm->card_name, + strlcpy(hdspm_version.cardname, hdspm->card_name, sizeof(hdspm_version.cardname)); hdspm_version.serial = hdspm->serial; hdspm_version.firmware_rev = hdspm->firmware_rev;
The right attenuation bits aren't needed to be shifted.
Spotted by coverity CID 11427.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/ad1889.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/pci/ad1889.c b/sound/pci/ad1889.c index d2b9d617aee5..b680d03e2419 100644 --- a/sound/pci/ad1889.c +++ b/sound/pci/ad1889.c @@ -739,7 +739,7 @@ snd_ad1889_proc_read(struct snd_info_entry *entry, struct snd_info_buffer *buffe reg = ad1889_readw(chip, AD_DS_WADA); snd_iprintf(buffer, "Right: %s, -%d dB\n", (reg & AD_DS_WADA_RWAM) ? "mute" : "unmute", - ((reg & AD_DS_WADA_RWAA) >> 8) * 3); + (reg & AD_DS_WADA_RWAA) * 3); reg = ad1889_readw(chip, AD_DS_WAS); snd_iprintf(buffer, "Wave samplerate: %u Hz\n", reg);
The variable runtime is never used, and this might be even a source of NULL-dereference. Nothing better than killing it.
Spotted by coverity CID 100862.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/ali5451/ali5451.c | 2 -- 1 file changed, 2 deletions(-)
diff --git a/sound/pci/ali5451/ali5451.c b/sound/pci/ali5451/ali5451.c index 3dfa12b670eb..c6835a3d64fb 100644 --- a/sound/pci/ali5451/ali5451.c +++ b/sound/pci/ali5451/ali5451.c @@ -855,7 +855,6 @@ static void snd_ali_disable_spdif_out(struct snd_ali *codec) static void snd_ali_update_ptr(struct snd_ali *codec, int channel) { struct snd_ali_voice *pvoice; - struct snd_pcm_runtime *runtime; struct snd_ali_channel_control *pchregs; unsigned int old, mask; #ifdef ALI_DEBUG @@ -872,7 +871,6 @@ static void snd_ali_update_ptr(struct snd_ali *codec, int channel) return;
pvoice = &codec->synth.voices[channel]; - runtime = pvoice->substream->runtime;
udelay(100); spin_lock(&codec->reg_lock);
Just pass the error code returned from copy_from_user_toio() and copy_to_user_fromio() helpers.
Spotted by coverity CID 114119.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/rme96.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/sound/pci/rme96.c b/sound/pci/rme96.c index bb9ebc5543d7..0236363c301f 100644 --- a/sound/pci/rme96.c +++ b/sound/pci/rme96.c @@ -350,9 +350,8 @@ snd_rme96_playback_copy(struct snd_pcm_substream *substream, struct rme96 *rme96 = snd_pcm_substream_chip(substream); count <<= rme96->playback_frlog; pos <<= rme96->playback_frlog; - copy_from_user_toio(rme96->iobase + RME96_IO_PLAY_BUFFER + pos, src, - count); - return 0; + return copy_from_user_toio(rme96->iobase + RME96_IO_PLAY_BUFFER + pos, src, + count); }
static int @@ -365,9 +364,8 @@ snd_rme96_capture_copy(struct snd_pcm_substream *substream, struct rme96 *rme96 = snd_pcm_substream_chip(substream); count <<= rme96->capture_frlog; pos <<= rme96->capture_frlog; - copy_to_user_fromio(dst, rme96->iobase + RME96_IO_REC_BUFFER + pos, - count); - return 0; + return copy_to_user_fromio(dst, rme96->iobase + RME96_IO_REC_BUFFER + pos, + count); }
/*
On 29.10.2013 17:00, Takashi Iwai wrote:
Just pass the error code returned from copy_from_user_toio() and copy_to_user_fromio() helpers.
Spotted by coverity CID 114119.
Signed-off-by: Takashi Iwai tiwai@suse.de
sound/pci/rme96.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/sound/pci/rme96.c b/sound/pci/rme96.c index bb9ebc5543d7..0236363c301f 100644 --- a/sound/pci/rme96.c +++ b/sound/pci/rme96.c @@ -350,9 +350,8 @@ snd_rme96_playback_copy(struct snd_pcm_substream *substream, struct rme96 *rme96 = snd_pcm_substream_chip(substream); count <<= rme96->playback_frlog; pos <<= rme96->playback_frlog;
- copy_from_user_toio(rme96->iobase + RME96_IO_PLAY_BUFFER + pos, src,
count);- return 0;
- return copy_from_user_toio(rme96->iobase + RME96_IO_PLAY_BUFFER + pos, src,
count);
Under which circumstances could copy_from_user_toio() fail?
}
static int @@ -365,9 +364,8 @@ snd_rme96_capture_copy(struct snd_pcm_substream *substream, struct rme96 *rme96 = snd_pcm_substream_chip(substream); count <<= rme96->capture_frlog; pos <<= rme96->capture_frlog;
- copy_to_user_fromio(dst, rme96->iobase + RME96_IO_REC_BUFFER + pos,
count);return 0;
- return copy_to_user_fromio(dst, rme96->iobase + RME96_IO_REC_BUFFER + pos,
count);
When will copy_to_user_fromio() fail?
cu, Knut
}
/*
At Fri, 15 Nov 2013 10:40:07 +0100, Knut Petersen wrote:
On 29.10.2013 17:00, Takashi Iwai wrote:
Just pass the error code returned from copy_from_user_toio() and copy_to_user_fromio() helpers.
Spotted by coverity CID 114119.
Signed-off-by: Takashi Iwai tiwai@suse.de
sound/pci/rme96.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/sound/pci/rme96.c b/sound/pci/rme96.c index bb9ebc5543d7..0236363c301f 100644 --- a/sound/pci/rme96.c +++ b/sound/pci/rme96.c @@ -350,9 +350,8 @@ snd_rme96_playback_copy(struct snd_pcm_substream *substream, struct rme96 *rme96 = snd_pcm_substream_chip(substream); count <<= rme96->playback_frlog; pos <<= rme96->playback_frlog;
- copy_from_user_toio(rme96->iobase + RME96_IO_PLAY_BUFFER + pos, src,
count);- return 0;
- return copy_from_user_toio(rme96->iobase + RME96_IO_PLAY_BUFFER + pos, src,
count);Under which circumstances could copy_from_user_toio() fail?
When copy_from_user() fails.
}
static int @@ -365,9 +364,8 @@ snd_rme96_capture_copy(struct snd_pcm_substream *substream, struct rme96 *rme96 = snd_pcm_substream_chip(substream); count <<= rme96->capture_frlog; pos <<= rme96->capture_frlog;
- copy_to_user_fromio(dst, rme96->iobase + RME96_IO_REC_BUFFER + pos,
count);return 0;
- return copy_to_user_fromio(dst, rme96->iobase + RME96_IO_REC_BUFFER + pos,
count);When will copy_to_user_fromio() fail?
When copy_to_user() fails.
Takashi
The size of the register cache array is actually 6 instead of 7, as it caches up to AK4114_REG_INT1_MASK. This resulted in unexpected access out of array range, although most of them aren't so serious (just reading one more byte on the stack at snd_ak4114_create()).
Also, the check of cache size was wrongly done by checking with sizeof() instead of ARRAY_SIZE(). Fixed this together.
(And yes, hardcoded numbers are bad, but I keep the coding style as is for making it clear what this patch actually does.)
Spotted by coverity among several CIDs, e.g. 711621.
Signed-off-by: Takashi Iwai tiwai@suse.de --- include/sound/ak4114.h | 4 ++-- sound/i2c/other/ak4114.c | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/include/sound/ak4114.h b/include/sound/ak4114.h index 3ce69fd92523..52f02a60dba7 100644 --- a/include/sound/ak4114.h +++ b/include/sound/ak4114.h @@ -170,7 +170,7 @@ struct ak4114 { void * private_data; unsigned int init: 1; spinlock_t lock; - unsigned char regmap[7]; + unsigned char regmap[6]; unsigned char txcsb[5]; struct snd_kcontrol *kctls[AK4114_CONTROLS]; struct snd_pcm_substream *playback_substream; @@ -189,7 +189,7 @@ struct ak4114 {
int snd_ak4114_create(struct snd_card *card, ak4114_read_t *read, ak4114_write_t *write, - const unsigned char pgm[7], const unsigned char txcsb[5], + const unsigned char pgm[6], const unsigned char txcsb[5], void *private_data, struct ak4114 **r_ak4114); void snd_ak4114_reg_write(struct ak4114 *ak4114, unsigned char reg, unsigned char mask, unsigned char val); void snd_ak4114_reinit(struct ak4114 *ak4114); diff --git a/sound/i2c/other/ak4114.c b/sound/i2c/other/ak4114.c index 5bf4fca19e48..15ae0250eace 100644 --- a/sound/i2c/other/ak4114.c +++ b/sound/i2c/other/ak4114.c @@ -60,7 +60,7 @@ static void reg_dump(struct ak4114 *ak4114)
printk(KERN_DEBUG "AK4114 REG DUMP:\n"); for (i = 0; i < 0x20; i++) - printk(KERN_DEBUG "reg[%02x] = %02x (%02x)\n", i, reg_read(ak4114, i), i < sizeof(ak4114->regmap) ? ak4114->regmap[i] : 0); + printk(KERN_DEBUG "reg[%02x] = %02x (%02x)\n", i, reg_read(ak4114, i), i < ARRAY_SIZE(ak4114->regmap) ? ak4114->regmap[i] : 0); } #endif
@@ -81,7 +81,7 @@ static int snd_ak4114_dev_free(struct snd_device *device)
int snd_ak4114_create(struct snd_card *card, ak4114_read_t *read, ak4114_write_t *write, - const unsigned char pgm[7], const unsigned char txcsb[5], + const unsigned char pgm[6], const unsigned char txcsb[5], void *private_data, struct ak4114 **r_ak4114) { struct ak4114 *chip; @@ -101,7 +101,7 @@ int snd_ak4114_create(struct snd_card *card, chip->private_data = private_data; INIT_DELAYED_WORK(&chip->work, ak4114_stats);
- for (reg = 0; reg < 7; reg++) + for (reg = 0; reg < 6; reg++) chip->regmap[reg] = pgm[reg]; for (reg = 0; reg < 5; reg++) chip->txcsb[reg] = txcsb[reg]; @@ -142,7 +142,7 @@ static void ak4114_init_regs(struct ak4114 *chip) /* release reset, but leave powerdown */ reg_write(chip, AK4114_REG_PWRDN, (old | AK4114_RST) & ~AK4114_PWN); udelay(200); - for (reg = 1; reg < 7; reg++) + for (reg = 1; reg < 6; reg++) reg_write(chip, reg, chip->regmap[reg]); for (reg = 0; reg < 5; reg++) reg_write(chip, reg + AK4114_REG_TXCSB0, chip->txcsb[reg]);
Spotted by coverity CIDs 751505 and 751506.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/ice1712/wm8766.c | 3 ++- sound/pci/ice1712/wm8776.c | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/sound/pci/ice1712/wm8766.c b/sound/pci/ice1712/wm8766.c index e473f8a88f9c..21b373b2e260 100644 --- a/sound/pci/ice1712/wm8766.c +++ b/sound/pci/ice1712/wm8766.c @@ -253,7 +253,8 @@ static int snd_wm8766_ctl_get(struct snd_kcontrol *kcontrol, } if (wm->ctl[n].flags & WM8766_FLAG_INVERT) { val1 = wm->ctl[n].max - (val1 - wm->ctl[n].min); - val2 = wm->ctl[n].max - (val2 - wm->ctl[n].min); + if (wm->ctl[n].flags & WM8766_FLAG_STEREO) + val2 = wm->ctl[n].max - (val2 - wm->ctl[n].min); } ucontrol->value.integer.value[0] = val1; if (wm->ctl[n].flags & WM8766_FLAG_STEREO) diff --git a/sound/pci/ice1712/wm8776.c b/sound/pci/ice1712/wm8776.c index 5227cb08247f..e66c0da62014 100644 --- a/sound/pci/ice1712/wm8776.c +++ b/sound/pci/ice1712/wm8776.c @@ -526,7 +526,8 @@ static int snd_wm8776_ctl_get(struct snd_kcontrol *kcontrol, } if (wm->ctl[n].flags & WM8776_FLAG_INVERT) { val1 = wm->ctl[n].max - (val1 - wm->ctl[n].min); - val2 = wm->ctl[n].max - (val2 - wm->ctl[n].min); + if (wm->ctl[n].flags & WM8776_FLAG_STEREO) + val2 = wm->ctl[n].max - (val2 - wm->ctl[n].min); } ucontrol->value.integer.value[0] = val1; if (wm->ctl[n].flags & WM8776_FLAG_STEREO)
The FUNCTION_TYPE parameter isn't associated with any NID, thus showing the uninitialized nid in the error message is simply nonsense.
Spotted by coverity CID 145068.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/lola/lola.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/pci/lola/lola.c b/sound/pci/lola/lola.c index 7307d97186cb..0568540dc8d3 100644 --- a/sound/pci/lola/lola.c +++ b/sound/pci/lola/lola.c @@ -463,7 +463,7 @@ static int lola_parse_tree(struct lola *chip)
err = lola_read_param(chip, 1, LOLA_PAR_FUNCTION_TYPE, &val); if (err < 0) { - printk(KERN_ERR SFX "Can't read FUNCTION_TYPE for 0x%x\n", nid); + printk(KERN_ERR SFX "Can't read FUNCTION_TYPE\n"); return err; } if (val != 1) {
participants (2)
-
Knut Petersen -
Takashi Iwai