linux-cifs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH][CIFS] Clarify SMB1 code for rename open file
@ 2021-07-07 18:40 Steve French
  2021-07-07 21:15 ` Paulo Alcantara
  0 siblings, 1 reply; 2+ messages in thread
From: Steve French @ 2021-07-07 18:40 UTC (permalink / raw)
  To: CIFS

[-- Attachment #1: Type: text/plain, Size: 1385 bytes --]

And one more trivial coverity issue related patch ...
(with fewer old issues like this, in the future it will be easier
to spot important new ones that tools like this report)

Coverity also complains about the way we calculate the offset
(starting from the address of a 4 byte array within the
header structure rather than from the beginning of the struct
plus 4 bytes) for SMB1 RenameOpenFile. This changeset
doesn't change the address but makes it slightly clearer.

Addresses-Coverity: 711521 ("Out of bounds write")
Signed-off-by: Steve French <stfrench@microsoft.com>
---
 fs/cifs/cifssmb.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
index 0863238ddd20..16bd4cf3bceb 100644
--- a/fs/cifs/cifssmb.c
+++ b/fs/cifs/cifssmb.c
@@ -2767,7 +2767,8 @@ int CIFSSMBRenameOpenFile(const unsigned int
xid, struct cifs_tcon *pTcon,
  param_offset = offsetof(struct smb_com_transaction2_sfi_req, Fid) - 4;
  offset = param_offset + params;

- data_offset = (char *) (&pSMB->hdr.Protocol) + offset;
+ /* SMB offsets are from the beginning of SMB which is 4 bytes in,
after RFC1001 field */
+ data_offset = (char *)(pSMB) + offset + 4;
  rename_info = (struct set_file_rename *) data_offset;
  pSMB->MaxParameterCount = cpu_to_le16(2);
  pSMB->MaxDataCount = cpu_to_le16(1000); /* BB find max SMB from sess */

-- 
Thanks,

Steve

[-- Attachment #2: 0001-CIFS-Clarify-SMB1-code-for-rename-open-file.patch --]
[-- Type: text/x-patch, Size: 1416 bytes --]

From a2c6b5f95226d58f85864f5a30e892b5ec885bd6 Mon Sep 17 00:00:00 2001
From: Steve French <stfrench@microsoft.com>
Date: Wed, 7 Jul 2021 13:34:47 -0500
Subject: [PATCH] CIFS: Clarify SMB1 code for rename open file

Coverity also complains about the way we calculate the offset
(starting from the address of a 4 byte array within the
header structure rather than from the beginning of the struct
plus 4 bytes) for SMB1 RenameOpenFile. This changeset
doesn't change the address but makes it slightly clearer.

Addresses-Coverity: 711521 ("Out of bounds write")
Signed-off-by: Steve French <stfrench@microsoft.com>
---
 fs/cifs/cifssmb.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
index 0863238ddd20..16bd4cf3bceb 100644
--- a/fs/cifs/cifssmb.c
+++ b/fs/cifs/cifssmb.c
@@ -2767,7 +2767,8 @@ int CIFSSMBRenameOpenFile(const unsigned int xid, struct cifs_tcon *pTcon,
 	param_offset = offsetof(struct smb_com_transaction2_sfi_req, Fid) - 4;
 	offset = param_offset + params;
 
-	data_offset = (char *) (&pSMB->hdr.Protocol) + offset;
+	/* SMB offsets are from the beginning of SMB which is 4 bytes in, after RFC1001 field */
+	data_offset = (char *)(pSMB) + offset + 4;
 	rename_info = (struct set_file_rename *) data_offset;
 	pSMB->MaxParameterCount = cpu_to_le16(2);
 	pSMB->MaxDataCount = cpu_to_le16(1000); /* BB find max SMB from sess */
-- 
2.30.2


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH][CIFS] Clarify SMB1 code for rename open file
  2021-07-07 18:40 [PATCH][CIFS] Clarify SMB1 code for rename open file Steve French
@ 2021-07-07 21:15 ` Paulo Alcantara
  0 siblings, 0 replies; 2+ messages in thread
From: Paulo Alcantara @ 2021-07-07 21:15 UTC (permalink / raw)
  To: Steve French, CIFS

Steve French <smfrench@gmail.com> writes:

> And one more trivial coverity issue related patch ...
> (with fewer old issues like this, in the future it will be easier
> to spot important new ones that tools like this report)
>
> Coverity also complains about the way we calculate the offset
> (starting from the address of a 4 byte array within the
> header structure rather than from the beginning of the struct
> plus 4 bytes) for SMB1 RenameOpenFile. This changeset
> doesn't change the address but makes it slightly clearer.
>
> Addresses-Coverity: 711521 ("Out of bounds write")
> Signed-off-by: Steve French <stfrench@microsoft.com>
> ---
>  fs/cifs/cifssmb.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Paulo Alcantara (SUSE) <pc@cjr.nz>

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2021-07-07 21:15 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-07 18:40 [PATCH][CIFS] Clarify SMB1 code for rename open file Steve French
2021-07-07 21:15 ` Paulo Alcantara

This is a public inbox, see mirroring instructions
on how to clone and mirror all data and code used for this inbox