Skip to content

Commit eee7f5b

Browse files
committed
Merge tag '6.7-rc6-smb3-client-fixes' of git://git.samba.org/sfrench/cifs-2.6
Pull smb client fixes from Steve French: - two multichannel reconnect fixes, one fixing an important refcounting problem that can lead to umount problems - atime fix - five fixes for various potential OOB accesses, including a CVE fix, and two additional fixes for problems pointed out by Robert Morris's fuzzing investigation * tag '6.7-rc6-smb3-client-fixes' of git://git.samba.org/sfrench/cifs-2.6: cifs: do not let cifs_chan_update_iface deallocate channels cifs: fix a pending undercount of srv_count fs: cifs: Fix atime update check smb: client: fix potential OOB in smb2_dump_detail() smb: client: fix potential OOB in cifs_dump_detail() smb: client: fix OOB in smbCalcSize() smb: client: fix OOB in SMB2_query_info_init() smb: client: fix OOB in cifsd when receiving compounded resps
2 parents 1bf5c89 + 12d1e30 commit eee7f5b

File tree

9 files changed

+93
-72
lines changed

9 files changed

+93
-72
lines changed

fs/smb/client/cifs_debug.c

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -40,11 +40,13 @@ void cifs_dump_detail(void *buf, struct TCP_Server_Info *server)
4040
#ifdef CONFIG_CIFS_DEBUG2
4141
struct smb_hdr *smb = buf;
4242

43-
cifs_dbg(VFS, "Cmd: %d Err: 0x%x Flags: 0x%x Flgs2: 0x%x Mid: %d Pid: %d\n",
44-
smb->Command, smb->Status.CifsError,
45-
smb->Flags, smb->Flags2, smb->Mid, smb->Pid);
46-
cifs_dbg(VFS, "smb buf %p len %u\n", smb,
47-
server->ops->calc_smb_size(smb));
43+
cifs_dbg(VFS, "Cmd: %d Err: 0x%x Flags: 0x%x Flgs2: 0x%x Mid: %d Pid: %d Wct: %d\n",
44+
smb->Command, smb->Status.CifsError, smb->Flags,
45+
smb->Flags2, smb->Mid, smb->Pid, smb->WordCount);
46+
if (!server->ops->check_message(buf, server->total_read, server)) {
47+
cifs_dbg(VFS, "smb buf %p len %u\n", smb,
48+
server->ops->calc_smb_size(smb));
49+
}
4850
#endif /* CONFIG_CIFS_DEBUG2 */
4951
}
5052

fs/smb/client/cifsglob.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -532,7 +532,8 @@ struct smb_version_operations {
532532
struct mid_q_entry **, char **, int *);
533533
enum securityEnum (*select_sectype)(struct TCP_Server_Info *,
534534
enum securityEnum);
535-
int (*next_header)(char *);
535+
int (*next_header)(struct TCP_Server_Info *server, char *buf,
536+
unsigned int *noff);
536537
/* ioctl passthrough for query_info */
537538
int (*ioctl_query_info)(const unsigned int xid,
538539
struct cifs_tcon *tcon,

fs/smb/client/connect.c

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1201,7 +1201,12 @@ cifs_demultiplex_thread(void *p)
12011201
server->total_read += length;
12021202

12031203
if (server->ops->next_header) {
1204-
next_offset = server->ops->next_header(buf);
1204+
if (server->ops->next_header(server, buf, &next_offset)) {
1205+
cifs_dbg(VFS, "%s: malformed response (next_offset=%u)\n",
1206+
__func__, next_offset);
1207+
cifs_reconnect(server, true);
1208+
continue;
1209+
}
12051210
if (next_offset)
12061211
server->pdu_size = next_offset;
12071212
}

fs/smb/client/file.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4671,7 +4671,7 @@ static int cifs_readpage_worker(struct file *file, struct page *page,
46714671
/* we do not want atime to be less than mtime, it broke some apps */
46724672
atime = inode_set_atime_to_ts(inode, current_time(inode));
46734673
mtime = inode_get_mtime(inode);
4674-
if (timespec64_compare(&atime, &mtime))
4674+
if (timespec64_compare(&atime, &mtime) < 0)
46754675
inode_set_atime_to_ts(inode, inode_get_mtime(inode));
46764676

46774677
if (PAGE_SIZE > rc)

fs/smb/client/misc.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -363,6 +363,10 @@ checkSMB(char *buf, unsigned int total_read, struct TCP_Server_Info *server)
363363
cifs_dbg(VFS, "Length less than smb header size\n");
364364
}
365365
return -EIO;
366+
} else if (total_read < sizeof(*smb) + 2 * smb->WordCount) {
367+
cifs_dbg(VFS, "%s: can't read BCC due to invalid WordCount(%u)\n",
368+
__func__, smb->WordCount);
369+
return -EIO;
366370
}
367371

