[2/2] media: siano: Fix the missing err path in smsdvb_debugfs_create()

Message ID 20230914035035.3765754-3-ruanjinjie@huawei.com (mailing list archive)
State Changes Requested
Delegated to: Hans Verkuil
Headers
Series media: siano: Fix two bugs in smsdvb_debugfs_create() |

Commit Message

Jinjie Ruan Sept. 14, 2023, 3:50 a.m. UTC
  If kzalloc() fails in smsdvb_debugfs_create(), the dir and file which
is created by debugfs_create_dir() and debugfs_create_file() is
not freed. So use debugfs_remove_recursive() to free them.

Fixes: 503efe5cfc9f ("[media] siano: split debugfs code into a separate file")
Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
---
 drivers/media/common/siano/smsdvb-debugfs.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
  

Comments

Hans Verkuil Oct. 5, 2023, 8:30 a.m. UTC | #1
On 14/09/2023 05:50, Jinjie Ruan wrote:
> If kzalloc() fails in smsdvb_debugfs_create(), the dir and file which
> is created by debugfs_create_dir() and debugfs_create_file() is
> not freed. So use debugfs_remove_recursive() to free them.
> 
> Fixes: 503efe5cfc9f ("[media] siano: split debugfs code into a separate file")
> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
> ---
>  drivers/media/common/siano/smsdvb-debugfs.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/common/siano/smsdvb-debugfs.c b/drivers/media/common/siano/smsdvb-debugfs.c
> index 16d3b9ab31c5..38b25e88ce57 100644
> --- a/drivers/media/common/siano/smsdvb-debugfs.c
> +++ b/drivers/media/common/siano/smsdvb-debugfs.c
> @@ -375,8 +375,10 @@ int smsdvb_debugfs_create(struct smsdvb_client_t *client)
>  	}
>  
>  	debug_data = kzalloc(sizeof(*client->debug_data), GFP_KERNEL);
> -	if (!debug_data)
> +	if (!debug_data) {
> +		debugfs_remove_recursive(client->debugfs);
>  		return -ENOMEM;
> +	}
>  
>  	client->debug_data        = debug_data;
>  	client->prt_dvb_stats     = smsdvb_print_dvb_stats;

It's much better to first allocate debug_data before calling debugfs_create_dir.

No need to clean anything up in that case.

You can also ignore any errors from debugfs_create_file.

Regards,

	Hans
  
Jinjie Ruan Oct. 7, 2023, 11:10 a.m. UTC | #2
On 2023/10/5 16:30, Hans Verkuil wrote:
> On 14/09/2023 05:50, Jinjie Ruan wrote:
>> If kzalloc() fails in smsdvb_debugfs_create(), the dir and file which
>> is created by debugfs_create_dir() and debugfs_create_file() is
>> not freed. So use debugfs_remove_recursive() to free them.
>>
>> Fixes: 503efe5cfc9f ("[media] siano: split debugfs code into a separate file")
>> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
>> ---
>>  drivers/media/common/siano/smsdvb-debugfs.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/media/common/siano/smsdvb-debugfs.c b/drivers/media/common/siano/smsdvb-debugfs.c
>> index 16d3b9ab31c5..38b25e88ce57 100644
>> --- a/drivers/media/common/siano/smsdvb-debugfs.c
>> +++ b/drivers/media/common/siano/smsdvb-debugfs.c
>> @@ -375,8 +375,10 @@ int smsdvb_debugfs_create(struct smsdvb_client_t *client)
>>  	}
>>  
>>  	debug_data = kzalloc(sizeof(*client->debug_data), GFP_KERNEL);
>> -	if (!debug_data)
>> +	if (!debug_data) {
>> +		debugfs_remove_recursive(client->debugfs);
>>  		return -ENOMEM;
>> +	}
>>  
>>  	client->debug_data        = debug_data;
>>  	client->prt_dvb_stats     = smsdvb_print_dvb_stats;
> 
> It's much better to first allocate debug_data before calling debugfs_create_dir.

Right! After that the code will be more cleaner.

> 
> No need to clean anything up in that case.
> 
> You can also ignore any errors from debugfs_create_file.
> 
> Regards,
> 
> 	Hans
>
  

Patch

diff --git a/drivers/media/common/siano/smsdvb-debugfs.c b/drivers/media/common/siano/smsdvb-debugfs.c
index 16d3b9ab31c5..38b25e88ce57 100644
--- a/drivers/media/common/siano/smsdvb-debugfs.c
+++ b/drivers/media/common/siano/smsdvb-debugfs.c
@@ -375,8 +375,10 @@  int smsdvb_debugfs_create(struct smsdvb_client_t *client)
 	}
 
 	debug_data = kzalloc(sizeof(*client->debug_data), GFP_KERNEL);
-	if (!debug_data)
+	if (!debug_data) {
+		debugfs_remove_recursive(client->debugfs);
 		return -ENOMEM;
+	}
 
 	client->debug_data        = debug_data;
 	client->prt_dvb_stats     = smsdvb_print_dvb_stats;