| Message ID | 20231216041005.7948-2-pc@manguebit.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] smb: client: fix potential OOB in cifs_dump_detail() | expand |
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
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).
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 }
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(-)