All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jim Meyering <jim-Oxw1nKZkIVjk1uMJSBkQmQ@public.gmane.org>
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: Jim Meyering <meyering-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>,
	Steve French <sfrench-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>,
	linux-cifs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	samba-technical-w/Ol4Ecudpl8XjKLYN78aQ@public.gmane.org
Subject: [PATCH] cifs: remove misleading strncpy: each name has length < 16
Date: Mon, 20 Aug 2012 18:55:24 +0200	[thread overview]
Message-ID: <1345481724-30108-6-git-send-email-jim@meyering.net> (raw)
In-Reply-To: <1345481724-30108-1-git-send-email-jim-Oxw1nKZkIVjk1uMJSBkQmQ@public.gmane.org>

From: Jim Meyering <meyering-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Each of the protocols[i].name strings (statically declared above)
has length less than 16, so this use of strncpy is misleading:
  strncpy(pSMB->DialectsArray+count, protocols[i].name, 16);
Besides, if a new name were added with length N >= 16, the existing
strncpy-using code would be buggy, creating a ->DialectsArray buffer
containing N-16+1 unset bytes where the NUL terminator should have
been.  Instead, traverse the name only once go get its length,
use a BUG_ON assertion to enforce the length restriction
and use memcpy to perform the copy.

Signed-off-by: Jim Meyering <meyering-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 fs/cifs/cifssmb.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
index 074923c..16a9018 100644
--- a/fs/cifs/cifssmb.c
+++ b/fs/cifs/cifssmb.c
@@ -441,8 +441,10 @@ CIFSSMBNegotiate(const unsigned int xid, struct cifs_ses *ses)

 	count = 0;
 	for (i = 0; i < CIFS_NUM_PROT; i++) {
-		strncpy(pSMB->DialectsArray+count, protocols[i].name, 16);
-		count += strlen(protocols[i].name) + 1;
+		size_t len = strlen(protocols[i].name);
+		BUG_ON(len >= 16);
+		memcpy(pSMB->DialectsArray+count, protocols[i].name, len + 1);
+		count += len + 1;
 		/* null at end of source and target buffers anyway */
 	}
 	inc_rfc1001_len(pSMB, count);
-- 
1.7.12

WARNING: multiple messages have this Message-ID (diff)
From: Jim Meyering <jim@meyering.net>
To: linux-kernel@vger.kernel.org
Cc: Jim Meyering <meyering@redhat.com>,
	Steve French <sfrench@samba.org>,
	linux-cifs@vger.kernel.org, samba-technical@lists.samba.org
Subject: [PATCH] cifs: remove misleading strncpy: each name has length < 16
Date: Mon, 20 Aug 2012 18:55:24 +0200	[thread overview]
Message-ID: <1345481724-30108-6-git-send-email-jim@meyering.net> (raw)
In-Reply-To: <1345481724-30108-1-git-send-email-jim@meyering.net>

From: Jim Meyering <meyering@redhat.com>

Each of the protocols[i].name strings (statically declared above)
has length less than 16, so this use of strncpy is misleading:
  strncpy(pSMB->DialectsArray+count, protocols[i].name, 16);
Besides, if a new name were added with length N >= 16, the existing
strncpy-using code would be buggy, creating a ->DialectsArray buffer
containing N-16+1 unset bytes where the NUL terminator should have
been.  Instead, traverse the name only once go get its length,
use a BUG_ON assertion to enforce the length restriction
and use memcpy to perform the copy.

Signed-off-by: Jim Meyering <meyering@redhat.com>
---
 fs/cifs/cifssmb.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
index 074923c..16a9018 100644
--- a/fs/cifs/cifssmb.c
+++ b/fs/cifs/cifssmb.c
@@ -441,8 +441,10 @@ CIFSSMBNegotiate(const unsigned int xid, struct cifs_ses *ses)

 	count = 0;
 	for (i = 0; i < CIFS_NUM_PROT; i++) {
-		strncpy(pSMB->DialectsArray+count, protocols[i].name, 16);
-		count += strlen(protocols[i].name) + 1;
+		size_t len = strlen(protocols[i].name);
+		BUG_ON(len >= 16);
+		memcpy(pSMB->DialectsArray+count, protocols[i].name, len + 1);
+		count += len + 1;
 		/* null at end of source and target buffers anyway */
 	}
 	inc_rfc1001_len(pSMB, count);
-- 
1.7.12


  parent reply	other threads:[~2012-08-20 16:55 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-20 16:55 a few strncpy-related patches Jim Meyering
2012-08-20 16:55 ` [PATCH] ACPI: remove unwarranted use of strncpy Jim Meyering
2012-08-20 16:55 ` [PATCH] fs/9p: avoid debug OOPS when reading a long symlink Jim Meyering
2012-08-21  7:20   ` [PATCHv2] " Jim Meyering
2012-08-20 16:55 ` [PATCH] kmemleak: avoid buffer overrun: NUL-terminate strncpy-copied command Jim Meyering
2012-08-20 16:55   ` Jim Meyering
2012-08-24 10:27   ` Catalin Marinas
2012-08-24 10:27     ` Catalin Marinas
2012-08-24 11:23     ` Jim Meyering
2012-08-24 11:23       ` Jim Meyering
2012-08-28 20:24       ` Dan Carpenter
2012-08-28 20:24         ` Dan Carpenter
2012-08-29  6:28         ` Jim Meyering
2012-08-29  6:28           ` Jim Meyering
2012-08-29 15:56           ` Dan Carpenter
2012-08-29 15:56             ` Dan Carpenter
2012-08-20 16:55 ` [PATCH] bfa: avoid buffer overrun for 12-byte model name Jim Meyering
2012-08-20 19:42   ` Krishna Gudipati
2012-08-20 20:38     ` Jim Meyering
2012-08-20 20:38       ` Jim Meyering
2012-10-14  7:53       ` Jim Meyering
2012-10-14  7:53         ` Jim Meyering
2012-10-14  8:20         ` Jim Meyering
2012-10-14  8:20           ` Jim Meyering
2012-12-24  7:43           ` Vijay Mohan Guvva
     [not found] ` <1345481724-30108-1-git-send-email-jim-Oxw1nKZkIVjk1uMJSBkQmQ@public.gmane.org>
2012-08-20 16:55   ` Jim Meyering [this message]
2012-08-20 16:55     ` [PATCH] cifs: remove misleading strncpy: each name has length < 16 Jim Meyering
2012-08-20 17:36     ` Bastien ROUCARIES
     [not found]       ` <CAE2SPAbBmRov9qK2HiBQBQXaZpfJ8pmZJ-PL18FEyoZhzDza4A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2012-08-20 18:40         ` Jim Meyering
2012-08-20 18:40           ` Jim Meyering
     [not found]     ` <1345481724-30108-6-git-send-email-jim-Oxw1nKZkIVjk1uMJSBkQmQ@public.gmane.org>
2012-08-20 18:41       ` Jim Meyering
2012-08-20 18:41         ` Jim Meyering
2012-08-20 20:18 ` a few strncpy-related patches Andi Kleen
2012-08-20 20:47   ` Jim Meyering

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1345481724-30108-6-git-send-email-jim@meyering.net \
    --to=jim-oxw1nkzkivjk1umjsbkqmq@public.gmane.org \
    --cc=linux-cifs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=meyering-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
    --cc=samba-technical-w/Ol4Ecudpl8XjKLYN78aQ@public.gmane.org \
    --cc=sfrench-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.