xfs
[Top] [All Lists]

Re: [patch 02/11] Fix the compat XFS_IOC_FSGEOMETRY_V1 ioctl

To: sandeen@xxxxxxxxxxx
Subject: Re: [patch 02/11] Fix the compat XFS_IOC_FSGEOMETRY_V1 ioctl
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Wed, 19 Nov 2008 10:02:32 -0500
Cc: xfs@xxxxxxxxxxx, hch@xxxxxxxxxxxxx
In-reply-to: <20081119044908.158054231@xxxxxxxxxxx>
References: <20081119044401.573365619@xxxxxxxxxxx> <20081119044908.158054231@xxxxxxxxxxx>
User-agent: Mutt/1.5.18 (2008-05-17)
On Tue, Nov 18, 2008 at 10:44:03PM -0600, sandeen@xxxxxxxxxxx wrote:
> This ioctl copies kernel data to the user, so we
> must have a compat helper to copy it out to the
> 32-bit structure; the current code had it backward,
> and translated the 32-bit arg to 64-bit, and called
> the native ioctl, which copied it back as if it were
> talking to 64-bit userspace.  Because the 64-bit arg
> has padding on the end on intel, I think this risked
> corruption in userspace..
> 
> Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxxx>
> --

> +/* This handles a copy-out, where the 32-bit user struct lacks padding */
> +STATIC int
> +xfs_ioc_fsgeometry_v1_compat(
> +     xfs_mount_t             *mp,

        struct xfs_mount        *mp,

please

> +     void                    __user *arg)
>  {
> +     xfs_fsop_geom_v1_t      fsgeo;
> +     int                     error;
>  
> +     error = xfs_fs_geometry(mp, (xfs_fsop_geom_t *)&fsgeo, 3);
> +     if (error)
> +             return -error;
> +
> +     if (copy_to_user(arg, &fsgeo, sizeof(struct compat_xfs_fsop_geom_v1)))
>               return -XFS_ERROR(EFAULT);
> +     return 0;

Any reason you only allocate a xfs_fsop_geom_v1_t on stack?  Just
allocating a xfs_fsop_geom_t even if you don't use it would be cleaner.

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