xfs
[Top] [All Lists]

Re: [patch 08/11] Hook up compat XFS_IOC_ATTRLIST_BY_HANDLE ioctl handle

To: sandeen@xxxxxxxxxxx
Subject: Re: [patch 08/11] Hook up compat XFS_IOC_ATTRLIST_BY_HANDLE ioctl handler
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Wed, 19 Nov 2008 10:13:48 -0500
Cc: xfs@xxxxxxxxxxx, hch@xxxxxxxxxxxxx
In-reply-to: <20081119044909.981142660@xxxxxxxxxxx>
References: <20081119044401.573365619@xxxxxxxxxxx> <20081119044909.981142660@xxxxxxxxxxx>
User-agent: Mutt/1.5.18 (2008-05-17)
On Tue, Nov 18, 2008 at 10:44:09PM -0600, sandeen@xxxxxxxxxxx wrote:
> +/*
> + * Convert userspace handle data into inode.
> + *
> + * We use the fact that all the fsop_handlereq ioctl calls have a data
> + * structure argument whose first component is always a xfs_fsop_handlereq_t,
> + * so we can pass that sub structure into this handy, shared routine.
> + *
> + * If no error, caller must always iput the returned inode.
> + */
> +STATIC int
> +xfs_vget_fsop_handlereq_compat(
> +     xfs_mount_t             *mp,
> +     struct inode            *parinode,      /* parent inode pointer    */
> +     compat_xfs_fsop_handlereq_t     *hreq,
> +     struct inode            **inode)
> +{
> +     void                    __user *hanp;
> +     size_t                  hlen;
> +     xfs_fid_t               *xfid;
> +     xfs_handle_t            *handlep;
> +     xfs_handle_t            handle;
> +     xfs_inode_t             *ip;
> +     xfs_ino_t               ino;
> +     __u32                   igen;
> +     int                     error;
> +
> +     /*
> +      * Only allow handle opens under a directory.
> +      */
> +     if (!S_ISDIR(parinode->i_mode))
> +             return XFS_ERROR(ENOTDIR);
> +
> +     hanp = compat_ptr(hreq->ihandle);
> +     hlen = hreq->ihandlen;
> +     handlep = &handle;
> +
> +     if (hlen < sizeof(handlep->ha_fsid) || hlen > sizeof(*handlep))
> +             return XFS_ERROR(EINVAL);
> +     if (copy_from_user(handlep, hanp, hlen))
> +             return XFS_ERROR(EFAULT);
> +     if (hlen < sizeof(*handlep))
> +             memset(((char *)handlep) + hlen, 0, sizeof(*handlep) - hlen);
> +     if (hlen > sizeof(handlep->ha_fsid)) {
> +             if (handlep->ha_fid.fid_len !=
> +                 (hlen - sizeof(handlep->ha_fsid) -
> +                         sizeof(handlep->ha_fid.fid_len)) ||
> +                 handlep->ha_fid.fid_pad)
> +                     return XFS_ERROR(EINVAL);
> +     }
> +
> +     /*
> +      * Crack the handle, obtain the inode # & generation #
> +      */
> +     xfid = (struct xfs_fid *)&handlep->ha_fid;
> +     if (xfid->fid_len == sizeof(*xfid) - sizeof(xfid->fid_len)) {
> +             ino  = xfid->fid_ino;
> +             igen = xfid->fid_gen;
> +     } else {
> +             return XFS_ERROR(EINVAL);
> +     }
> +
> +     /*
> +      * Get the XFS inode, building a Linux inode to go with it.
> +      */
> +     error = xfs_iget(mp, NULL, ino, 0, XFS_ILOCK_SHARED, &ip, 0);
> +     if (error)
> +             return error;
> +     if (ip == NULL)
> +             return XFS_ERROR(EIO);
> +     if (ip->i_d.di_gen != igen) {
> +             xfs_iput_new(ip, XFS_ILOCK_SHARED);
> +             return XFS_ERROR(ENOENT);
> +     }
> +
> +     xfs_iunlock(ip, XFS_ILOCK_SHARED);
> +
> +     *inode = VFS_I(ip);
> +     return 0;

Not really a big fan of duplicating this, at least the second half could
be factored out into a common helper for native and compat handle ops,
dmapi and nfs exporting.  But I think this is fine for now, I have some
bigger plans for this area anyway.

So ACK for this patch.

<Prev in Thread] Current Thread [Next in Thread>