Received-SPF: pass (google.com: domain of linux-kernel+bounces-112278-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Sat, 23 Mar 2024 10:29:22 +0100
Message-ID: <87msqpxpn1.wl-tiwai@suse.de>
From: Takashi Iwai <tiwai@suse.de>
To: LuMingYin <lumingyindetect@163.com>
Cc: linux-sound@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	tiwai@suse.com,
	perex@perex.cz,
	minhuadotchen@gmail.com
Subject: Re: [PATCH] sound:Fix a memory leak related to variable data
In-Reply-To: <20240323080413.641089-1-lumingyindetect@163.com>
References: <20240323080413.641089-1-lumingyindetect@163.com>
User-Agent: Wanderlust/2.15.9 (Almost Unreal) Emacs/27.2 Mule/6.0
Precedence: bulk
MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue")
Content-Type: text/plain; charset=US-ASCII

On Sat, 23 Mar 2024 09:04:13 +0100,
LuMingYin wrote:
> 
> In the file /linux/sound/core/control_compat.c, a pointer named 'data' is defined at line 82. This pointer allocates a block of dynamic memory using the 'kzalloc' function at line 85. When the if statement at line 86 returns false, it indicates successful allocation of the dynamic memory area pointed to by 'data'. However, when the if statement at line 90 returns true, the program returns at line 91. During this process, the dynamic memory area pointed to by 'data' is neither used as in the switch statement at line 108 nor deallocated, leading to a memory leak bug. The if statement at line 95 also has the same issue. This commit fixes this problem.
> 
> Signed-off-by: LuMingYin <lumingyindetect@163.com>

It's using the new automatic free mechanism.  See the commit
1052d9882269 ("ALSA: control: Use automatic cleanup of kfree()")


thanks,

Takashi


> ---
>  sound/core/control_compat.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/sound/core/control_compat.c b/sound/core/control_compat.c
> index 934bb945e702..32113eb06f68 100644
> --- a/sound/core/control_compat.c
> +++ b/sound/core/control_compat.c
> @@ -87,13 +87,19 @@ static int snd_ctl_elem_info_compat(struct snd_ctl_file *ctl,
>  		return -ENOMEM;
>  
>  	/* copy id */
> -	if (copy_from_user(&data->id, &data32->id, sizeof(data->id)))
> +	if (copy_from_user(&data->id, &data32->id, sizeof(data->id))){
> +		kfree(data);
> +		data = NULL;
>  		return -EFAULT;
> +	}
>  	/* we need to copy the item index.
>  	 * hope this doesn't break anything..
>  	 */
> -	if (get_user(data->value.enumerated.item, &data32->value.enumerated.item))
> +	if (get_user(data->value.enumerated.item, &data32->value.enumerated.item)){
> +		kfree(data);
> +		data = NULL;
>  		return -EFAULT;
> +	}
>  
>  	err = snd_ctl_elem_info(ctl, data);
>  	if (err < 0)
> -- 
> 2.25.1
>