Commit b32a09db authored by Markus Armbruster's avatar Markus Armbruster Committed by Eric Van Hensbergen
Browse files

add match_strlcpy() us it to make v9fs make uname and remotename parsing more robust



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: default avatarMarkus Armbruster <armbru@redhat.com>
Cc: Latchesar Ionkov <lucho@ionkov.net>
Cc: Jim Meyering <meyering@redhat.com>
Cc: "Randy.Dunlap" <rdunlap@xenotime.net>
Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
Signed-off-by: default avatarEric Van Hensbergen <ericvh@gmail.com>
parent dd286422
...@@ -125,10 +125,10 @@ static void v9fs_parse_options(struct v9fs_session_info *v9ses) ...@@ -125,10 +125,10 @@ static void v9fs_parse_options(struct v9fs_session_info *v9ses)
v9ses->afid = option; v9ses->afid = option;
break; break;
case Opt_uname: case Opt_uname:
match_strcpy(v9ses->uname, &args[0]); match_strlcpy(v9ses->uname, &args[0], PATH_MAX);
break; break;
case Opt_remotename: case Opt_remotename:
match_strcpy(v9ses->aname, &args[0]); match_strlcpy(v9ses->aname, &args[0], PATH_MAX);
break; break;
case Opt_nodevmap: case Opt_nodevmap:
v9ses->nodev = 1; v9ses->nodev = 1;
......
...@@ -29,5 +29,5 @@ int match_token(char *, match_table_t table, substring_t args[]); ...@@ -29,5 +29,5 @@ int match_token(char *, match_table_t table, substring_t args[]);
int match_int(substring_t *, int *result); int match_int(substring_t *, int *result);
int match_octal(substring_t *, int *result); int match_octal(substring_t *, int *result);
int match_hex(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 *); char *match_strdup(const substring_t *);
...@@ -182,18 +182,25 @@ int match_hex(substring_t *s, int *result) ...@@ -182,18 +182,25 @@ int match_hex(substring_t *s, int *result)
} }
/** /**
* match_strcpy: - copies the characters from a substring_t to a string * match_strlcpy: - Copy the characters from a substring_t to a sized buffer
* @to: string to copy characters to. * @dest: where to copy to
* @s: &substring_t to copy * @src: &substring_t to copy
* @size: size of destination buffer
* *
* Description: Copies the set of characters represented by the given * Description: Copy the characters in &substring_t @src to the
* &substring_t @s to the c-style string @to. Caller guarantees that @to is * c-style string @dest. Copy no more than @size - 1 characters, plus
* large enough to hold the characters of @s. * 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); size_t ret = src->to - src->from;
to[s->to - s->from] = '\0';
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) ...@@ -206,9 +213,10 @@ void match_strcpy(char *to, const substring_t *s)
*/ */
char *match_strdup(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) if (p)
match_strcpy(p, s); match_strlcpy(p, s, sz);
return p; return p;
} }
...@@ -216,5 +224,5 @@ EXPORT_SYMBOL(match_token); ...@@ -216,5 +224,5 @@ EXPORT_SYMBOL(match_token);
EXPORT_SYMBOL(match_int); EXPORT_SYMBOL(match_int);
EXPORT_SYMBOL(match_octal); EXPORT_SYMBOL(match_octal);
EXPORT_SYMBOL(match_hex); EXPORT_SYMBOL(match_hex);
EXPORT_SYMBOL(match_strcpy); EXPORT_SYMBOL(match_strlcpy);
EXPORT_SYMBOL(match_strdup); EXPORT_SYMBOL(match_strdup);
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment