Skip to contentSkip to conversation

cifs: using strlcpy instead of strncpy

PATCH Applied Zhao Hongjiang 3 participants 4 messages Jun 20, 2013 – Jun 21, 2013 2 messages not in the archive

Subsystems (2)
SMB client (CIFS) Kernel SMB3 server (ksmbd)
lore
Share & tools
lore

Zhao Hongjiang

author
#0 lore raw

PATCH 1/1 cifs: using strlcpy instead of strncpy

for NUL terminated string, need alway set '\0' in the end.

Signed-off-by Zhao Hongjiang <[email protected]>
---
 fs/cifs/connect.c |    2 +-
 fs/cifs/smb2pdu.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
fs/cifs/connect.c +1 -1
index e3bc39b..2ea78a3 100644
@@ -3738,7 +3738,7 @@ CIFSTCon(const unsigned int xid, struct cifs_ses *ses,
 		}
 		bcc_ptr += length + 1;
 		bytes_left -= (length + 1);
-		strncpy(tcon->treeName, tree, MAX_TREE_SIZE);
+		strlcpy(tcon->treeName, tree, sizeof(tcon->treeName));
 
 		/* mostly informational -- no need to fail on error here */
 		kfree(tcon->nativeFileSystem);
fs/cifs/smb2pdu.c +1 -1
index 2b95ce2..99bf768 100644
@@ -792,7 +792,7 @@ SMB2_tcon(const unsigned int xid, struct cifs_ses *ses, const char *tree,
 	tcon->tidStatus = CifsGood;
 	tcon->need_reconnect = false;
 	tcon->tid = rsp->hdr.TreeId;
-	strncpy(tcon->treeName, tree, MAX_TREE_SIZE);
+	strlcpy(tcon->treeName, tree, sizeof(tcon->treeName));
 
 	if ((rsp->Capabilities & SMB2_SHARE_CAP_DFS) &&
 	    ((tcon->share_flags & SHI1005_FLAGS_DFS) == 0))
Signature · 1 line
1.7.1
Acked-by
#2 lore raw
In reply to a message not in the archive
Quoted 26 lines · On Thu, 20 Jun 2013 18:56:22 +0800 Zhao Hongjiang…

for NUL terminated string, need alway set '\0' in the end.

Signed-off-by Zhao Hongjiang <[email protected]>
---
 fs/cifs/connect.c |    2 +-
 fs/cifs/smb2pdu.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
fs/cifs/connect.c +1 -1
index e3bc39b..2ea78a3 100644
@@ -3738,7 +3738,7 @@ CIFSTCon(const unsigned int xid, struct cifs_ses *ses,
 		}
 		bcc_ptr += length + 1;
 		bytes_left -= (length + 1);
-		strncpy(tcon->treeName, tree, MAX_TREE_SIZE);
+		strlcpy(tcon->treeName, tree, sizeof(tcon->treeName));
 
 		/* mostly informational -- no need to fail on error here */
 		kfree(tcon->nativeFileSystem);
fs/cifs/smb2pdu.c +0 -0
index 2b95ce2..99bf768 100644

fs/cifs/smb2pdu.c SMB2_tcon(const unsigned int xid, struct cifs_ses *ses, const char *tree, · line 798

Quoted 8 lines
fs/cifs/smb2pdu.c +1 -1
@@ -792,7 +792,7 @@ SMB2_tcon(const unsigned int xid, struct cifs_ses *ses, const char *tree,
 	tcon->tidStatus = CifsGood;
 	tcon->need_reconnect = false;
 	tcon->tid = rsp->hdr.TreeId;
-	strncpy(tcon->treeName, tree, MAX_TREE_SIZE);
+	strlcpy(tcon->treeName, tree, sizeof(tcon->treeName));
 
 	if ((rsp->Capabilities & SMB2_SHARE_CAP_DFS) &&
 	    ((tcon->share_flags & SHI1005_FLAGS_DFS) == 0))

Seems like a reasonable cleanup, but if you're going to do that then it might not hurt to convert most of the places where MAX_TREE_SIZE is used to use sizeof(tcon->treeName) instead.

Either way...

Acked-by Jeff Layton <[email protected]>
#5 lore raw
In reply to a message not in the archive

I actually prefer his change. He potentially fixes a bug changing
these two of six places where it is used and the other four places MAX_TREE_SIZE is used are clear enough either way.

Quoted 38 lines · Zhao Hongjiang

for NUL terminated string, need alway set '\0' in the end.

Signed-off-by Zhao Hongjiang <[email protected]>
---
 fs/cifs/connect.c |    2 +-
 fs/cifs/smb2pdu.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
fs/cifs/connect.c +1 -1
index e3bc39b..2ea78a3 100644
@@ -3738,7 +3738,7 @@ CIFSTCon(const unsigned int xid, struct cifs_ses *ses,
                }
                bcc_ptr += length + 1;
                bytes_left -= (length + 1);
-               strncpy(tcon->treeName, tree, MAX_TREE_SIZE);
+               strlcpy(tcon->treeName, tree, sizeof(tcon->treeName));
 
                /* mostly informational -- no need to fail on error here */
                kfree(tcon->nativeFileSystem);
fs/cifs/smb2pdu.c +1 -2
index 2b95ce2..99bf768 100644
@@ -792,7 +792,7 @@ SMB2_tcon(const unsigned int xid, struct cifs_ses *ses, const char *tree,
        tcon->tidStatus = CifsGood;
        tcon->need_reconnect = false;
        tcon->tid = rsp->hdr.TreeId;
-       strncpy(tcon->treeName, tree, MAX_TREE_SIZE);
+       strlcpy(tcon->treeName, tree, sizeof(tcon->treeName));
 
        if ((rsp->Capabilities & SMB2_SHARE_CAP_DFS) &&
            ((tcon->share_flags & SHI1005_FLAGS_DFS) == 0))
--

1.7.1

Quoted 4 lines

--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

--
Thanks,

Steve

Applied
#4 lore raw
In reply to a message not in the archive

merged into for-next cifs-2.6.git

Quoted 45 lines · Jeff Layton
Quoted 26 lines · On Thu, 20 Jun 2013 18:56:22 +0800 Zhao Hongjiang…

for NUL terminated string, need alway set '\0' in the end.

Signed-off-by Zhao Hongjiang <[email protected]>
---
 fs/cifs/connect.c |    2 +-
 fs/cifs/smb2pdu.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
fs/cifs/connect.c +1 -1
index e3bc39b..2ea78a3 100644
@@ -3738,7 +3738,7 @@ CIFSTCon(const unsigned int xid, struct cifs_ses *ses,
              }
              bcc_ptr += length + 1;
              bytes_left -= (length + 1);
-             strncpy(tcon->treeName, tree, MAX_TREE_SIZE);
+             strlcpy(tcon->treeName, tree, sizeof(tcon->treeName));
 
              /* mostly informational -- no need to fail on error here */
              kfree(tcon->nativeFileSystem);
fs/cifs/smb2pdu.c +0 -0
index 2b95ce2..99bf768 100644

fs/cifs/smb2pdu.c SMB2_tcon(const unsigned int xid, struct cifs_ses *ses, const char *tree, · line 798

Quoted 8 lines
fs/cifs/smb2pdu.c +1 -1
@@ -792,7 +792,7 @@ SMB2_tcon(const unsigned int xid, struct cifs_ses *ses, const char *tree,
      tcon->tidStatus = CifsGood;
      tcon->need_reconnect = false;
      tcon->tid = rsp->hdr.TreeId;
-     strncpy(tcon->treeName, tree, MAX_TREE_SIZE);
+     strlcpy(tcon->treeName, tree, sizeof(tcon->treeName));
 
      if ((rsp->Capabilities & SMB2_SHARE_CAP_DFS) &&
          ((tcon->share_flags & SHI1005_FLAGS_DFS) == 0))

Seems like a reasonable cleanup, but if you're going to do that then it might not hurt to convert most of the places where MAX_TREE_SIZE is used to use sizeof(tcon->treeName) instead.

Either way...

Acked-by Jeff Layton <[email protected]>
Signature · 3 lines
Thanks,

Steve