Skip to content

Commit 729c0c9

Browse files
sbrivio-rhsmfrench
authored andcommitted
cifs: Fix stack out-of-bounds in smb{2,3}_create_lease_buf()
smb{2,3}_create_lease_buf() store a lease key in the lease context for later usage on a lease break. In most paths, the key is currently sourced from data that happens to be on the stack near local variables for oplock in SMB2_open() callers, e.g. from open_shroot(), whereas smb2_open_file() properly allocates space on its stack for it. The address of those local variables holding the oplock is then passed to create_lease_buf handlers via SMB2_open(), and 16 bytes near oplock are used. This causes a stack out-of-bounds access as reported by KASAN on SMB2.1 and SMB3 mounts (first out-of-bounds access is shown here): [ 111.528823] BUG: KASAN: stack-out-of-bounds in smb3_create_lease_buf+0x399/0x3b0 [cifs] [ 111.530815] Read of size 8 at addr ffff88010829f249 by task mount.cifs/985 [ 111.532838] CPU: 3 PID: 985 Comm: mount.cifs Not tainted 4.18.0-rc3+ #91 [ 111.534656] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1 04/01/2014 [ 111.536838] Call Trace: [ 111.537528] dump_stack+0xc2/0x16b [ 111.540890] print_address_description+0x6a/0x270 [ 111.542185] kasan_report+0x258/0x380 [ 111.544701] smb3_create_lease_buf+0x399/0x3b0 [cifs] [ 111.546134] SMB2_open+0x1ef8/0x4b70 [cifs] [ 111.575883] open_shroot+0x339/0x550 [cifs] [ 111.591969] smb3_qfs_tcon+0x32c/0x1e60 [cifs] [ 111.617405] cifs_mount+0x4f3/0x2fc0 [cifs] [ 111.674332] cifs_smb3_do_mount+0x263/0xf10 [cifs] [ 111.677915] mount_fs+0x55/0x2b0 [ 111.679504] vfs_kern_mount.part.22+0xaa/0x430 [ 111.684511] do_mount+0xc40/0x2660 [ 111.698301] ksys_mount+0x80/0xd0 [ 111.701541] do_syscall_64+0x14e/0x4b0 [ 111.711807] entry_SYSCALL_64_after_hwframe+0x44/0xa9 [ 111.713665] RIP: 0033:0x7f372385b5fa [ 111.715311] Code: 48 8b 0d 99 78 2c 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 49 89 ca b8 a5 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 66 78 2c 00 f7 d8 64 89 01 48 [ 111.720330] RSP: 002b:00007ffff27049d8 EFLAGS: 00000206 ORIG_RAX: 00000000000000a5 [ 111.722601] RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f372385b5fa [ 111.724842] RDX: 000055c2ecdc73b2 RSI: 000055c2ecdc73f9 RDI: 00007ffff270580f [ 111.727083] RBP: 00007ffff2705804 R08: 000055c2ee976060 R09: 0000000000001000 [ 111.729319] R10: 0000000000000000 R11: 0000000000000206 R12: 00007f3723f4d000 [ 111.731615] R13: 000055c2ee976060 R14: 00007f3723f4f90f R15: 0000000000000000 [ 111.735448] The buggy address belongs to the page: [ 111.737420] page:ffffea000420a7c0 count:0 mapcount:0 mapping:0000000000000000 index:0x0 [ 111.739890] flags: 0x17ffffc0000000() [ 111.741750] raw: 0017ffffc0000000 0000000000000000 dead000000000200 0000000000000000 [ 111.744216] raw: 0000000000000000 0000000000000000 00000000ffffffff 0000000000000000 [ 111.746679] page dumped because: kasan: bad access detected [ 111.750482] Memory state around the buggy address: [ 111.752562] ffff88010829f100: 00 f2 f2 f2 f2 f2 f2 f2 00 00 00 00 00 00 00 00 [ 111.754991] ffff88010829f180: 00 00 f2 f2 00 00 00 00 00 00 00 00 00 00 00 00 [ 111.757401] >ffff88010829f200: 00 00 00 00 00 f1 f1 f1 f1 01 f2 f2 f2 f2 f2 f2 [ 111.759801] ^ [ 111.762034] ffff88010829f280: f2 02 f2 f2 f2 f2 f2 f2 f2 00 00 00 00 00 00 00 [ 111.764486] ffff88010829f300: f2 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [ 111.766913] ================================================================== Lease keys are however already generated and stored in fid data on open and create paths: pass them down to the lease context creation handlers and use them. Suggested-by: Aurélien Aptel <aaptel@suse.com> Reviewed-by: Aurelien Aptel <aaptel@suse.com> Fixes: b8c32db ("CIFS: Request SMB2.1 leases") Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Signed-off-by: Steve French <stfrench@microsoft.com>
1 parent 7ffbe65 commit 729c0c9

