[PATCH] media: dvb-bt8xx: Fix potential double-free in frontend_init

[PATCH] media: dvb-bt8xx: Fix potential double-free in frontend_init

[Tuo Li]

The caller in dvb-bt8xx.c (frontend_init) already frees the allocated
dst_state when dvb_attach(dst_attach, ...) returns NULL:

  if (dvb_attach(dst_attach, state, &card->dvb_adapter) == NULL) { 
    pr_err("%s: Could not find a Twinhan DST\n", __func__);
	kfree(state);
	break;
  }

Remove the redundant kfree() calls from dst_attach() to avoid
double-freeing the state on failure paths. This keeps ownership with the
caller until a successful attachment transfers it to the frontend.

Signed-off-by: Tuo Li <islituo@gmail.com>
---
 drivers/media/pci/bt8xx/dst.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/media/pci/bt8xx/dst.c b/drivers/media/pci/bt8xx/dst.c
index 110651e47831..5cdf8a77260c 100644
--- a/drivers/media/pci/bt8xx/dst.c
+++ b/drivers/media/pci/bt8xx/dst.c
@@ -1694,7 +1694,6 @@ struct dst_state *dst_attach(struct dst_state *state, struct dvb_adapter *dvb_ad
 {
 	/* check if the ASIC is there */
 	if (dst_probe(state) < 0) {
-		kfree(state);
 		return NULL;
 	}
 	/* determine settings based on type */
@@ -1714,7 +1713,6 @@ struct dst_state *dst_attach(struct dst_state *state, struct dvb_adapter *dvb_ad
 		break;
 	default:
 		pr_err("unknown DST type. please report to the LinuxTV.org DVB mailinglist.\n");
-		kfree(state);
 		return NULL;
 	}
 	state->frontend.demodulator_priv = state;
-- 
2.43.0

Re: [PATCH] media: dvb-bt8xx: Fix potential double-free in frontend_init

[Markus Elfring]

…
> Remove the redundant kfree() calls from dst_attach() to avoid
> double-freeing the state on failure paths. This keeps ownership with the
> caller until a successful attachment transfers it to the frontend.

How do you think about to add any tags (like “Fixes” and “Cc”) accordingly?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v7.4-rc2#n145
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/stable-kernel-rules.rst?h=v7.4-rc2#n34


Would the specification “dst_attach()” be more appropriate in the summary phrase
instead of “frontend_init”?
https://elixir.bootlin.com/linux/v7.3.1/source/drivers/media/pci/bt8xx/dst.c#L1693-L1723

Regards,
Markus