Re: [PATCH 48/48] ALSA: 6fire: Use standard printk helpers

[Takashi Iwai <tiwai@suse.de>]

At Wed, 26 Feb 2014 16:53:50 +0100,
Takashi Iwai wrote:
> 
> Convert with dev_err() and co from snd_printk(), etc.
> 
> Signed-off-by: Takashi Iwai <tiwai@suse.de>

There was a use-after-free bug.  The fixed patch is below.


Takashi

-- 8< --
From: Takashi Iwai <tiwai@suse.de>
Subject: [PATCH v2] ALSA: 6fire: Use standard printk helpers

Convert with dev_err() and co from snd_printk(), etc.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/usb/6fire/chip.c     |  8 +++---
 sound/usb/6fire/comm.c     |  4 +--
 sound/usb/6fire/control.c  | 18 +++++++-----
 sound/usb/6fire/firmware.c | 70 ++++++++++++++++++++++++----------------------
 sound/usb/6fire/midi.c     | 12 ++++----
 sound/usb/6fire/pcm.c      | 46 ++++++++++++++++--------------
 6 files changed, 86 insertions(+), 72 deletions(-)

diff --git a/sound/usb/6fire/chip.c b/sound/usb/6fire/chip.c
index e0fe0d92db8c..dcddfc354ba6 100644
--- a/sound/usb/6fire/chip.c
+++ b/sound/usb/6fire/chip.c
@@ -106,7 +106,7 @@ static int usb6fire_chip_probe(struct usb_interface *intf,
 	}
 	if (regidx < 0) {
 		mutex_unlock(&register_mutex);
-		snd_printk(KERN_ERR PREFIX "too many cards registered.\n");
+		dev_err(&intf->dev, "too many cards registered.\n");
 		return -ENODEV;
 	}
 	devices[regidx] = device;
