From: Markus Armbruster <armbru@redhat.com>
To: linux-kernel@vger.kernel.org
Cc: Latchesar Ionkov <lucho@ionkov.net>,
Eric Van Hensbergen <ericvh@gmail.com>,
Jim Meyering <meyering@redhat.com>
Subject: [PATCH] Make v9fs uname and remotename parsing more robust
Date: Fri, 15 Feb 2008 18:54:34 +0100 [thread overview]
Message-ID: <87skzu5cmd.fsf@pike.pond.sub.org> (raw)
match_strcpy() is a somewhat creepy function: the caller needs to make
sure that the destination buffer is big enough, and when he screws up
or forgets, match_strcpy() happily overruns the buffer.
There's exactly one customer: v9fs_parse_options(). I believe it
currently can't overflow its buffer, but that's not exactly obvious.
The source string is a substing of the mount options. The kernel
silently truncates those to PAGE_SIZE bytes, including the terminating
zero. See compat_sys_mount() and do_mount().
The destination buffer is obtained from __getname(), which allocates
from name_cachep, which is initialized by vfs_caches_init() for size
PATH_MAX.
We're safe as long as PATH_MAX <= PAGE_SIZE. PATH_MAX is 4096. As
far as I know, the smallest PAGE_SIZE is also 4096.
Here's a patch that makes the code a bit more obviously correct. It
doesn't depend on PATH_MAX <= PAGE_SIZE.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
diff --git a/fs/9p/v9fs.c b/fs/9p/v9fs.c
index fbb12da..e42948b 100644
--- a/fs/9p/v9fs.c
+++ b/fs/9p/v9fs.c
@@ -135,10 +135,10 @@ static void v9fs_parse_options(struct v9fs_session_info *v9ses)
v9ses->trans = v9fs_match_trans(&args[0]);
break;
case Opt_uname:
- match_strcpy(v9ses->uname, &args[0]);
+ match_strlcpy(v9ses->uname, &args[0], PATH_MAX);
break;
case Opt_remotename:
- match_strcpy(v9ses->aname, &args[0]);
+ match_strlcpy(v9ses->aname, &args[0], PATH_MAX);
break;
case Opt_legacy:
v9ses->flags &= ~V9FS_EXTENDED;
diff --git a/include/linux/parser.h b/include/linux/parser.h
index 26b2bdf..7dcd050 100644
--- a/include/linux/parser.h
+++ b/include/linux/parser.h
@@ -29,5 +29,5 @@ int match_token(char *, match_table_t table, substring_t args[]);
int match_int(substring_t *, int *result);
int match_octal(substring_t *, int *result);
int match_hex(substring_t *, int *result);
-void match_strcpy(char *, const substring_t *);
+size_t match_strlcpy(char *, const substring_t *, size_t);
char *match_strdup(const substring_t *);
diff --git a/lib/parser.c b/lib/parser.c
index 703c8c1..4f0cbc0 100644
--- a/lib/parser.c
+++ b/lib/parser.c
@@ -182,18 +182,25 @@ int match_hex(substring_t *s, int *result)
}
/**
- * match_strcpy: - copies the characters from a substring_t to a string
- * @to: string to copy characters to.
- * @s: &substring_t to copy
+ * match_strlcpy: - Copy the characters from a substring_t to a sized buffer
+ * @dest: where to copy to
+ * @src: &substring_t to copy
+ * @size: size of destination buffer
*
- * Description: Copies the set of characters represented by the given
- * &substring_t @s to the c-style string @to. Caller guarantees that @to is
- * large enough to hold the characters of @s.
+ * Description: Copy the characters in &substring_t @src to the
+ * c-style string @dest. Copy no more than @size - 1 characters, plus
+ * the terminating NUL. Return length of @src.
*/
-void match_strcpy(char *to, const substring_t *s)
+size_t match_strlcpy(char *dest, const substring_t *src, size_t size)
{
- memcpy(to, s->from, s->to - s->from);
- to[s->to - s->from] = '\0';
+ size_t ret = src->to - src->from;
+
+ if (size) {
+ size_t len = ret >= size ? size - 1 : ret;
+ memcpy(dest, src->from, len);
+ dest[len] = '\0';
+ }
+ return ret;
}
/**
@@ -206,9 +213,10 @@ void match_strcpy(char *to, const substring_t *s)
*/
char *match_strdup(const substring_t *s)
{
- char *p = kmalloc(s->to - s->from + 1, GFP_KERNEL);
+ size_t sz = s->to - s->from + 1;
+ char *p = kmalloc(sz, GFP_KERNEL);
if (p)
- match_strcpy(p, s);
+ match_strlcpy(p, s, sz);
return p;
}
@@ -216,5 +224,5 @@ EXPORT_SYMBOL(match_token);
EXPORT_SYMBOL(match_int);
EXPORT_SYMBOL(match_octal);
EXPORT_SYMBOL(match_hex);
-EXPORT_SYMBOL(match_strcpy);
+EXPORT_SYMBOL(match_strlcpy);
EXPORT_SYMBOL(match_strdup);
next reply other threads:[~2008-02-15 17:54 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-02-15 17:54 Markus Armbruster [this message]
2008-02-22 15:57 ` [PATCH] 9p: Make " Markus Armbruster
2008-02-23 8:07 ` [PATCH] Make v9fs " Andrew Morton
2008-02-24 15:37 ` Eric Van Hensbergen
2008-02-24 22:17 ` Andrew Morton
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=87skzu5cmd.fsf@pike.pond.sub.org \
--to=armbru@redhat.com \
--cc=ericvh@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=lucho@ionkov.net \
--cc=meyering@redhat.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Powered by JetHome