Received-SPF: pass (google.com: domain of linux-kernel+bounces-185003-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <f61cb0cb-8a74-494a-b191-8503b51897c5@suse.com>
Date: Tue, 21 May 2024 15:18:44 +0200
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: memory leakage in ncm_wrap_ntb() in USB ncm mode with kernel 5.15
To: "nanfengwq@sina.com" <nanfengwq@sina.com>,
 gregkh <gregkh@linuxfoundation.org>
Cc: balbi <balbi@kernel.org>, linux-usb <linux-usb@vger.kernel.org>,
 linux-kernel <linux-kernel@vger.kernel.org>
References: <2024051922230825069112@sina.com>
 <2024051901-gimmick-cosponsor-f2dd@gregkh> <202405202232198531894@sina.com>
Content-Language: en-US
From: Oliver Neukum <oneukum@suse.com>
In-Reply-To: <202405202232198531894@sina.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit


On 20.05.24 16:32, nanfengwq@sina.com wrote:
> hello:
> In embedded devices with relatively small memory, if the transfer speed of ncm is fast and other programs occupy CPU memory, it is likely that the return value ncm ->skd_tx_data of alloc_skb() is NULL, and the code is likely to enter err. If skb2 is not processed in err, it will cause memory leakage.

Hi,

apart from the submission process, could you please explain how this
condition may happem?

         if (skb) {

[..]
                 if (ncm->skb_tx_data

We know ncm->skb_tx_data != NULL && skb != NULL

                     && (ncm->ndp_dgram_count >= TX_MAX_NUM_DPE
                     || (ncm->skb_tx_data->len +
                     div + rem + skb->len +
                     ncm->skb_tx_ndp->len + ndp_align + (2 * dgram_idx_len))
                     > max_size)) {
                         skb2 = package_for_tx(ncm);


		if (!ncm->skb_tx_data) {

We know ncm->skb_tx_data == NULL && skb != NULL

                         /* Create a new skb for the NTH and datagrams. */
                         ncm->skb_tx_data = alloc_skb(max_size, GFP_ATOMIC);
                         if (!ncm->skb_tx_data)
                                 goto err;

It seems to me that either

skb2 = package_for_tx(ncm);

or

ncm->skb_tx_data = alloc_skb(max_size, GFP_ATOMIC);

can be executed. The code paths seem to be mutually exclusive.

	Regards
		Oliver