* Shailabh Nagar ([EMAIL PROTECTED]) wrote:
> Index: ckrm-2611rc5/fs/rcfs/magic.c
> ===================================================================
> --- ckrm-2611rc5.orig/fs/rcfs/magic.c 2005-03-07 14:49:19.000000000 -0500
> +++ ckrm-2611rc5/fs/rcfs/magic.c 2005-03-18 16:50:58.313766048 -0500
> @@ -165,27 +165,24 @@ magic_write(struct file *file, const cha
> core = ri->core;
> if (!ckrm_is_core_valid(core))
> return -EINVAL;
> -
> if (count > MAX_INPUT_SIZE)
> return -EINVAL;
>
> - if (!access_ok(VERIFY_READ, buf, count))
> - return -EFAULT;
> -
> - down(&(ri->vfs_inode.i_sem));
> -
> - optbuf = kmalloc(MAX_INPUT_SIZE+1, GFP_KERNEL);
> - if (!optbuf) {
> - up(&(ri->vfs_inode.i_sem));
> + optbuf = kmalloc(count+1, GFP_KERNEL);
> + if (!optbuf)
> return -ENOMEM;
> - }
> - __copy_from_user(optbuf, buf, count);
> +
> + rc = copy_from_user(optbuf, buf, count);
> + if (rc)
> + goto out_optbuf;
I've lost context of the full patch. But, note, that copy_from_user
returns non-zero on failure in the form of the number of bytes that
couldn't be copied (i.e. not an error code). Typical return value would
then be -EFAULT.
> mkvalidstr(optbuf);
> +
> + down(&(ri->vfs_inode.i_sem));
> done = magic_parse(ri->mfdentry->d_name.name,
> - optbuf, &resname, &otherstr);
> + optbuf, &resname, &otherstr);
> if (!done) {
> printk(KERN_ERR "Error parsing data written to %s\n",
> - ri->mfdentry->d_name.name);
> + ri->mfdentry->d_name.name);
> goto out;
> }
> if (!strcmp(ri->mfdentry->d_name.name, RCFS_CONFIG_NAME)) {
> @@ -197,14 +194,15 @@ magic_write(struct file *file, const cha
> rc = func(core, resname, otherstr);
> if (rc) {
> printk(KERN_ERR "magic_write: %s: error\n",
> - ri->mfdentry->d_name.name);
> + ri->mfdentry->d_name.name);
> }
> }
> out:
> up(&(ri->vfs_inode.i_sem));
> - kfree(optbuf);
> kfree(otherstr);
> kfree(resname);
> +out_optbuf:
> + kfree(optbuf);
> return rc ? rc : count;
So, if this needed to be an actual errno, it just became some small
positive number less than count. Same for the other conversions.
> }
>
> @@ -223,16 +221,22 @@ target_reclassify_write(struct file *fil
>
> if (count > MAX_INPUT_SIZE)
> return -EINVAL;
> - if (!access_ok(VERIFY_READ, buf, count))
> - return -EFAULT;
> - down(&(ri->vfs_inode.i_sem));
> - optbuf = kmalloc(MAX_INPUT_SIZE, GFP_KERNEL);
> - __copy_from_user(optbuf, buf, count);
> +
> + optbuf = kmalloc(count+1, GFP_KERNEL);
> + if (!optbuf)
> + return -ENOMEM;
> +
> + rc = copy_from_user(optbuf, buf, count);
> + if (rc)
> + goto out_optbuf;
> mkvalidstr(optbuf);
> +
> + down(&(ri->vfs_inode.i_sem));
> clstype = ri->core->classtype;
> if (clstype->forced_reclassify)
> rc = (*clstype->forced_reclassify) (manual ? ri->core: NULL,
> optbuf);
> up(&(ri->vfs_inode.i_sem));
> +out_optbuf:
> kfree(optbuf);
> return (!rc ? count : rc);
>
> @@ -426,40 +430,40 @@ shares_write(struct file *file, const ch
> printk(KERN_ERR "shares_write: Error accessing core class\n");
> return -EFAULT;
> }
> - down(&inode->i_sem);
> - core = ri->core;
> +
> optbuf = kmalloc(SHARES_MAX_INPUT_SIZE, GFP_KERNEL);
> - if (!optbuf) {
> - up(&inode->i_sem);
> + if (!optbuf)
> return -ENOMEM;
> - }
> - __copy_from_user(optbuf, buf, count);
> +
> + rc = copy_from_user(optbuf, buf, count);
> + if (rc)
> + goto out_optbuf;
> mkvalidstr(optbuf);
> +
> +
> + down(&inode->i_sem);
> + core = ri->core;
> done = shares_parse(optbuf, &resname, &newshares);
> if (!done) {
> printk(KERN_ERR "Error parsing shares\n");
> rc = -EINVAL;
> - goto write_out;
> + goto out;
> }
> if (core->classtype->set_shares) {
> rc = (*core->classtype->set_shares) (core, resname, &newshares);
> if (rc) {
> printk(KERN_ERR
> "shares_write: resctlr share set error\n");
> - goto write_out;
> + goto out;
> }
> }
> - printk(KERN_ERR "Set %s shares to %d %d %d %d\n",
> - resname,
> - newshares.my_guarantee,
> - newshares.my_limit,
> - newshares.total_guarantee, newshares.max_limit);
> rc = count;
>
> -write_out:
> +out:
> up(&inode->i_sem);
> - kfree(optbuf);
> kfree(resname);
> +out_optbuf:
> + kfree(optbuf);
> return rc;
> }
>
> Index: ckrm-2611rc5/fs/rcfs/inode.c
> ===================================================================
> --- ckrm-2611rc5.orig/fs/rcfs/inode.c 2005-03-07 14:49:19.000000000 -0500
> +++ ckrm-2611rc5/fs/rcfs/inode.c 2005-03-18 17:23:42.331190016 -0500
> @@ -128,7 +128,7 @@ struct dentry *rcfs_create_internal(stru
> up(&parent->d_inode->i_sem);
> if (err) {
> dput(mfdentry);
> - return mfdentry;
> + return NULL;
If it'd be useful to propagate that error you could return ERR_PTR(err).
> }
> }
> return mfdentry;
> Index: ckrm-2611rc5/fs/rcfs/magic.c
> ===================================================================
> --- ckrm-2611rc5.orig/fs/rcfs/magic.c 2005-03-18 16:50:58.313766048 -0500
> +++ ckrm-2611rc5/fs/rcfs/magic.c 2005-03-18 17:38:07.359685696 -0500
> @@ -502,7 +502,7 @@ int rcfs_create_magic(struct dentry *par
>
> for (i = 0; i < count; i++) {
> mfdentry = rcfs_create_internal(parent, &magf[i], 0);
> - if (IS_ERR(mfdentry)) {
> + if (!mfdentry) {
And if you did, of course, then drop this change.
> rcfs_clear_magic(parent);
> return -ENOMEM;
> }
thanks,
-chris
-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
_______________________________________________
ckrm-tech mailing list
https://lists.sourceforge.net/lists/listinfo/ckrm-tech
