[PATCH] soc: aspeed: deregister a misc device on error
Joe Hattori
joe at pf.is.s.u-tokyo.ac.jp
Wed Dec 11 14:52:30 AEDT 2024
Hi Andrew,
Thank you for your review.
On 12/9/24 09:28, Andrew Jeffery wrote:
> Hi Joe,
>
> On Sat, 2024-12-07 at 12:25 +0900, Joe Hattori wrote:
>> The error path of aspeed_lpc_enable_snoop() does not deregister the
>> misc
>> device, which results in a memory leak. Therefore, add a
>> misc_deregister() call in the error path.
>>
>> Fixes: 524feb799408 ("soc: add aspeed folder and misc drivers")
>> Signed-off-by: Joe Hattori <joe at pf.is.s.u-tokyo.ac.jp>
>> ---
>> drivers/soc/aspeed/aspeed-lpc-snoop.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/soc/aspeed/aspeed-lpc-snoop.c
>> b/drivers/soc/aspeed/aspeed-lpc-snoop.c
>> index 9ab5ba9cf1d6..083ddf6dcb7a 100644
>> --- a/drivers/soc/aspeed/aspeed-lpc-snoop.c
>> +++ b/drivers/soc/aspeed/aspeed-lpc-snoop.c
>> @@ -221,6 +221,7 @@ static int aspeed_lpc_enable_snoop(struct
>> aspeed_lpc_snoop *lpc_snoop,
>> hicrb_en = HICRB_ENSNP1D;
>> break;
>> default:
>> + misc_deregister(&lpc_snoop->chan[channel].miscdev);
>
> We should free the kfifo too.
Yes, fixed in the V2 patch.
>
> Anyway, all the switch statement is doing is setting up mask metadata,
> the non-default cases don't depend on the acquired resources. I think
> it would make more sense to move it prior to any resource acquisition,
> rather than try to unwind their acquisition in the default case.
Definitely, it only depends on the function arg `channel`. Modified in
the v2 patch as well.
>
> Andrew
Best,
Joe
More information about the Linux-aspeed
mailing list