linux-kernel - Re: [PATCH] generic compat_sys

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <200811261417.18016.arnd@arndb.de>
Date:	Wed, 26 Nov 2008 14:17:16 +0100
From:	Arnd Bergmann <arnd@...db.de>
To:	Christoph Hellwig <hch@....de>
Cc:	linux-kernel@...r.kernel.org, sandeen@...deen.net,
	davem@...emloft.net, tony.luck@...el.com, ralf@...ux-mips.org,
	kyle@...artin.ca, schwidefsky@...ibm.com
Subject: Re: [PATCH] generic compat_sys_ustat

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 <><
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/