linux-kernel - Re: [PATCH V2 0/4] Fix mid_q_entry memory leaks in SMB client

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: <fk2zvqpxznijk27ourdtyiezi3nl2b6nwsfpjnue4phne5rnqb@d66oqhlwxygs>
Date: Wed, 6 Aug 2025 09:24:03 -0300
From: Enzo Matsumiya <ematsumiya@...e.de>
To: Wang Zhaolong <wangzhaolong@...weicloud.com>
Cc: Steve French <smfrench@...il.com>, pshilov@...rosoft.com, 
	linux-cifs@...r.kernel.org, samba-technical@...ts.samba.org, linux-kernel@...r.kernel.org, 
	chengzhihao1@...wei.com, yi.zhang@...wei.com, yangerkun@...wei.com
Subject: Re: [PATCH V2 0/4] Fix mid_q_entry memory leaks in SMB client

On 08/06, Wang Zhaolong wrote:
>>The first three patches (cleanup) look fine and have added to
>>cifs-2.6.git for-next (also added Enzo Acked-by) but the fourth patch
>>("smb: client: fix mid_q_entry memleak leak with per-mid locking")
>>causes xfstest generic/001 to fail with signing enabled.  See
>>http://smb311-linux-testing.southcentralus.cloudapp.azure.com/#/builders/5/builds/58/steps/34/logs/stdio
>>and http://smb311-linux-testing.southcentralus.cloudapp.azure.com/#/builders/5/builds/59/steps/34/logs/stdio
>>
>
>I am unable to view any information in the link above. Is this information
>only visible to logged-in users?

That one is publicly visible.

If you're using a Chrome-based browser, you might need to whitelist the
website though as it doesn't use HTTPS.

> ...
>>
>>(it worked without the patch see e.g.
>>http://smb311-linux-testing.southcentralus.cloudapp.azure.com/#/builders/5/builds/60
>>and http://smb311-linux-testing.southcentralus.cloudapp.azure.com/#/builders/5/builds/56)
>>
>>On Tue, Aug 5, 2025 at 1:54 AM Wang Zhaolong
>><wangzhaolong@...weicloud.com> wrote:
>
>
>It's quite strange that the lock reported in the stack trace is an internal
>lock of the crypto module, which only protects the internal logic of crypto.
>Moreover, I have not yet found a path where the callback for cifs registration
>is executed within the scope of this lock.
>
>```c
>// crypto/api.c
>static struct crypto_alg *crypto_alg_lookup(const char *name, u32 type,
>					    u32 mask)
>{
>	const u32 fips = CRYPTO_ALG_FIPS_INTERNAL;
>	struct crypto_alg *alg;
>	u32 test = 0;
>
>	if (!((type | mask) & CRYPTO_ALG_TESTED))
>		test |= CRYPTO_ALG_TESTED;
>
>	down_read(&crypto_alg_sem);
>	...
>	up_read(&crypto_alg_sem);
>	return alg;
>```
>More information is needed to confirm this issue. Could you please provide it?

In summary the problem is in mid_execute_callback() when callback is
smb2_writev_callback.

Cleaning it up for clarity:

---- begin ----
     cifsd/24912 is trying to lock crypto_alg_sem at crypto_alg_lookup+0x40/0x120
     cifsd/24912 is holding &temp->mid_lock at mid_execute_callback+0x19/0x40

     Reversed call trace:
     cifs_demultiplex_thread
       mid_execute_callback
         <lock mid_lock>
         smb2_writev_callback
           smb2_check_receive
             smb2_verify_signature
               smb3_calc_signature
                 cifs_alloc_hash
                   crypto_alloc_tfm_node
                     crypto_alg_mod_lookup
                       crypto_alg_lookup
                         down_read
                           lock_acquire
                             __lock_acquire
                               >>> BUG() here <<<
         ...
         <unlock mid_lock>
---- end ----

Before the patch I sent you privately yesterday, I confirmed this
by locking mid_lock directly only for cifs_compound*callback and
cifs_wake_up_task.

Also you need to make sure to use a reproducer/tester that uses writev,
which your poc.c from original bug report doesn't.

HTH


Cheers,

Enzo