Commit 024e01de authored by Hans Verkuil's avatar Hans Verkuil Committed by Mauro Carvalho Chehab
Browse files

media: pulse8-cec: fix duplicate free at disconnect or probe error 



Commit 601282d6 ("media: pulse8-cec: use adap_free callback") used
the adap_free callback to clean up on disconnect. What I forgot was that
in the probe it will call cec_delete_adapter() followed by kfree(pulse8)
if an error occurs. But by using the adap_free callback,
cec_delete_adapter() is already freeing the pulse8 struct.

This wasn't noticed since normally the probe works fine, but Pulse-Eight
published a new firmware version that caused a probe error, so now it
hits this bug. This affects firmware version 12, but probably any
version >= 10.

Commit aa9eda76 ("media: pulse8-cec: close serio in disconnect, not
adap_free") made this worse by adding the line 'pulse8->serio = NULL'
right after the call to cec_unregister_adapter in the disconnect()
function. Unfortunately, cec_unregister_adapter will typically call
cec_delete_adapter (unless a filehandle to the cec device is still
open), which frees the pulse8 struct. So now it will also crash on a
simple unplug of the Pulse-Eight device.

With this fix both the unplug issue and a probe() error situation are
handled correctly again.

It will still fail to probe() with a v12 firmware, that's something
to look at separately.

Signed-off-by: default avatarHans Verkuil <hverkuil-cisco@xs4all.nl>
Reported-by: default avatarMaxime Ripard <mripard@kernel.org>
Tested-by: default avatarMaxime Ripard <mripard@kernel.org>
Fixes: aa9eda76 ("media: pulse8-cec: close serio in disconnect, not adap_free")
Fixes: 601282d6 ("media: pulse8-cec: use adap_free callback")
Cc: <stable@vger.kernel.org>
Signed-off-by: default avatarMauro Carvalho Chehab <mchehab+huawei@kernel.org>
parent d904eb0b

drivers/media/cec/usb/pulse8/pulse8-cec.c

+4 −5
Original line number Diff line number Diff line
@@ -650,7 +650,6 @@ static void pulse8_disconnect(struct serio *serio) 
	struct pulse8 *pulse8 = serio_get_drvdata(serio);



	cec_unregister_adapter(pulse8->adap);

	pulse8->serio = NULL;

	serio_set_drvdata(serio, NULL);

	serio_close(serio);

}
@@ -830,8 +829,10 @@ static int pulse8_connect(struct serio *serio, struct serio_driver *drv) 
	pulse8->adap = cec_allocate_adapter(&pulse8_cec_adap_ops, pulse8,

					    dev_name(&serio->dev), caps, 1);

	err = PTR_ERR_OR_ZERO(pulse8->adap);

	if (err < 0)

		goto free_device;

	if (err < 0) {

		kfree(pulse8);

		return err;

	}



	pulse8->dev = &serio->dev;

	serio_set_drvdata(serio, pulse8);
@@ -874,8 +875,6 @@ static int pulse8_connect(struct serio *serio, struct serio_driver *drv) 
	serio_close(serio);

delete_adap:

	cec_delete_adapter(pulse8->adap);

free_device:

	kfree(pulse8);

	return err;

}