@@ -121,13 +121,13 @@ static int usb6fire_chip_probe(struct usb_interface *intf,
 
 	/* if we are here, card can be registered in alsa. */
 	if (usb_set_interface(device, 0, 0) != 0) {
-		snd_printk(KERN_ERR PREFIX "can't set first interface.\n");
+		dev_err(&intf->dev, "can't set first interface.\n");
 		return -EIO;
 	}
 	ret = snd_card_new(&intf->dev, index[regidx], id[regidx],
 			   THIS_MODULE, sizeof(struct sfire_chip), &card);
 	if (ret < 0) {
-		snd_printk(KERN_ERR PREFIX "cannot create alsa card.\n");
+		dev_err(&intf->dev, "cannot create alsa card.\n");
 		return ret;
 	}
 	strcpy(card->driver, "6FireUSB");
@@ -168,7 +168,7 @@ static int usb6fire_chip_probe(struct usb_interface *intf,
 
 	ret = snd_card_register(card);
 	if (ret < 0) {
-		snd_printk(KERN_ERR PREFIX "cannot register card.");
+		dev_err(&intf->dev, "cannot register card.");
 		usb6fire_chip_destroy(chip);
 		return ret;
 	}
diff --git a/sound/usb/6fire/comm.c b/sound/usb/6fire/comm.c
index 23452ee617e1..161215d78d95 100644
--- a/sound/usb/6fire/comm.c
+++ b/sound/usb/6fire/comm.c
@@ -51,7 +51,7 @@ static void usb6fire_comm_receiver_handler(struct urb *urb)
 		urb->status = 0;
 		urb->actual_length = 0;
 		if (usb_submit_urb(urb, GFP_ATOMIC) < 0)
-			snd_printk(KERN_WARNING PREFIX
+			dev_warn(&urb->dev->dev,
 					"comm data receiver aborted.\n");
 	}
 }
@@ -179,7 +179,7 @@ int usb6fire_comm_init(struct sfire_chip *chip)
 	if (ret < 0) {
 		kfree(rt->receiver_buffer);
 		kfree(rt);
-		snd_printk(KERN_ERR PREFIX "cannot create comm data receiver.");
+		dev_err(&chip->dev->dev, "cannot create comm data receiver.");
 		return ret;
 	}
 	chip->comm = rt;
diff --git a/sound/usb/6fire/control.c b/sound/usb/6fire/control.c
index f6434c245720..184e3987ac24 100644
--- a/sound/usb/6fire/control.c
+++ b/sound/usb/6fire/control.c
@@ -194,7 +194,8 @@ static int usb6fire_control_output_vol_put(struct snd_kcontrol *kcontrol,
 	int changed = 0;
 
 	if (ch > 4) {
-		snd_printk(KERN_ERR PREFIX "Invalid channel in volume control.");
+		dev_err(&rt->chip->dev->dev,
+			"Invalid channel in volume control.");
 		return -EINVAL;
 	}
 
@@ -222,7 +223,8 @@ static int usb6fire_control_output_vol_get(struct snd_kcontrol *kcontrol,
 	unsigned int ch = kcontrol->private_value;
 
 	if (ch > 4) {
-		snd_printk(KERN_ERR PREFIX "Invalid channel in volume control.");
+		dev_err(&rt->chip->dev->dev,
+			"Invalid channel in volume control.");
 		return -EINVAL;
 	}
 
@@ -240,7 +242,8 @@ static int usb6fire_control_output_mute_put(struct snd_kcontrol *kcontrol,
 	u8 value = 0;
 
 	if (ch > 4) {
-		snd_printk(KERN_ERR PREFIX "Invalid channel in volume control.");
+		dev_err(&rt->chip->dev->dev,
+			"Invalid channel in volume control.");
 		return -EINVAL;
 	}
 
@@ -265,7 +268,8 @@ static int usb6fire_control_output_mute_get(struct snd_kcontrol *kcontrol,
 	u8 value = rt->output_mute >> ch;
 
 	if (ch > 4) {
-		snd_printk(KERN_ERR PREFIX "Invalid channel in volume control.");
+		dev_err(&rt->chip->dev->dev,
+			"Invalid channel in volume control.");
 		return -EINVAL;
 	}
 
@@ -594,14 +598,14 @@ int usb6fire_control_init(struct sfire_chip *chip)
 	ret = usb6fire_control_add_virtual(rt, chip->card,
 		"Master Playback Volume", vol_elements);
 	if (ret) {
-		snd_printk(KERN_ERR PREFIX "cannot add control.\n");
+		dev_err(&chip->dev->dev, "cannot add control.\n");
 		kfree(rt);
 		return ret;
 	}
 	ret = usb6fire_control_add_virtual(rt, chip->card,
 		"Master Playback Switch", mute_elements);
 	if (ret) {
-		snd_printk(KERN_ERR PREFIX "cannot add control.\n");
+		dev_err(&chip->dev->dev, "cannot add control.\n");
 		kfree(rt);
 		return ret;
 	}
@@ -611,7 +615,7 @@ int usb6fire_control_init(struct sfire_chip *chip)
 		ret = snd_ctl_add(chip->card, snd_ctl_new1(&elements[i], rt));
 		if (ret < 0) {
 			kfree(rt);
-			snd_printk(KERN_ERR PREFIX "cannot add control.\n");
+			dev_err(&chip->dev->dev, "cannot add control.\n");
 			return ret;
 		}
 		i++;
diff --git a/sound/usb/6fire/firmware.c b/sound/usb/6fire/firmware.c
index 780bf3f62d28..3b02e54b8f6d 100644
--- a/sound/usb/6fire/firmware.c
+++ b/sound/usb/6fire/firmware.c
@@ -219,16 +219,16 @@ static int usb6fire_fw_ezusb_upload(
 	ret = request_firmware(&fw, fwname, &device->dev);
 	if (ret < 0) {
 		kfree(rec);
-		snd_printk(KERN_ERR PREFIX "error requesting ezusb "
-				"firmware %s.\n", fwname);
+		dev_err(&intf->dev,
+			"error requesting ezusb firmware %s.\n", fwname);
 		return ret;
 	}
 	ret = usb6fire_fw_ihex_init(fw, rec);
 	if (ret < 0) {
 		kfree(rec);
 		release_firmware(fw);
-		snd_printk(KERN_ERR PREFIX "error validating ezusb "
-				"firmware %s.\n", fwname);
+		dev_err(&intf->dev,
+			"error validating ezusb firmware %s.\n", fwname);
 		return ret;
 	}
 	/* upload firmware image */
@@ -237,8 +237,9 @@ static int usb6fire_fw_ezusb_upload(
 	if (ret < 0) {
 		kfree(rec);
 		release_firmware(fw);
-		snd_printk(KERN_ERR PREFIX "unable to upload ezusb "
-				"firmware %s: begin message.\n", fwname);
+		dev_err(&intf->dev,
+			"unable to upload ezusb firmware %s: begin message.\n",
+			fwname);
 		return ret;
 	}
 
@@ -248,8 +249,9 @@ static int usb6fire_fw_ezusb_upload(
 		if (ret < 0) {
 			kfree(rec);
 			release_firmware(fw);
-			snd_printk(KERN_ERR PREFIX "unable to upload ezusb "
-					"firmware %s: data urb.\n", fwname);
+			dev_err(&intf->dev,
+				"unable to upload ezusb firmware %s: data urb.\n",
+				fwname);
 			return ret;
 		}
 	}
@@ -260,8 +262,9 @@ static int usb6fire_fw_ezusb_upload(
 		ret = usb6fire_fw_ezusb_write(device, 0xa0, postaddr,
 				postdata, postlen);
 		if (ret < 0) {
-			snd_printk(KERN_ERR PREFIX "unable to upload ezusb "
-					"firmware %s: post urb.\n", fwname);
+			dev_err(&intf->dev,
+				"unable to upload ezusb firmware %s: post urb.\n",
+				fwname);
 			return ret;
 		}
 	}
@@ -269,8 +272,9 @@ static int usb6fire_fw_ezusb_upload(
 	data = 0x00; /* resume ezusb cpu */
 	ret = usb6fire_fw_ezusb_write(device, 0xa0, 0xe600, &data, 1);
 	if (ret < 0) {
-		snd_printk(KERN_ERR PREFIX "unable to upload ezusb "
-				"firmware %s: end message.\n", fwname);
+		dev_err(&intf->dev,
+			"unable to upload ezusb firmware %s: end message.\n",
+			fwname);
 		return ret;
 	}
 	return 0;
@@ -292,7 +296,7 @@ static int usb6fire_fw_fpga_upload(
 
 	ret = request_firmware(&fw, fwname, &device->dev);
 	if (ret < 0) {
-		snd_printk(KERN_ERR PREFIX "unable to get fpga firmware %s.\n",
+		dev_err(&intf->dev, "unable to get fpga firmware %s.\n",
 				fwname);
 		kfree(buffer);
 		return -EIO;
@@ -305,8 +309,8 @@ static int usb6fire_fw_fpga_upload(
 	if (ret < 0) {
 		kfree(buffer);
 		release_firmware(fw);
-		snd_printk(KERN_ERR PREFIX "unable to upload fpga firmware: "
-				"begin urb.\n");
+		dev_err(&intf->dev,
+			"unable to upload fpga firmware: begin urb.\n");
 		return ret;
 	}
 
@@ -318,8 +322,8 @@ static int usb6fire_fw_fpga_upload(
 		if (ret < 0) {
 			release_firmware(fw);
 			kfree(buffer);
-			snd_printk(KERN_ERR PREFIX "unable to upload fpga "
-					"firmware: fw urb.\n");
+			dev_err(&intf->dev,
+				"unable to upload fpga firmware: fw urb.\n");
 			return ret;
 		}
 	}
@@ -328,8 +332,8 @@ static int usb6fire_fw_fpga_upload(
 
 	ret = usb6fire_fw_ezusb_write(device, 9, 0, NULL, 0);
 	if (ret < 0) {
-		snd_printk(KERN_ERR PREFIX "unable to upload fpga firmware: "
-				"end urb.\n");
+		dev_err(&intf->dev,
+			"unable to upload fpga firmware: end urb.\n");
 		return ret;
 	}
 	return 0;
@@ -338,7 +342,7 @@ static int usb6fire_fw_fpga_upload(
 /* check, if the firmware version the devices has currently loaded
  * is known by this driver. 'version' needs to have 4 bytes version
  * info data. */
-static int usb6fire_fw_check(u8 *version)
+static int usb6fire_fw_check(struct usb_interface *intf, const u8 *version)
 {
 	int i;
 
@@ -346,7 +350,7 @@ static int usb6fire_fw_check(u8 *version)
 		if (!memcmp(version, known_fw_versions + i, 2))
 			return 0;
 
-	snd_printk(KERN_ERR PREFIX "invalid fimware version in device: %4ph. "
+	dev_err(&intf->dev, "invalid fimware version in device: %4ph. "
 			"please reconnect to power. if this failure "
 			"still happens, check your firmware installation.",
 			version);
@@ -364,16 +368,16 @@ int usb6fire_fw_init(struct usb_interface *intf)
 
 	ret = usb6fire_fw_ezusb_read(device, 1, 0, buffer, 8);
 	if (ret < 0) {
-		snd_printk(KERN_ERR PREFIX "unable to receive device "
-				"firmware state.\n");
+		dev_err(&intf->dev,
+			"unable to receive device firmware state.\n");
 		return ret;
 	}
 	if (buffer[0] != 0xeb || buffer[1] != 0xaa || buffer[2] != 0x55) {
-		snd_printk(KERN_ERR PREFIX "unknown device firmware state "
-				"received from device: ");
+		dev_err(&intf->dev,
+			"unknown device firmware state received from device:");
 		for (i = 0; i < 8; i++)
-			snd_printk("%02x ", buffer[i]);
-		snd_printk("\n");
+			printk(KERN_CONT "%02x ", buffer[i]);
+		printk(KERN_CONT "\n");
 		return -EIO;
 	}
 	/* do we need fpga loader ezusb firmware? */
@@ -386,7 +390,7 @@ int usb6fire_fw_init(struct usb_interface *intf)
 	}
 	/* do we need fpga firmware and application ezusb firmware? */
 	else if (buffer[3] == 0x02) {
-		ret = usb6fire_fw_check(buffer + 4);
+		ret = usb6fire_fw_check(intf, buffer + 4);
 		if (ret < 0)
 			return ret;
 		ret = usb6fire_fw_fpga_upload(intf, "6fire/dmx6firecf.bin");
@@ -402,14 +406,14 @@ int usb6fire_fw_init(struct usb_interface *intf)
 	}
 	/* all fw loaded? */
 	else if (buffer[3] == 0x03)
-		return usb6fire_fw_check(buffer + 4);
+		return usb6fire_fw_check(intf, buffer + 4);
 	/* unknown data? */
 	else {
-		snd_printk(KERN_ERR PREFIX "unknown device firmware state "
-				"received from device: ");
+		dev_err(&intf->dev,
+			"unknown device firmware state received from device: ");
 		for (i = 0; i < 8; i++)
-			snd_printk("%02x ", buffer[i]);
-		snd_printk("\n");
+			printk(KERN_CONT "%02x ", buffer[i]);
+		printk(KERN_CONT "\n");
 		return -EIO;
 	}
 	return 0;
diff --git a/sound/usb/6fire/midi.c b/sound/usb/6fire/midi.c
index f3dd7266c391..3d410969553e 100644
--- a/sound/usb/6fire/midi.c
+++ b/sound/usb/6fire/midi.c
@@ -41,8 +41,9 @@ static void usb6fire_midi_out_handler(struct urb *urb)
 
 			ret = usb_submit_urb(urb, GFP_ATOMIC);
 			if (ret < 0)
-				snd_printk(KERN_ERR PREFIX "midi out urb "
-						"submit failed: %d\n", ret);
+				dev_err(&urb->dev->dev,
+					"midi out urb submit failed: %d\n",
+					ret);
 		} else /* no more data to transmit */
 			rt->out = NULL;
 	}
@@ -94,8 +95,9 @@ static void usb6fire_midi_out_trigger(
 
 			ret = usb_submit_urb(urb, GFP_ATOMIC);
 			if (ret < 0)
-				snd_printk(KERN_ERR PREFIX "midi out urb "
-						"submit failed: %d\n", ret);
+				dev_err(&urb->dev->dev,
+					"midi out urb submit failed: %d\n",
+					ret);
 			else
 				rt->out = alsa_sub;
 		}
@@ -181,7 +183,7 @@ int usb6fire_midi_init(struct sfire_chip *chip)
 	if (ret < 0) {
 		kfree(rt->out_buffer);
 		kfree(rt);
-		snd_printk(KERN_ERR PREFIX "unable to create midi.\n");
+		dev_err(&chip->dev->dev, "unable to create midi.\n");
 		return ret;
 	}
 	rt->instance->private_data = rt;
diff --git a/sound/usb/6fire/pcm.c b/sound/usb/6fire/pcm.c
index b5eb97fdc842..ba40489b2de4 100644
--- a/sound/usb/6fire/pcm.c
+++ b/sound/usb/6fire/pcm.c
@@ -79,32 +79,35 @@ static int usb6fire_pcm_set_rate(struct pcm_runtime *rt)
 	ctrl_rt->usb_streaming = false;
 	ret = ctrl_rt->update_streaming(ctrl_rt);
 	if (ret < 0) {
-		snd_printk(KERN_ERR PREFIX "error stopping streaming while "
-				"setting samplerate %d.\n", rates[rt->rate]);
+		dev_err(&rt->chip->dev->dev,
+			"error stopping streaming while setting samplerate %d.\n",
+			rates[rt->rate]);
 		return ret;
 	}
 
 	ret = ctrl_rt->set_rate(ctrl_rt, rt->rate);
 	if (ret < 0) {
-		snd_printk(KERN_ERR PREFIX "error setting samplerate %d.\n",
-				rates[rt->rate]);
+		dev_err(&rt->chip->dev->dev,
+			"error setting samplerate %d.\n",
+			rates[rt->rate]);
 		return ret;
 	}
 
 	ret = ctrl_rt->set_channels(ctrl_rt, OUT_N_CHANNELS, IN_N_CHANNELS,
 			false, false);
 	if (ret < 0) {
-		snd_printk(KERN_ERR PREFIX "error initializing channels "
-				"while setting samplerate %d.\n",
-				rates[rt->rate]);
+		dev_err(&rt->chip->dev->dev,
+			"error initializing channels while setting samplerate %d.\n",
+			rates[rt->rate]);
 		return ret;
 	}
 
 	ctrl_rt->usb_streaming = true;
 	ret = ctrl_rt->update_streaming(ctrl_rt);
 	if (ret < 0) {
-		snd_printk(KERN_ERR PREFIX "error starting streaming while "
-				"setting samplerate %d.\n", rates[rt->rate]);
+		dev_err(&rt->chip->dev->dev,
+			"error starting streaming while setting samplerate %d.\n",
+			rates[rt->rate]);
 		return ret;
 	}
 
@@ -124,7 +127,7 @@ static struct pcm_substream *usb6fire_pcm_get_substream(
 		return &rt->playback;
 	else if (alsa_sub->stream == SNDRV_PCM_STREAM_CAPTURE)
 		return &rt->capture;
-	snd_printk(KERN_ERR PREFIX "error getting pcm substream slot.\n");
+	dev_err(&rt->chip->dev->dev, "error getting pcm substream slot.\n");
 	return NULL;
 }
 
@@ -257,7 +260,7 @@ static void usb6fire_pcm_playback(struct pcm_substream *sub,
 	else if (alsa_rt->format == SNDRV_PCM_FORMAT_S24_LE)
 		dest = (u32 *) (urb->buffer);
 	else {
-		snd_printk(KERN_ERR PREFIX "Unknown sample format.");
+		dev_err(&rt->chip->dev->dev, "Unknown sample format.");
 		return;
 	}
 
@@ -307,8 +310,8 @@ static void usb6fire_pcm_in_urb_handler(struct urb *usb_urb)
 		}
 
 	if (rt->stream_state == STREAM_DISABLED) {
-		snd_printk(KERN_ERR PREFIX "internal error: "
-				"stream disabled in in-urb handler.\n");
+		dev_err(&rt->chip->dev->dev,
+			"internal error: stream disabled in in-urb handler.\n");
 		return;
 	}
 
@@ -410,7 +413,7 @@ static int usb6fire_pcm_open(struct snd_pcm_substream *alsa_sub)
 
 	if (!sub) {
 		mutex_unlock(&rt->stream_mutex);
-		snd_printk(KERN_ERR PREFIX "invalid stream type.\n");
+		dev_err(&rt->chip->dev->dev, "invalid stream type.\n");
 		return -EINVAL;
 	}
 
@@ -481,8 +484,9 @@ static int usb6fire_pcm_prepare(struct snd_pcm_substream *alsa_sub)
 				break;
 		if (rt->rate == ARRAY_SIZE(rates)) {
 			mutex_unlock(&rt->stream_mutex);
-			snd_printk("invalid rate %d in prepare.\n",
-					alsa_rt->rate);
+			dev_err(&rt->chip->dev->dev,
+				"invalid rate %d in prepare.\n",
+				alsa_rt->rate);
 			return -EINVAL;
 		}
 
@@ -494,8 +498,8 @@ static int usb6fire_pcm_prepare(struct snd_pcm_substream *alsa_sub)
 		ret = usb6fire_pcm_stream_start(rt);
 		if (ret) {
 			mutex_unlock(&rt->stream_mutex);
-			snd_printk(KERN_ERR PREFIX
-					"could not start pcm stream.\n");
+			dev_err(&rt->chip->dev->dev,
+				"could not start pcm stream.\n");
 			return ret;
 		}
 	}
@@ -650,7 +654,7 @@ int usb6fire_pcm_init(struct sfire_chip *chip)
 	if (ret < 0) {
 		usb6fire_pcm_buffers_destroy(rt);
 		kfree(rt);
-		snd_printk(KERN_ERR PREFIX "cannot create pcm instance.\n");
+		dev_err(&chip->dev->dev, "cannot create pcm instance.\n");
 		return ret;
 	}
 
@@ -662,8 +666,8 @@ int usb6fire_pcm_init(struct sfire_chip *chip)
 	if (ret) {
 		usb6fire_pcm_buffers_destroy(rt);
 		kfree(rt);
-		snd_printk(KERN_ERR PREFIX
-				"error preallocating pcm buffers.\n");
+		dev_err(&chip->dev->dev,
+			"error preallocating pcm buffers.\n");
 		return ret;
 	}
 	rt->instance = pcm;
-- 
1.9.0