Re: [PATCH] generic compat_sys_ustat

[Arnd Bergmann <arnd@arndb.de>]

On Wednesday 26 November 2008, Christoph Hellwig wrote:
> +asmlinkage long compat_sys_ustat(unsigned dev, struct compat_ustat __user *cu)
> +{
> +       struct ustat __user *u = compat_alloc_user_space(sizeof(*u));
> +       int ret;
> +
> +       ret = sys_ustat(dev, u);
> +       if (ret < 0)
> +               return ret;
> +
> +       if (!access_ok(VERIFY_WRITE, cu, sizeof(*cu)) ||
> +           __copy_in_user(&cu->f_tfree, &u->f_tfree, sizeof(compat_daddr_t)) ||
> +           __copy_in_user(&cu->f_tinode, &u->f_tinode, sizeof(compat_ino_t)) ||
> +           __copy_in_user(&cu->f_fname, u->f_fname, sizeof(cu->f_fname)) ||
> +           __copy_in_user(&cu->f_fpack, u->f_fpack, sizeof(cu->f_fpack)))
> +               return -EFAULT;
> +       return 0;
> +}

The __copy_in_user for f_tinode and f_tfree only work on little-endian
systems, or if the sizes are the same for 32 and 64 bit. f_fname and
f_fpack don't need to be copied at all, as they are always zero-filled in
the current implementation (which is unlikely to ever change).

Also, this function is not much simpler than the actual sys_ustat function,
so have you considered implementing it directly like this?

asmlinkage long compat_sys_ustat(unsigned dev, struct compat_ustat __user * ubuf)
{
        struct super_block *s;
        struct compat_ustat tmp;
        struct kstatfs sbuf;
        int err = -EINVAL;

        s = user_get_super(new_decode_dev(dev));
        if (s == NULL)
                goto out;
        err = vfs_statfs(s->s_root, &sbuf);
        drop_super(s);
        if (err)
                goto out;

        memset(&tmp,0,sizeof(struct compat_ustat));
        tmp.f_tfree = sbuf.f_bfree;
        tmp.f_tinode = sbuf.f_ffree;

        err = copy_to_user(ubuf,&tmp,sizeof(struct compat_ustat)) ? -EFAULT : 0;
out:
        return err;
}

This code is directly lifted from the sys_ustat implementation, with a few
compat_ added in the right places.

	Arnd <><