Message-ID: <2d3ef764f006af12216c663b7a0f532741a04dd6.camel@sipsolutions.net>
Subject: Re: [PATCH] mac80211: fix a memory leak where sta_info is not freed
From:   Johannes Berg <johannes@sipsolutions.net>
To:     Ahmed Zaki <anzaki@gmail.com>
Cc:     linux-wireless@vger.kernel.org
Date:   Mon, 15 Nov 2021 11:06:47 +0100
In-Reply-To: <20211002145329.3125293-1-anzaki@gmail.com>
References: <20211002145329.3125293-1-anzaki@gmail.com>
Content-Type: text/plain; charset="UTF-8"
User-Agent: Evolution 3.42.1 (3.42.1-1.fc35) 
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Sat, 2021-10-02 at 08:53 -0600, Ahmed Zaki wrote:
> 
> +++ b/net/mac80211/sta_info.c
> @@ -640,12 +640,14 @@ static int sta_info_insert_finish(struct sta_info *sta) __acquires(RCU)
>  
>  	/* check if STA exists already */
>  	if (sta_info_get_bss(sdata, sta->sta.addr)) {
> +		sta_info_free(local, sta);
>  		err = -EEXIST;
>  		goto out_err;
>  	}
>  
>  	sinfo = kzalloc(sizeof(struct station_info), GFP_KERNEL);
>  	if (!sinfo) {
> +		sta_info_free(local, sta);
>  		err = -ENOMEM;
>  		goto out_err;
>  	}

We already have a "out_drop_sta", that calls cleanup_single_sta(), so
shouldn't we just add a label there ("out_free_sta") just before
cleanup_single_sta() and then we don't need the extra call?

johannes