On 23 June 2014 23:18, Paul Burton <p...@archlinuxmips.org> wrote: > On Mon, Jun 23, 2014 at 11:12:42PM +0100, Peter Maydell wrote: >> On 23 June 2014 22:40, Paul Burton <p...@archlinuxmips.org> wrote: >> > The ptr argument to the ipc syscall was incorrectly being used as the >> > value of the argument union for the SEMCTL call. It is actually, as its >> > name would suggest, a pointer to that union. >> >> Have you checked this on other architectures than MIPS? >> I have a vague recollection that there are between-arch >> differences regarding handling of the semctl argument... > > I haven't tried running code for any other targets, but the pointer is > dereferenced from generic code in Linux, see ipc/syscall.c: > > > http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/ipc/syscall.c#n39
I see that code has a NULL pointer check which your patch doesn't... >> Also, VERIFY_READ doesn't seem right for some of the >> semctl operations which will modify the target_semun. > That part I think you're right about, I'll switch to VERIFY_WRITE. On the other hand that doesn't line up with the kernel code, which just does a get_user() of a single target_ulong and passes that to the sys_semctl function (which then might interpret it as a user pointer to a thing that needs locking and might be written to). That would suggest that you should be using get_user_ual() here, passing an abi_ulong into do_semctl() and probably fixing up that function and its callers. Basically I think the semctl code is probably buggy in a number of ways and so I'm dubious about a patch that's trying to make a very small change to it without looking at the larger picture and testing and fixing bugs on more than one architecture. (http://patchwork.ozlabs.org/patch/217249/ is a previous attempt at fixing up semctl; on reflection I may have been wrong about the relevance of compat_sys_semctl, though.) thanks -- PMM
