On 02/03/2011 08:59 PM, Trond Myklebust wrote: > On Thu, 2011-02-03 at 16:13 -0600, Rob Landley wrote: >> From: Rob Landley <rlandley@xxxxxxxxxxxxx> >> >> Make sure user string is null terminated before copying it. >> >> Signed-off-by: Rob Landley <rlandley@xxxxxxxxxxxxx> >> --- >> >> fs/nfs/super.c | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/fs/nfs/super.c b/fs/nfs/super.c >> index b68c860..0ad1255 100644 >> --- a/fs/nfs/super.c >> +++ b/fs/nfs/super.c >> @@ -1881,9 +1881,12 @@ static int nfs_validate_mount_data(void *options, >> >> if (!(data->flags & NFS_MOUNT_TCP)) >> args->nfs_server.protocol = XPRT_TRANSPORT_UDP; >> + /* Force null termination of data->hostname no matter what >> + user passed in. */ >> + args->namlen = data->namlen; >> + data->namlen = 0; >> /* N.B. caller will free nfs_server.hostname in all cases */ >> args->nfs_server.hostname = kstrdup(data->hostname, GFP_KERNEL); >> - args->namlen = data->namlen; >> args->bsize = data->bsize; >> >> if (data->flags & NFS_MOUNT_SECFLAVOUR) > > The "namlen" mount option bears absolutely no relation to the server > hostname AFAIK. I can't see how this patch makes sense... That field is right after the hostname in the struct, therefore if it's zeroed the data->hostname is guaranteed to be null terminated at the end of its nominal range. (A bit subtle, hence the comment.) This isn't necessarily the proper fix, but I don't see why the user can't feed in a "data" blob with an arbitrarily long hostname. This is user supplied data we're arbitrarily kstrdup()ing without checking it, right? (Is there a size limit on the option string? Does the range check on userspace pointers prevent it from being right _before_ something we shouldn't read and padding up to it?) Entirely possible this is benign and irrelevant, and you probably have to be root to trigger it, but it's the kind of security "huh" I thought I'd ping the list about rather than assuming my inability to figure out how to exploit it of the top of my head means there's nothing lurking. Rob -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
