linux-kernel - Re: [PATCH v3 3/3] generic 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: <201001081157.17195.arnd@arndb.de>
Date:	Fri, 8 Jan 2010 11:57:17 +0100
From:	Arnd Bergmann <arnd@...db.de>
To:	Christoph Hellwig <hch@....de>
Cc:	akpm@...ux-foundation.org, linux-kernel@...r.kernel.org,
	linux-arch@...r.kernel.org, linux@....linux.org.uk,
	starvik@...s.com, jesper.nilsson@...s.com, dhowells@...hat.com,
	ysato@...rs.sourceforge.jp, takata@...ux-m32r.org,
	geert@...ux-m68k.org, zippel@...ux-m68k.org, gerg@...inux.org,
	ralf@...ux-mips.org, benh@...nel.crashing.org,
	schwidefsky@...ibm.com, heiko.carstens@...ibm.com,
	lethal@...ux-sh.org, davem@...emloft.net, jdike@...toit.com,
	tglx@...utronix.de, mingo@...hat.com, hpa@...or.com,
	viro@...iv.linux.org.uk
Subject: Re: [PATCH v3 3/3] generic sys_ipc wrapper

On Friday 08 January 2010 11:03:47 Christoph Hellwig wrote:
> On Fri, Jan 08, 2010 at 10:50:23AM +0100, Christoph Hellwig wrote:
> > Always return -EINVAL for the iBCS2 special case in SHMAT, and add a
> > prototype to linux/syscalls.h

ok, the changes look good.

> Add a generic implementation of the ipc demultiplexer syscall.  Except for
> s390 and sparc64 all implementations of the sys_ipc are nearly identical.

I think the s390 version is trivial to add as well, like

SYSCALL_DEFINE5(s390_ipc, uint, call, int, first, unsigned long, second,
 		unsigned long, third, void __user *, ptr)
{
	if (call == SEMTIMEDOP)
		return sys_semtimedop(first, (struct sembuf __user *)ptr,
				      (unsigned) second,
				      (const struct timespec __user *)third);

	return sys_ipc(call & 0xffff, first, second, third, ptr, 0);
}

But while going over the code again, I noticed that you broke sign extension
at least on powerpc and s390, possibly on all 64 bit machines:

> +SYSCALL_DEFINE6(ipc, unsigned int, call, int, first, int, second,
> +		unsigned long, third, void __user *, ptr, long, fifth)
> +{
> +	int version, ret;
> +
> +	version = call >> 16; /* hack for backward compatibility */
> +	call &= 0xffff;
> +
> +	switch (call) {
> +	case SEMOP:
> +		return sys_semtimedop(first, (struct sembuf __user *)ptr,
> +				      second, NULL);

                                       (unsigned)second, NULL

> +	case SEMTIMEDOP:
> +		return sys_semtimedop(first, (struct sembuf __user *)ptr,
> +				      second,

                                      (unsigned)second,

> +				      (const struct timespec __user *)fifth);
> +
> +	case SEMGET:
> +		return sys_semget(first, second, third);

                                         (int)second,

> +	case SEMCTL: {
> +		union semun fourth;
> +		if (!ptr)
> +			return -EINVAL;
> +		if (get_user(fourth.__pad, (void __user * __user *) ptr))
> +			return -EFAULT;
> +		return sys_semctl(first, second, third, fourth);

		return sys_semctl(first, (int)second, third, fourth);

> +	case MSGSND:
> +		return sys_msgsnd(first, (struct msgbuf __user *) ptr,
> +				  second, third);

				  (size_t)second, third);

> +	case MSGRCV:
> +		switch (version) {
> +		case 0: {
> +			struct ipc_kludge tmp;
> +			if (!ptr)
> +				return -EINVAL;
> +
> +			if (copy_from_user(&tmp,
> +					   (struct ipc_kludge __user *) ptr,
> +					   sizeof(tmp)))
> +				return -EFAULT;
> +			return sys_msgrcv(first, tmp.msgp, second,

			return sys_msgrcv(first, tmp.msgp, (size_t)second,

> +					   tmp.msgtyp, third);
> +		}
> +		default:
> +			return sys_msgrcv(first,
> +					   (struct msgbuf __user *) ptr,
> +					   second, fifth, third);

					   (size_t)second, fifth, third);

> +		}
> +	case MSGGET:
> +		return sys_msgget((key_t) first, second);

		return sys_msgget((key_t) first, (int)second);

> +	case MSGCTL:
> +		return sys_msgctl(first, second, (struct msqid_ds __user *)ptr);

		return sys_msgctl(first, (int)second, (struct msqid_ds __user *)ptr);

> +	case SHMAT:
> +		switch (version) {
> +		default: {
> +			unsigned long raddr;
> +			ret = do_shmat(first, (char __user *)ptr,
> +				       second, &raddr);

				       (int)second, &raddr);

> +			if (ret)
> +				return ret;
> +			return put_user(raddr, (unsigned long __user *) third);
> +		}
> +		case 1:
> +			/*
> +			 * This was the entry point for kernel-originating calls
> +			 * from iBCS2 in 2.2 days.
> +			 */
> +			return -EINVAL;
> +		}
> +	case SHMDT:
> +		return sys_shmdt((char __user *)ptr);
> +	case SHMGET:
> +		return sys_shmget(first, second, third);

				       (size_t)second, &raddr);

> +	case SHMCTL:
> +		return sys_shmctl(first, second,

		return sys_shmctl(first, (int)second,

> +				   (struct shmid_ds __user *) ptr);
> +	default:
> +		return -ENOSYS;
> +	}
> +}

This is needed to make sure the upper half of the register is filled with
zero-extended or sign-extended correctly and does not contain random garbage
in the native 64 bit case. IIRC, x86_64 does not have this problem and mips64
may have the wrong code already. Alpha, parisc and ia64 don't have a native
sys_ipc and the rest are 32 bit, so they don't care.

	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/