This is an automatic generated email to let you know that the following patch were queued:
Subject: media: pulse8-cec: fix duplicate free at disconnect or probe error
Author: Hans Verkuil <hverkuil-cisco(a)xs4all.nl>
Date: Fri Nov 27 10:36:32 2020 +0100
Commit 601282d65b96 ("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 aa9eda76129c ("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: Hans Verkuil <hverkuil-cisco(a)xs4all.nl>
Reported-by: Maxime Ripard <mripard(a)kernel.org>
Tested-by: Maxime Ripard <mripard(a)kernel.org>
Fixes: aa9eda76129c ("media: pulse8-cec: close serio in disconnect, not adap_free")
Fixes: 601282d65b96 ("media: pulse8-cec: use adap_free callback")
Cc: <stable(a)vger.kernel.org>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei(a)kernel.org>
drivers/media/cec/usb/pulse8/pulse8-cec.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
---
diff --git a/drivers/media/cec/usb/pulse8/pulse8-cec.c b/drivers/media/cec/usb/pulse8/pulse8-cec.c
index e4d8446b87da..5d3a3f775bc8 100644
--- a/drivers/media/cec/usb/pulse8/pulse8-cec.c
+++ b/drivers/media/cec/usb/pulse8/pulse8-cec.c
@@ -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 @@ close_serio:
serio_close(serio);
delete_adap:
cec_delete_adapter(pulse8->adap);
-free_device:
- kfree(pulse8);
return err;
}
This is an automatic generated email to let you know that the following patch were queued:
Subject: media: pulse8-cec: fix duplicate free at disconnect or probe error
Author: Hans Verkuil <hverkuil-cisco(a)xs4all.nl>
Date: Fri Nov 27 10:36:32 2020 +0100
Commit 601282d65b96 ("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 aa9eda76129c ("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: Hans Verkuil <hverkuil-cisco(a)xs4all.nl>
Reported-by: Maxime Ripard <mripard(a)kernel.org>
Tested-by: Maxime Ripard <mripard(a)kernel.org>
Fixes: aa9eda76129c ("media: pulse8-cec: close serio in disconnect, not adap_free")
Fixes: 601282d65b96 ("media: pulse8-cec: use adap_free callback")
Cc: <stable(a)vger.kernel.org>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei(a)kernel.org>
drivers/media/cec/usb/pulse8/pulse8-cec.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
---
diff --git a/drivers/media/cec/usb/pulse8/pulse8-cec.c b/drivers/media/cec/usb/pulse8/pulse8-cec.c
index e4d8446b87da..5d3a3f775bc8 100644
--- a/drivers/media/cec/usb/pulse8/pulse8-cec.c
+++ b/drivers/media/cec/usb/pulse8/pulse8-cec.c
@@ -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 @@ close_serio:
serio_close(serio);
delete_adap:
cec_delete_adapter(pulse8->adap);
-free_device:
- kfree(pulse8);
return err;
}
On Tue, Dec 01, 2020 at 08:04:51AM +0800, dinghua ma wrote:
> I feel very sorry, I found the subject of the email was wrong at the
> moment I sent the email, which wasted everyone’s reading time. I have
> already sent the third email and I am sorry again
I don't appear to have any other mail from you recently and can't look
it up with lore either so I think perhaps it got stopped by some filter
somewhere in your mail system and you've not actually been disrupting
anything, at least for me - can you resend please?