Re: [PATCH] usb: host: sl811_cs: fix memory leak on probe failure

[Nikolay Kulikov]

On Mon, Aug 31, 2026 at 02:11:16PM +0800, Zongmin Zhou wrote:
> 
> 在 2026/8/29 01:36, Nikolay Kulikov 写道:
> > On Thu, Aug 27, 2026 at 03:41:20PM +0800, Zongmin Zhou wrote:
> > > From: Zongmin Zhou <zhouzongmin@xxxxxxxxxx>
> > > 
> > > sl811_cs_probe() leaks the local_info_t allocated into link->priv when
> > > sl811_cs_config() fails: the only kfree() lives in the remove callback
> > > sl811_cs_detach(), which the PCMCIA core never calls for a device whose
> > > probe failed.
> > > 
> > > Free the private data when sl811_cs_config() fails.
> > > 
> > > Fixes: c6de2b64eb57 ("[PATCH] USB: add sl811_cs support")
> > The code looks good to me, but I have a question regarding the Fixes
> > tag.
> > 
> > In the commit you cited, a failure triggered a call to
> > sl811_cs_detach(), which freed that memory. However, that behavior was
> > changed in commit
> > 
> > f8cfa618dccb ("[PATCH] pcmcia: unify attach, EVENT_CARD_INSERTION handlers into one probe callback")
> > 
> > where the call was removed, leaving the memory unfreed. Shouldn't that
> > be the commit referenced in the Fixes?
> Hi Nikolay,
> 
> You're right that c6de2b64eb57 did not introduce the leak -- but neither did
> f8cfa618dccb.
> The leak was introduced by 15b99ac17295 ("[PATCH] pcmcia: add return value
> to _config() functions")
> 
> f8cfa618dccb dropped the detach-on-failure path because registration
> moved into the core, and its probe ended with
>     sl811_cs_config(link);
>     return 0;
> A config failure was never propagated, so probe() always succeeded
> after the allocation and it remained paired with the kfree() in
> remove(). No leak either.
> 
> 15b99ac17295 made sl811_cs_config() return -ENODEV and the probe return
> that value -- the first time probe could fail after the allocation.
> Since the core never calls ->remove() for a failed probe, link->priv
> leaked from then on.
> 
> If you agree, I'll send a v2 with
>     Fixes: 15b99ac17295 ("[PATCH] pcmcia: add return value to _config()
> functions")

Yes, it really should be 15b99ac17295.
In v2, you can add my

Reviewed-by: Nikolay Kulikov <nikolayof23@xxxxxxxxx>

> 
> Thanks,
> Zongmin
> > 
> > 
> > Thanks,
> > Nikolay
> 
>