I also see a problem with smb2.1 signed mounts (will need to make sure
our buildbot is testing with this config) so have reverted this patch
in for-next
On Mon, Sep 30, 2024 at 6:35 PM Paulo Alcantara <pc@xxxxxxxxxxxxx> wrote:
>
> Enzo Matsumiya <ematsumiya@xxxxxxx> writes:
>
> > The SHA-512 shash TFM is used only briefly during Session Setup stage,
> > when computing SMB 3.1.1 preauth hash.
> >
> > There's no need to keep it allocated in servers' secmech the whole time,
> > so keep its lifetime inside smb311_update_preauth_hash().
> >
> > This also makes smb311_crypto_shash_allocate() redundant, so expose
> > smb3_crypto_shash_allocate() and use that.
> >
> > Signed-off-by: Enzo Matsumiya <ematsumiya@xxxxxxx>
> > ---
> > fs/smb/client/cifsencrypt.c | 1 -
> > fs/smb/client/cifsglob.h | 1 -
> > fs/smb/client/sess.c | 2 +-
> > fs/smb/client/smb2misc.c | 28 ++++++++++++++--------------
> > fs/smb/client/smb2proto.h | 2 +-
> > fs/smb/client/smb2transport.c | 30 +-----------------------------
> > 6 files changed, 17 insertions(+), 47 deletions(-)
>
> This commit leads to the following NULL ptr deref when mounting a share
> with 'vers=2.1'. Reproduced it against Windows Server 2022 and samba
> 4.21.
>
> $ mount.cifs //srv/share /mnt -o ...,vers=2.1
>
> BUG: KASAN: null-ptr-deref in smb2_calc_signature+0x195/0x4f0 [cifs]
> Read of size 8 at addr 0000000000000000 by task mount.cifs/693
>
> CPU: 2 UID: 0 PID: 693 Comm: mount.cifs Not tainted 6.11.0 #3
> Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-2.fc40
> 04/01/2014
> Call Trace:
> <TASK>
> dump_stack_lvl+0x5d/0x80
> ? smb2_calc_signature+0x195/0x4f0 [cifs]
> kasan_report+0xda/0x110
> ? smb2_calc_signature+0x195/0x4f0 [cifs]
> smb2_calc_signature+0x195/0x4f0 [cifs]
> ? __pfx_smb2_calc_signature+0x10/0x10 [cifs]
> ? do_raw_spin_trylock+0xc6/0x120
> ? do_raw_spin_unlock+0x9a/0xf0
> ? _raw_spin_unlock+0x23/0x40
> ? smb2_sign_rqst+0x10c/0x160 [cifs]
> smb2_setup_request+0x225/0x3a0 [cifs]
> compound_send_recv+0x417/0x1140 [cifs]
> ? __pfx_compound_send_recv+0x10/0x10 [cifs]
> ? __create_object+0x5e/0x90
> ? hlock_class+0x32/0xb0
> ? do_raw_spin_unlock+0x9a/0xf0
> cifs_send_recv+0x23/0x30 [cifs]
> SMB2_tcon+0x3ec/0xb30 [cifs]
> ? __pfx_SMB2_tcon+0x10/0x10 [cifs]
> ? ___ratelimit+0x133/0x1f0
> ? __pfx____ratelimit+0x10/0x10
> ? __pfx_SMB2_tcon+0x10/0x10 [cifs]
> ? cifs_get_smb_ses+0xcaf/0x1070 [cifs]
> cifs_get_smb_ses+0xcaf/0x1070 [cifs]
> ? __pfx_cifs_get_smb_ses+0x10/0x10 [cifs]
> ? cifs_get_tcp_session+0xaa0/0xca0 [cifs]
> cifs_mount_get_session+0x8a/0x210 [cifs]
> dfs_mount_share+0x1b0/0x11d0 [cifs]
> ? __pfx___lock_acquire+0x10/0x10
> ? __pfx_dfs_mount_share+0x10/0x10 [cifs]
> ? lock_acquire+0x14b/0x3e0
> ? find_held_lock+0x8a/0xa0
> ? hlock_class+0x32/0xb0
> ? lock_release+0x203/0x5d0
> cifs_mount+0xb3/0x3d0 [cifs]
> ? do_raw_spin_trylock+0xc6/0x120
> ? __pfx_cifs_mount+0x10/0x10 [cifs]
> ? ___ratelimit+0x133/0x1f0
> ? smb3_update_mnt_flags+0x372/0x3b0 [cifs]
> cifs_smb3_do_mount+0x1e2/0xc80 [cifs]
> ? __pfx___mutex_lock+0x10/0x10
> ? __pfx_cifs_smb3_do_mount+0x10/0x10 [cifs]
> smb3_get_tree+0x1bf/0x330 [cifs]
> vfs_get_tree+0x4a/0x160
> path_mount+0x3c1/0xfb0
> ? kasan_quarantine_put+0xc7/0x1d0
> ? __pfx_path_mount+0x10/0x10
> ? kmem_cache_free+0x118/0x3e0
> ? user_path_at+0x74/0xa0
> __x64_sys_mount+0x1a6/0x1e0
> ? __pfx___x64_sys_mount+0x10/0x10
> ? mark_held_locks+0x1a/0x90
> do_syscall_64+0xbb/0x1d0
> entry_SYSCALL_64_after_hwframe+0x77/0x7f
> RIP: 0033:0x7fd23a9a88fe
> Code: 48 8b 0d 1d a5 0c 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84
> 00 00 00 00 00 90 f3 0f 1e fa 49 89 ca b8 a5 00 00 00 0f 05 <48> 3d 01
> f0 ff ff 73 01 c3 48 8b 0d ea a4 0c 00 f7 d8 64 89 01 48
> RSP: 002b:00007ffdeef79388 EFLAGS: 00000246 ORIG_RAX: 00000000000000a5
> RAX: ffffffffffffffda RBX: 0000562a96057eb0 RCX: 00007fd23a9a88fe
> RDX: 0000562a6d33347e RSI: 0000562a6d3334e4 RDI: 00007ffdeef7a4a2
> RBP: 00007ffdeef79440 R08: 0000562a96057eb0 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000246 R12: 000000000000000a
> R13: 00007ffdeef7a4a2 R14: 0000562a96058f00 R15: 00007fd23aa96000
> </TASK>
>
> $ ./scripts/faddr2line --list fs/smb/client/cifs.o smb2_calc_signature+0x195
> smb2_calc_signature+0x195/0x4f0:
>
> smb2_calc_signature at /home/pc/g/linux/fs/smb/client/smb2transport.c:235
> 230 }
> 231 } else {
> 232 shash = server->secmech.hmacsha256;
> 233 }
> 234
> >235< rc = crypto_shash_setkey(shash->tfm, ses->auth_key.response,
> 236 SMB2_NTLMV2_SESSKEY_SIZE);
> 237 if (rc) {
> 238 cifs_server_dbg(VFS,
> 239 "%s: Could not update with response\n",
> 240 __func__);
>
> What's the problem with having it allocated the whole time? Reconnect
> path will need it to re-establish SMB sessions, so this means we'll now
> have to allocate it every time we attempt to reconnect a session? This
> could be worse when smb2_reconnect_server() is periodically attemping to
> reconnect a session.
--
Thanks,
Steve