368372
/* otherwise, there is enough to get to the BCC */

fs/smb/client/sess.c

Lines changed: 19 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -439,15 +439,15 @@ cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server)
439439
cifs_dbg(FYI, "unable to find a suitable iface\n");
440440
}
441441

442-
if (!chan_index && !iface) {
442+
if (!iface) {
443443
cifs_dbg(FYI, "unable to get the interface matching: %pIS\n",
444444
&ss);
445445
spin_unlock(&ses->iface_lock);
446446
return 0;
447447
}
448448

449449
/* now drop the ref to the current iface */
450-
if (old_iface && iface) {
450+
if (old_iface) {
451451
cifs_dbg(FYI, "replacing iface: %pIS with %pIS\n",
452452
&old_iface->sockaddr,
453453
&iface->sockaddr);
@@ -460,44 +460,32 @@ cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server)
460460

461461
kref_put(&old_iface->refcount, release_iface);
462462
} else if (old_iface) {
463-
cifs_dbg(FYI, "releasing ref to iface: %pIS\n",
463+
/* if a new candidate is not found, keep things as is */
464+
cifs_dbg(FYI, "could not replace iface: %pIS\n",
464465
&old_iface->sockaddr);
465-
466-
old_iface->num_channels--;
467-
if (old_iface->weight_fulfilled)
468-
old_iface->weight_fulfilled--;
469-
470-
kref_put(&old_iface->refcount, release_iface);
471466
} else if (!chan_index) {
472467
/* special case: update interface for primary channel */
473-
cifs_dbg(FYI, "referencing primary channel iface: %pIS\n",
474-
&iface->sockaddr);
475-
iface->num_channels++;
476-
iface->weight_fulfilled++;
477-
} else {
478-
WARN_ON(!iface);
479-
cifs_dbg(FYI, "adding new iface: %pIS\n", &iface->sockaddr);
468+
if (iface) {
469+
cifs_dbg(FYI, "referencing primary channel iface: %pIS\n",
470+
&iface->sockaddr);
471+
iface->num_channels++;
472+
iface->weight_fulfilled++;
473+
}
480474
}
481475
spin_unlock(&ses->iface_lock);
482476

483-
spin_lock(&ses->chan_lock);
484-
chan_index = cifs_ses_get_chan_index(ses, server);
485-
if (chan_index == CIFS_INVAL_CHAN_INDEX) {
477+
if (iface) {
478+
spin_lock(&ses->chan_lock);
479+
chan_index = cifs_ses_get_chan_index(ses, server);
480+
if (chan_index == CIFS_INVAL_CHAN_INDEX) {
481+
spin_unlock(&ses->chan_lock);
482+
return 0;
483+
}
484+
485+
ses->chans[chan_index].iface = iface;
486486
spin_unlock(&ses->chan_lock);
487-
return 0;
488487
}
489488

490-
ses->chans[chan_index].iface = iface;
491-
492-
/* No iface is found. if secondary chan, drop connection */
493-
if (!iface && SERVER_IS_CHAN(server))
494-
ses->chans[chan_index].server = NULL;
495-
496-
spin_unlock(&ses->chan_lock);
497-
498-
if (!iface && SERVER_IS_CHAN(server))
499-
cifs_put_tcp_session(server, false);
500-
501489
return rc;
502490
}
503491

fs/smb/client/smb2misc.c

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,21 @@ smb2_check_message(char *buf, unsigned int len, struct TCP_Server_Info *server)
173173
}
174174

175175
mid = le64_to_cpu(shdr->MessageId);
176+
if (check_smb2_hdr(shdr, mid))
177+
return 1;
178+
179+
if (shdr->StructureSize != SMB2_HEADER_STRUCTURE_SIZE) {
180+
cifs_dbg(VFS, "Invalid structure size %u\n",
181+
le16_to_cpu(shdr->StructureSize));
182+
return 1;
183+
}
184+
185+
command = le16_to_cpu(shdr->Command);
186+
if (command >= NUMBER_OF_SMB2_COMMANDS) {
187+
cifs_dbg(VFS, "Invalid SMB2 command %d\n", command);
188+
return 1;
189+
}
190+
176191
if (len < pdu_size) {
177192
if ((len >= hdr_size)
178193
&& (shdr->Status != 0)) {
@@ -193,21 +208,6 @@ smb2_check_message(char *buf, unsigned int len, struct TCP_Server_Info *server)
193208
return 1;
194209
}
195210

196-
if (check_smb2_hdr(shdr, mid))
197-
return 1;
198-
199-
if (shdr->StructureSize != SMB2_HEADER_STRUCTURE_SIZE) {
200-
cifs_dbg(VFS, "Invalid structure size %u\n",
201-
le16_to_cpu(shdr->StructureSize));
202-
return 1;
203-
}
204-
205-
command = le16_to_cpu(shdr->Command);
206-
if (command >= NUMBER_OF_SMB2_COMMANDS) {
207-
cifs_dbg(VFS, "Invalid SMB2 command %d\n", command);
208-
return 1;
209-
}
210-
211211
if (smb2_rsp_struct_sizes[command] != pdu->StructureSize2) {
212212
if (command != SMB2_OPLOCK_BREAK_HE && (shdr->Status == 0 ||
213213
pdu->StructureSize2 != SMB2_ERROR_STRUCTURE_SIZE2_LE)) {

fs/smb/client/smb2ops.c

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -403,8 +403,10 @@ smb2_dump_detail(void *buf, struct TCP_Server_Info *server)
403403
cifs_server_dbg(VFS, "Cmd: %d Err: 0x%x Flags: 0x%x Mid: %llu Pid: %d\n",
404404
shdr->Command, shdr->Status, shdr->Flags, shdr->MessageId,
405405
shdr->Id.SyncId.ProcessId);
406-
cifs_server_dbg(VFS, "smb buf %p len %u\n", buf,
407-
server->ops->calc_smb_size(buf));
406+
if (!server->ops->check_message(buf, server->total_read, server)) {
407+
cifs_server_dbg(VFS, "smb buf %p len %u\n", buf,
408+
server->ops->calc_smb_size(buf));
409+
}
408410
#endif
409411
}
410412

@@ -5074,17 +5076,22 @@ smb3_handle_read_data(struct TCP_Server_Info *server, struct mid_q_entry *mid)
50745076
NULL, 0, false);
50755077
}
50765078

5077-
static int
5078-
smb2_next_header(char *buf)
5079+
static int smb2_next_header(struct TCP_Server_Info *server, char *buf,
5080+
unsigned int *noff)
50795081
{
50805082
struct smb2_hdr *hdr = (struct smb2_hdr *)buf;
50815083
struct smb2_transform_hdr *t_hdr = (struct smb2_transform_hdr *)buf;
50825084

5083-
if (hdr->ProtocolId == SMB2_TRANSFORM_PROTO_NUM)
5084-
return sizeof(struct smb2_transform_hdr) +
5085-
le32_to_cpu(t_hdr->OriginalMessageSize);
5086-
5087-
return le32_to_cpu(hdr->NextCommand);
5085+
if (hdr->ProtocolId == SMB2_TRANSFORM_PROTO_NUM) {
5086+
*noff = le32_to_cpu(t_hdr->OriginalMessageSize);
5087+
if (unlikely(check_add_overflow(*noff, sizeof(*t_hdr), noff)))
5088+
return -EINVAL;
5089+
} else {
5090+
*noff = le32_to_cpu(hdr->NextCommand);
5091+
}
5092+
if (unlikely(*noff && *noff < MID_HEADER_SIZE(server)))
5093+
return -EINVAL;
5094+
return 0;
50885095
}
50895096

50905097
int cifs_sfu_make_node(unsigned int xid, struct inode *inode,

fs/smb/client/smb2pdu.c

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -411,8 +411,7 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon,
411411
}
412412

413413
if (smb2_command != SMB2_INTERNAL_CMD)
414-
if (mod_delayed_work(cifsiod_wq, &server->reconnect, 0))
415-
cifs_put_tcp_session(server, false);
414+
mod_delayed_work(cifsiod_wq, &server->reconnect, 0);
416415

417416
atomic_inc(&tconInfoReconnectCount);
418417
out:
@@ -471,10 +470,15 @@ static int __smb2_plain_req_init(__le16 smb2_command, struct cifs_tcon *tcon,
471470
void **request_buf, unsigned int *total_len)
472471
{
473472
/* BB eventually switch this to SMB2 specific small buf size */
474-
if (smb2_command == SMB2_SET_INFO)
473+
switch (smb2_command) {
474+
case SMB2_SET_INFO:
475+
case SMB2_QUERY_INFO:
475476
*request_buf = cifs_buf_get();
476-
else
477+
break;
478+
default:
477479
*request_buf = cifs_small_buf_get();
480+
break;
481+
}
478482
if (*request_buf == NULL) {
479483
/* BB should we add a retry in here if not a writepage? */
480484
return -ENOMEM;
@@ -3587,8 +3591,13 @@ SMB2_query_info_init(struct cifs_tcon *tcon, struct TCP_Server_Info *server,
35873591
struct smb2_query_info_req *req;
35883592
struct kvec *iov = rqst->rq_iov;
35893593
unsigned int total_len;
3594+
size_t len;
35903595
int rc;
35913596

3597+
if (unlikely(check_add_overflow(input_len, sizeof(*req), &len) ||
3598+
len > CIFSMaxBufSize))
3599+
return -EINVAL;
3600+
35923601
rc = smb2_plain_req_init(SMB2_QUERY_INFO, tcon, server,
35933602
(void **) &req, &total_len);
35943603
if (rc)
@@ -3610,15 +3619,15 @@ SMB2_query_info_init(struct cifs_tcon *tcon, struct TCP_Server_Info *server,
36103619

36113620
iov[0].iov_base = (char *)req;
36123621
/* 1 for Buffer */
3613-
iov[0].iov_len = total_len - 1 + input_len;
3622+
iov[0].iov_len = len;
36143623
return 0;
36153624
}
36163625

36173626
void
36183627
SMB2_query_info_free(struct smb_rqst *rqst)
36193628
{
36203629
if (rqst && rqst->rq_iov)
3621-
cifs_small_buf_release(rqst->rq_iov[0].iov_base); /* request */
3630+
cifs_buf_release(rqst->rq_iov[0].iov_base); /* request */
36223631
}
36233632

36243633
static int
@@ -5493,6 +5502,11 @@ build_qfs_info_req(struct kvec *iov, struct cifs_tcon *tcon,
54935502
return 0;
54945503
}
54955504

5505+
static inline void free_qfs_info_req(struct kvec *iov)
5506+
{
5507+
cifs_buf_release(iov->iov_base);
5508+
}
5509+
54965510
int
54975511
SMB311_posix_qfs_info(const unsigned int xid, struct cifs_tcon *tcon,
54985512
u64 persistent_fid, u64 volatile_fid, struct kstatfs *fsdata)
@@ -5524,7 +5538,7 @@ SMB311_posix_qfs_info(const unsigned int xid, struct cifs_tcon *tcon,
55245538

55255539
rc = cifs_send_recv(xid, ses, server,
55265540
&rqst, &resp_buftype, flags, &rsp_iov);
5527-
cifs_small_buf_release(iov.iov_base);
5541+
free_qfs_info_req(&iov);
55285542
if (rc) {
55295543
cifs_stats_fail_inc(tcon, SMB2_QUERY_INFO_HE);
55305544
goto posix_qfsinf_exit;
@@ -5575,7 +5589,7 @@ SMB2_QFS_info(const unsigned int xid, struct cifs_tcon *tcon,
55755589

55765590
rc = cifs_send_recv(xid, ses, server,
55775591
&rqst, &resp_buftype, flags, &rsp_iov);
5578-
cifs_small_buf_release(iov.iov_base);
5592+
free_qfs_info_req(&iov);
55795593
if (rc) {
55805594
cifs_stats_fail_inc(tcon, SMB2_QUERY_INFO_HE);
55815595
goto qfsinf_exit;
@@ -5642,7 +5656,7 @@ SMB2_QFS_attr(const unsigned int xid, struct cifs_tcon *tcon,
56425656

56435657
rc = cifs_send_recv(xid, ses, server,
56445658
&rqst, &resp_buftype, flags, &rsp_iov);
5645-
cifs_small_buf_release(iov.iov_base);
5659+
free_qfs_info_req(&iov);
56465660
if (rc) {
56475661
cifs_stats_fail_inc(tcon, SMB2_QUERY_INFO_HE);
56485662
goto qfsattr_exit;

0 commit comments

Comments
 (0)