On Fri, 29 Sep 2006 13:09:13 +1000
NeilBrown <[email protected]> wrote:
>
> From: Manoj Naik <[email protected]>
>
> Define FS locations structures, some functions to manipulate them, and add
> code to parse FS locations in downcall and add to the exports structure.
>
> ...
>
> +#ifdef CONFIG_NFSD_V4
> +
> +static int
> +fsloc_parse(char **mesg, char *buf, struct nfsd4_fs_locations *fsloc)
> +{
> + int len;
> + int migrated, i, err;
> +
> + len = qword_get(mesg, buf, PAGE_SIZE);
> + if (len != 5 || memcmp(buf, "fsloc", 5))
> + return 0;
> +
> + /* listsize */
> + err = get_int(mesg, &fsloc->locations_count);
> + if (err)
> + return err;
> + if (fsloc->locations_count < 0)
this is unsigned.
> + return -EINVAL;
> + if (fsloc->locations_count == 0)
> + return 0;
> +
> + fsloc->locations = kzalloc(fsloc->locations_count
> + * sizeof(struct nfsd4_fs_location), GFP_KERNEL);
This is subject to multiplication overflow. If it's a privileged operation
and isn't dependent on stuff coming in over the wire then ok..
> + if (!fsloc->locations)
> + return -ENOMEM;
> + for (i=0; i < fsloc->locations_count; i++) {
> + /* colon separated host list */
> + err = -EINVAL;
> + len = qword_get(mesg, buf, PAGE_SIZE);
> + if (len <= 0)
> + goto out_free_all;
> + err = -ENOMEM;
> + fsloc->locations[i].hosts = kstrdup(buf, GFP_KERNEL);
> + if (!fsloc->locations[i].hosts)
> + goto out_free_all;
> + err = -EINVAL;
> + /* slash separated path component list */
> + len = qword_get(mesg, buf, PAGE_SIZE);
> + if (len <= 0)
> + goto out_free_all;
> + err = -ENOMEM;
> + fsloc->locations[i].path = kstrdup(buf, GFP_KERNEL);
> + if (!fsloc->locations[i].path)
> + goto out_free_all;
> + }
> + /* migrated */
> + err = get_int(mesg, &migrated);
> + if (err)
> + goto out_free_all;
> + err = -EINVAL;
> + if (migrated < 0 || migrated > 1)
> + goto out_free_all;
> + fsloc->migrated = migrated;
> + return 0;
> +out_free_all:
> + nfsd4_fslocs_free(fsloc);
This call to nfsd4_fslocs_free() can end up kfreeing members of
fsloc->locations[] which haven't been initialised here. Are we sure the
caller set them all to zero?
> + return err;
> +}
> +
> +#else /* CONFIG_NFSD_V4 */
> +static int fsloc_parse(char **, char *, struct svc_export *) { return 0; }
static inline
This has a different prototype from the other version of fsloc_parse()
This ain't C++ - function arguments need identifiers as well as types.
Someone needs to read Documentation/SubmitChecklist..
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
