Skip to contentSkip to conversation

media: dvb-bt8xx: Fix potential double-free in frontend_init

PATCH Superseded Tuo Li 2 participants 3 messages Nov 10 – Nov 11 Media (V4L/DVB)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Tuo Li

author
#0 lore raw

PATCH 1/1 media: dvb-bt8xx: Fix potential double-free in frontend_init

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.

---
 drivers/media/pci/bt8xx/dst.c | 2 --
 1 file changed, 2 deletions(-)
drivers/media/pci/bt8xx/dst.c +0 -2
index 110651e47831..5cdf8a77260c 100644
@@ -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;
Signature · 1 line
2.43.0
#1 lore raw

…

Quoted 3 lines

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

author
#2 lore raw

Hi Markus,

Quoted 6 lines · Markus Elfring

…

Quoted 3 lines

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.

Quoted 9 lines

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

Thanks for your suggestions. I will update the patch accordingly and send a v2 shortly.

Best regards,
Tuo