linux-kernel - [PATCH] scsi: aacraid: Fix memory leak in open_getadapter

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240903203121.5953-1-riyandhiman14@gmail.com>
Date: Wed,  4 Sep 2024 02:00:13 +0530
From: Riyan Dhiman <riyandhiman14@...il.com>
To: bvanassche@....org,
	aacraid@...rosemi.com,
	James.Bottomley@...senPartnership.com,
	martin.petersen@...cle.com
Cc: linux-scsi@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: [PATCH] scsi: aacraid: Fix memory leak in open_getadapter_fib function

>> This patch ensures that the allocated memory for fibctx is properly
>> freed if copy_to_user() fails, thereby preventing potential memory leaks.
>
> What made you analyze the code modified by this patch?

If copy_to_user() fails and returns an -EFAULT error, the memory allocated 
for fibctx was not being freed, which could lead to memory leaks.

> How has this patch been tested?

I have compiled tested the patch. I realize I should have specified "compile tested"
 in the commit message and written "preventing potential memory leaks" instead.  

>> Changes:
>> - Added kfree(fibctx); to release memory when copy_to_user() fails.
>
> Changes compared to what? I don't see a version number in the email
> subject.

I included the "Changes" section to indicate what was modified in the patch. I will 
remove this section in the updated message, as there is no version number to reference.

> Just above the copy_to_user() call there is the following statement:
> 
> 	list_add_tail(&fibctx->next, &dev->fib_list);
>
> Does that mean that the above kfree() will cause list corruption?

Yes, you are correct. I overlooked that fibctx is part of a list, and freeing the 
memory without removing the list entry would corrupt the list. 
The list entry should be deleted before freeing the memory if copy_to_user() fails.

Regards,
Riyan Dhiman