5 files changed

Lines changed: 14 additions & 21 deletions

File tree

fs/cifs/cifsglob.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -423,7 +423,7 @@ struct smb_version_operations {
423423
void (*set_oplock_level)(struct cifsInodeInfo *, __u32, unsigned int,
424424
bool *);
425425
/* create lease context buffer for CREATE request */
426-
char * (*create_lease_buf)(u8 *, u8);
426+
char * (*create_lease_buf)(u8 *lease_key, u8 oplock);
427427
/* parse lease context buffer and return oplock/epoch info */
428428
__u8 (*parse_lease_buf)(void *buf, unsigned int *epoch, char *lkey);
429429
ssize_t (*copychunk_range)(const unsigned int,

fs/cifs/smb2file.c

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ smb2_open_file(const unsigned int xid, struct cifs_open_parms *oparms,
4141
int rc;
4242
__le16 *smb2_path;
4343
struct smb2_file_all_info *smb2_data = NULL;
44-
__u8 smb2_oplock[17];
44+
__u8 smb2_oplock;
4545
struct cifs_fid *fid = oparms->fid;
4646
struct network_resiliency_req nr_ioctl_req;
4747

@@ -59,12 +59,9 @@ smb2_open_file(const unsigned int xid, struct cifs_open_parms *oparms,
5959
}
6060

6161
oparms->desired_access |= FILE_READ_ATTRIBUTES;
62-
*smb2_oplock = SMB2_OPLOCK_LEVEL_BATCH;
62+
smb2_oplock = SMB2_OPLOCK_LEVEL_BATCH;
6363

64-
if (oparms->tcon->ses->server->capabilities & SMB2_GLOBAL_CAP_LEASING)
65-
memcpy(smb2_oplock + 1, fid->lease_key, SMB2_LEASE_KEY_SIZE);
66-
67-
rc = SMB2_open(xid, oparms, smb2_path, smb2_oplock, smb2_data, NULL,
64+
rc = SMB2_open(xid, oparms, smb2_path, &smb2_oplock, smb2_data, NULL,
6865
NULL);
6966
if (rc)
7067
goto out;
@@ -101,7 +98,7 @@ smb2_open_file(const unsigned int xid, struct cifs_open_parms *oparms,
10198
move_smb2_info_to_cifs(buf, smb2_data);
10299
}
103100

104-
*oplock = *smb2_oplock;
101+
*oplock = smb2_oplock;
105102
out:
106103
kfree(smb2_data);
107104
kfree(smb2_path);

fs/cifs/smb2ops.c

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2222,8 +2222,7 @@ smb2_create_lease_buf(u8 *lease_key, u8 oplock)
22222222
if (!buf)
22232223
return NULL;
22242224

2225-
buf->lcontext.LeaseKeyLow = cpu_to_le64(*((u64 *)lease_key));
2226-
buf->lcontext.LeaseKeyHigh = cpu_to_le64(*((u64 *)(lease_key + 8)));
2225+
memcpy(&buf->lcontext.LeaseKey, lease_key, SMB2_LEASE_KEY_SIZE);
22272226
buf->lcontext.LeaseState = map_oplock_to_lease(oplock);
22282227

22292228
buf->ccontext.DataOffset = cpu_to_le16(offsetof
@@ -2249,8 +2248,7 @@ smb3_create_lease_buf(u8 *lease_key, u8 oplock)
22492248
if (!buf)
22502249
return NULL;
22512250

2252-
buf->lcontext.LeaseKeyLow = cpu_to_le64(*((u64 *)lease_key));
2253-
buf->lcontext.LeaseKeyHigh = cpu_to_le64(*((u64 *)(lease_key + 8)));
2251+
memcpy(&buf->lcontext.LeaseKey, lease_key, SMB2_LEASE_KEY_SIZE);
22542252
buf->lcontext.LeaseState = map_oplock_to_lease(oplock);
22552253

22562254
buf->ccontext.DataOffset = cpu_to_le16(offsetof
@@ -2287,8 +2285,7 @@ smb3_parse_lease_buf(void *buf, unsigned int *epoch, char *lease_key)
22872285
if (lc->lcontext.LeaseFlags & SMB2_LEASE_FLAG_BREAK_IN_PROGRESS)
22882286
return SMB2_OPLOCK_LEVEL_NOCHANGE;
22892287
if (lease_key)
2290-
memcpy(lease_key, &lc->lcontext.LeaseKeyLow,
2291-
SMB2_LEASE_KEY_SIZE);
2288+
memcpy(lease_key, &lc->lcontext.LeaseKey, SMB2_LEASE_KEY_SIZE);
22922289
return le32_to_cpu(lc->lcontext.LeaseState);
22932290
}
22942291

fs/cifs/smb2pdu.c

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1716,12 +1716,12 @@ parse_lease_state(struct TCP_Server_Info *server, struct smb2_create_rsp *rsp,
17161716

17171717
static int
17181718
add_lease_context(struct TCP_Server_Info *server, struct kvec *iov,
1719-
unsigned int *num_iovec, __u8 *oplock)
1719+
unsigned int *num_iovec, u8 *lease_key, __u8 *oplock)
17201720
{
17211721
struct smb2_create_req *req = iov[0].iov_base;
17221722
unsigned int num = *num_iovec;
17231723

1724-
iov[num].iov_base = server->ops->create_lease_buf(oplock+1, *oplock);
1724+
iov[num].iov_base = server->ops->create_lease_buf(lease_key, *oplock);
17251725
if (iov[num].iov_base == NULL)
17261726
return -ENOMEM;
17271727
iov[num].iov_len = server->vals->create_lease_size;
@@ -2181,7 +2181,8 @@ SMB2_open(const unsigned int xid, struct cifs_open_parms *oparms, __le16 *path,
21812181
*oplock == SMB2_OPLOCK_LEVEL_NONE)
21822182
req->RequestedOplockLevel = *oplock;
21832183
else {
2184-
rc = add_lease_context(server, iov, &n_iov, oplock);
2184+
rc = add_lease_context(server, iov, &n_iov,
2185+
oparms->fid->lease_key, oplock);
21852186
if (rc) {
21862187
cifs_small_buf_release(req);
21872188
kfree(copy_path);

fs/cifs/smb2pdu.h

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -678,16 +678,14 @@ struct create_context {
678678
#define SMB2_LEASE_KEY_SIZE 16
679679

680680
struct lease_context {
681-
__le64 LeaseKeyLow;
682-
__le64 LeaseKeyHigh;
681+
u8 LeaseKey[SMB2_LEASE_KEY_SIZE];
683682
__le32 LeaseState;
684683
__le32 LeaseFlags;
685684
__le64 LeaseDuration;
686685
} __packed;
687686

688687
struct lease_context_v2 {
689-
__le64 LeaseKeyLow;
690-
__le64 LeaseKeyHigh;
688+
u8 LeaseKey[SMB2_LEASE_KEY_SIZE];
691689
__le32 LeaseState;
692690
__le32 LeaseFlags;
693691
__le64 LeaseDuration;

0 commit comments

Comments
 (0)