I have also added a few tests to the 'buildbot' that will do signed
SMB2.1 mounts so this won't be missed in the future.
On Mon, Sep 30, 2024 at 8:05 PM Steve French <smfrench@xxxxxxxxx> wrote:
>
> 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
--
Thanks,
Steve
