* [PATCH 1/2] smb: client: fix potential OOB in cifs_dump_detail() @ 2023-12-16 4:10 Paulo Alcantara 2023-12-16 4:10 ` [PATCH 2/2] smb: client: fix potential OOB in smb2_dump_detail() Paulo Alcantara 0 siblings, 1 reply; 5+ messages in thread From: Paulo Alcantara @ 2023-12-16 4:10 UTC (permalink / raw) To: smfrench; +Cc: linux-cifs, Paulo Alcantara Validate SMB message with ->check_message() before calling ->calc_smb_size(). Signed-off-by: Paulo Alcantara (SUSE) <pc@manguebit.com> --- fs/smb/client/cifs_debug.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/fs/smb/client/cifs_debug.c b/fs/smb/client/cifs_debug.c index 5596c9f30ccb..60027f5aebe8 100644 --- a/fs/smb/client/cifs_debug.c +++ b/fs/smb/client/cifs_debug.c @@ -40,11 +40,13 @@ void cifs_dump_detail(void *buf, struct TCP_Server_Info *server) #ifdef CONFIG_CIFS_DEBUG2 struct smb_hdr *smb = buf; - cifs_dbg(VFS, "Cmd: %d Err: 0x%x Flags: 0x%x Flgs2: 0x%x Mid: %d Pid: %d\n", - smb->Command, smb->Status.CifsError, - smb->Flags, smb->Flags2, smb->Mid, smb->Pid); - cifs_dbg(VFS, "smb buf %p len %u\n", smb, - server->ops->calc_smb_size(smb)); + cifs_dbg(VFS, "Cmd: %d Err: 0x%x Flags: 0x%x Flgs2: 0x%x Mid: %d Pid: %d Wct: %d\n", + smb->Command, smb->Status.CifsError, smb->Flags, + smb->Flags2, smb->Mid, smb->Pid, smb->WordCount); + if (!server->ops->check_message(buf, server->total_read, server)) { + cifs_dbg(VFS, "smb buf %p len %u\n", smb, + server->ops->calc_smb_size(smb)); + } #endif /* CONFIG_CIFS_DEBUG2 */ } -- 2.43.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] smb: client: fix potential OOB in smb2_dump_detail() 2023-12-16 4:10 [PATCH 1/2] smb: client: fix potential OOB in cifs_dump_detail() Paulo Alcantara @ 2023-12-16 4:10 ` Paulo Alcantara 2023-12-19 14:34 ` Maximilian Heyne 2023-12-19 16:10 ` [PATCH v2] " Paulo Alcantara 0 siblings, 2 replies; 5+ messages in thread From: Paulo Alcantara @ 2023-12-16 4:10 UTC (permalink / raw) To: smfrench; +Cc: linux-cifs, Paulo Alcantara, j51569436 Validate SMB message with ->check_message() before calling ->calc_smb_size(). This fixes CVE-2023-6610. Reported-by: j51569436@gmail.com Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218219 Signed-off-by: Paulo Alcantara <pc@manguebit.com> --- fs/smb/client/smb2ops.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c index 62b0a8df867b..66b310208545 100644 --- a/fs/smb/client/smb2ops.c +++ b/fs/smb/client/smb2ops.c @@ -403,8 +403,10 @@ smb2_dump_detail(void *buf, struct TCP_Server_Info *server) cifs_server_dbg(VFS, "Cmd: %d Err: 0x%x Flags: 0x%x Mid: %llu Pid: %d\n", shdr->Command, shdr->Status, shdr->Flags, shdr->MessageId, shdr->Id.SyncId.ProcessId); - cifs_server_dbg(VFS, "smb buf %p len %u\n", buf, - server->ops->calc_smb_size(buf)); + if (!server->ops->check_message(buf, server->total_read, server)) { + cifs_server_dbg(VFS, "smb buf %p len %u\n", buf, + server->ops->calc_smb_size(buf)); + } #endif } -- 2.43.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] smb: client: fix potential OOB in smb2_dump_detail() 2023-12-16 4:10 ` [PATCH 2/2] smb: client: fix potential OOB in smb2_dump_detail() Paulo Alcantara @ 2023-12-19 14:34 ` Maximilian Heyne 2023-12-19 14:57 ` Paulo Alcantara 2023-12-19 16:10 ` [PATCH v2] " Paulo Alcantara 1 sibling, 1 reply; 5+ messages in thread From: Maximilian Heyne @ 2023-12-19 14:34 UTC (permalink / raw) To: Paulo Alcantara; +Cc: smfrench, linux-cifs, j51569436 On Sat, Dec 16, 2023 at 01:10:05AM -0300, Paulo Alcantara wrote: > Validate SMB message with ->check_message() before calling > ->calc_smb_size(). > > This fixes CVE-2023-6610. > > Reported-by: j51569436@gmail.com > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218219 > Signed-off-by: Paulo Alcantara <pc@manguebit.com> > --- > fs/smb/client/smb2ops.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c > index 62b0a8df867b..66b310208545 100644 > --- a/fs/smb/client/smb2ops.c > +++ b/fs/smb/client/smb2ops.c > @@ -403,8 +403,10 @@ smb2_dump_detail(void *buf, struct TCP_Server_Info *server) > cifs_server_dbg(VFS, "Cmd: %d Err: 0x%x Flags: 0x%x Mid: %llu Pid: %d\n", > shdr->Command, shdr->Status, shdr->Flags, shdr->MessageId, > shdr->Id.SyncId.ProcessId); > - cifs_server_dbg(VFS, "smb buf %p len %u\n", buf, > - server->ops->calc_smb_size(buf)); > + if (!server->ops->check_message(buf, server->total_read, server)) { Why is this check negated? Sorry for this stupid question. I'm not familiar with this code but only stumbled upon this commit due to a CVE. > + cifs_server_dbg(VFS, "smb buf %p len %u\n", buf, > + server->ops->calc_smb_size(buf)); > + } > #endif > } > > -- > 2.43.0 > Amazon Development Center Germany GmbH Krausenstr. 38 10117 Berlin Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B Sitz: Berlin Ust-ID: DE 289 237 879 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] smb: client: fix potential OOB in smb2_dump_detail() 2023-12-19 14:34 ` Maximilian Heyne @ 2023-12-19 14:57 ` Paulo Alcantara 0 siblings, 0 replies; 5+ messages in thread From: Paulo Alcantara @ 2023-12-19 14:57 UTC (permalink / raw) To: Maximilian Heyne; +Cc: smfrench, linux-cifs, j51569436 Maximilian Heyne <mheyne@amazon.de> writes: > On Sat, Dec 16, 2023 at 01:10:05AM -0300, Paulo Alcantara wrote: >> Validate SMB message with ->check_message() before calling >> ->calc_smb_size(). >> >> This fixes CVE-2023-6610. >> >> Reported-by: j51569436@gmail.com >> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218219 >> Signed-off-by: Paulo Alcantara <pc@manguebit.com> >> --- >> fs/smb/client/smb2ops.c | 6 ++++-- >> 1 file changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c >> index 62b0a8df867b..66b310208545 100644 >> --- a/fs/smb/client/smb2ops.c >> +++ b/fs/smb/client/smb2ops.c >> @@ -403,8 +403,10 @@ smb2_dump_detail(void *buf, struct TCP_Server_Info *server) >> cifs_server_dbg(VFS, "Cmd: %d Err: 0x%x Flags: 0x%x Mid: %llu Pid: %d\n", >> shdr->Command, shdr->Status, shdr->Flags, shdr->MessageId, >> shdr->Id.SyncId.ProcessId); >> - cifs_server_dbg(VFS, "smb buf %p len %u\n", buf, >> - server->ops->calc_smb_size(buf)); >> + if (!server->ops->check_message(buf, server->total_read, server)) { > > Why is this check negated? Sorry for this stupid question. I'm not > familiar with this code but only stumbled upon this commit due to a > CVE. Because smb2_check_message() returns 0 for a valid SMB response and then it would be safe to call ->calc_smb_size() as we know the header has a valid Command (< NUMBER_OF_SMB2_COMMANDS). ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2] smb: client: fix potential OOB in smb2_dump_detail() 2023-12-16 4:10 ` [PATCH 2/2] smb: client: fix potential OOB in smb2_dump_detail() Paulo Alcantara 2023-12-19 14:34 ` Maximilian Heyne @ 2023-12-19 16:10 ` Paulo Alcantara 1 sibling, 0 replies; 5+ messages in thread From: Paulo Alcantara @ 2023-12-19 16:10 UTC (permalink / raw) To: smfrench; +Cc: linux-cifs, Paulo Alcantara, j51569436 Validate SMB message with ->check_message() before calling ->calc_smb_size(). This fixes CVE-2023-6610. Reported-by: j51569436@gmail.com Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218219 Signed-off-by: Paulo Alcantara <pc@manguebit.com> --- Steve, I tried to make as little changes as possible for v1 to make backporting easier, but unfortunately we still need to handle "@len < @pdu_size" case in smb2_check_message() otherwise we could call smb2_calc_size() with an invalid command, too. fs/smb/client/smb2misc.c | 30 +++++++++++++++--------------- fs/smb/client/smb2ops.c | 6 ++++-- 2 files changed, 19 insertions(+), 17 deletions(-) diff --git a/fs/smb/client/smb2misc.c b/fs/smb/client/smb2misc.c index e20b4354e703..82b84a4941dd 100644 --- a/fs/smb/client/smb2misc.c +++ b/fs/smb/client/smb2misc.c @@ -173,6 +173,21 @@ smb2_check_message(char *buf, unsigned int len, struct TCP_Server_Info *server) } mid = le64_to_cpu(shdr->MessageId); + if (check_smb2_hdr(shdr, mid)) + return 1; + + if (shdr->StructureSize != SMB2_HEADER_STRUCTURE_SIZE) { + cifs_dbg(VFS, "Invalid structure size %u\n", + le16_to_cpu(shdr->StructureSize)); + return 1; + } + + command = le16_to_cpu(shdr->Command); + if (command >= NUMBER_OF_SMB2_COMMANDS) { + cifs_dbg(VFS, "Invalid SMB2 command %d\n", command); + return 1; + } + if (len < pdu_size) { if ((len >= hdr_size) && (shdr->Status != 0)) { @@ -193,21 +208,6 @@ smb2_check_message(char *buf, unsigned int len, struct TCP_Server_Info *server) return 1; } - if (check_smb2_hdr(shdr, mid)) - return 1; - - if (shdr->StructureSize != SMB2_HEADER_STRUCTURE_SIZE) { - cifs_dbg(VFS, "Invalid structure size %u\n", - le16_to_cpu(shdr->StructureSize)); - return 1; - } - - command = le16_to_cpu(shdr->Command); - if (command >= NUMBER_OF_SMB2_COMMANDS) { - cifs_dbg(VFS, "Invalid SMB2 command %d\n", command); - return 1; - } - if (smb2_rsp_struct_sizes[command] != pdu->StructureSize2) { if (command != SMB2_OPLOCK_BREAK_HE && (shdr->Status == 0 || pdu->StructureSize2 != SMB2_ERROR_STRUCTURE_SIZE2_LE)) { diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c index 62b0a8df867b..66b310208545 100644 --- a/fs/smb/client/smb2ops.c +++ b/fs/smb/client/smb2ops.c @@ -403,8 +403,10 @@ smb2_dump_detail(void *buf, struct TCP_Server_Info *server) cifs_server_dbg(VFS, "Cmd: %d Err: 0x%x Flags: 0x%x Mid: %llu Pid: %d\n", shdr->Command, shdr->Status, shdr->Flags, shdr->MessageId, shdr->Id.SyncId.ProcessId); - cifs_server_dbg(VFS, "smb buf %p len %u\n", buf, - server->ops->calc_smb_size(buf)); + if (!server->ops->check_message(buf, server->total_read, server)) { + cifs_server_dbg(VFS, "smb buf %p len %u\n", buf, + server->ops->calc_smb_size(buf)); + } #endif } -- 2.43.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-12-19 16:10 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-12-16 4:10 [PATCH 1/2] smb: client: fix potential OOB in cifs_dump_detail() Paulo Alcantara 2023-12-16 4:10 ` [PATCH 2/2] smb: client: fix potential OOB in smb2_dump_detail() Paulo Alcantara 2023-12-19 14:34 ` Maximilian Heyne 2023-12-19 14:57 ` Paulo Alcantara 2023-12-19 16:10 ` [PATCH v2] " Paulo Alcantara
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox