linux-kernel - Re: [RFC][PATCH] Anonymous shared memory (ashmem) subsystem

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: <201107191755.50749.arnd@arndb.de>
Date:	Tue, 19 Jul 2011 17:55:50 +0200
From:	Arnd Bergmann <arnd@...db.de>
To:	John Stultz <john.stultz@...aro.org>
Cc:	LKML <linux-kernel@...r.kernel.org>,
	Robert Love <rlove@...gle.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Hugh Dickins <hughd@...gle.com>, Mel Gorman <mel@....ul.ie>,
	Dave Hansen <dave@...ux.vnet.ibm.com>,
	Rik van Riel <riel@...hat.com>,
	Eric Anholt <eric@...olt.net>,
	Jesse Barnes <jbarnes@...tuousgeek.org>
Subject: Re: [RFC][PATCH] Anonymous shared memory (ashmem) subsystem

On Monday 18 July 2011, John Stultz wrote:

> +
> +#define ASHMEM_NAME_DEF		"dev/ashmem"
> +
> +/* Return values from ASHMEM_PIN: Was the mapping purged while unpinned? */
> +#define ASHMEM_NOT_PURGED	0
> +#define ASHMEM_WAS_PURGED	1
> +
> +/* Return values from ASHMEM_GET_PIN_STATUS: Is the mapping pinned? */
> +#define ASHMEM_IS_UNPINNED	0
> +#define ASHMEM_IS_PINNED	1
> +
> +struct ashmem_pin {
> +	__u32 offset;	/* offset into region, in bytes, page-aligned */
> +	__u32 len;	/* length forward from offset, in bytes, page-aligned */
> +};
> +
> +#define __ASHMEMIOC		0x77
> +
> +#define ASHMEM_SET_NAME		_IOW(__ASHMEMIOC, 1, char[ASHMEM_NAME_LEN])
> +#define ASHMEM_GET_NAME		_IOR(__ASHMEMIOC, 2, char[ASHMEM_NAME_LEN])

Having a name for an 'anonymous shared memory' segment is rather
counter-intuitive ;-)

> +#define ASHMEM_SET_SIZE		_IOW(__ASHMEMIOC, 3, size_t)
> +#define ASHMEM_GET_SIZE		_IO(__ASHMEMIOC, 4)
> +#define ASHMEM_SET_PROT_MASK	_IOW(__ASHMEMIOC, 5, unsigned long)
> +#define ASHMEM_GET_PROT_MASK	_IO(__ASHMEMIOC, 6)

size_t and unsigned long arguments in ioctl commands are harmful,
because they require a compat_ioctl function. It's often better to just
make these either __u32 or __u64.

> diff --git a/mm/Makefile b/mm/Makefile
> index 836e416..cd41f09 100644
> --- a/mm/Makefile
> +++ b/mm/Makefile
> @@ -30,6 +30,7 @@ obj-$(CONFIG_HUGETLBFS)	+= hugetlb.o
>  obj-$(CONFIG_NUMA) 	+= mempolicy.o
>  obj-$(CONFIG_SPARSEMEM)	+= sparse.o
>  obj-$(CONFIG_SPARSEMEM_VMEMMAP) += sparse-vmemmap.o
> +obj-$(CONFIG_ASHMEM)	+= ashmem.o
>  obj-$(CONFIG_SLOB) += slob.o
>  obj-$(CONFIG_COMPACTION) += compaction.o
>  obj-$(CONFIG_MMU_NOTIFIER) += mmu_notifier.o

Wouldn't this better live in drivers/char, next to mem.c or even
merged into that file?

> +static const struct file_operations ashmem_fops = {
> +	.owner = THIS_MODULE,
> +	.open = ashmem_open,
> +	.release = ashmem_release,
> +	.read = ashmem_read,
> +	.llseek = ashmem_llseek,
> +	.mmap = ashmem_mmap,
> +	.unlocked_ioctl = ashmem_ioctl,
> +	.compat_ioctl = ashmem_ioctl,
> +};

The compat_ioctl is wrong here, as described above.

	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/