linux-kernel - Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it

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]

Date:	Wed, 18 May 2016 12:21:46 +0100
From:	Catalin Marinas <catalin.marinas@....com>
To:	Yury Norov <ynorov@...iumnetworks.com>
Cc:	linux-arch@...r.kernel.org, linux-s390@...r.kernel.org,
	arnd@...db.de, pinskia@...il.com, Prasun.Kapoor@...iumnetworks.com,
	Nathan_Lynch@...tor.com, joseph@...esourcery.com,
	linux-doc@...r.kernel.org, heiko.carstens@...ibm.com,
	linux-kernel@...r.kernel.org, agraf@...e.de,
	klimov.linux@...il.com,
	Andrew Pinski <Andrew.Pinski@...iumnetworks.com>,
	broonie@...nel.org, bamvor.zhangjian@...wei.com,
	Andrew Pinski <apinski@...ium.com>, schwab@...e.de,
	schwidefsky@...ibm.com, linux-arm-kernel@...ts.infradead.org,
	christoph.muellner@...obroma-systems.com
Subject: Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table
 (in entry.S) to use it

On Tue, May 17, 2016 at 10:05:26PM +0300, Yury Norov wrote:
> On Mon, May 16, 2016 at 06:06:05PM +0100, Catalin Marinas wrote:
> > On Sat, May 14, 2016 at 06:03:52PM +0300, Yury Norov wrote:
> > > +SYSCALL_DEFINE6(mmap2, unsigned long, addr, unsigned long, len,
> > > +       unsigned long, prot, unsigned long, flags, unsigned long, fd,
> > > +       unsigned long, pgoff)
> > 
> > To avoid the types confusion we could add __SC_WRAP to mmap2 in unistd.h
> > and use COMPAT_SYSCALL_DEFINE6 here (together with compat_ptr_t etc.).
> > 
> > > +{
> > > +       if (pgoff & (~PAGE_MASK >> 12))
> > > +               return -EINVAL;
> > > +
> > > +       return sys_mmap_pgoff((compat_uptr_t) addr, (compat_size_t) len,
> > > +		       (int) prot, (int) flags, (int) fd,
> > > +		       pgoff >> (PAGE_SHIFT-12));
> > 
> > Then we wouldn't need the explicit casting here.
> 
> See below
> 
> > > +}
> > > +
> > > +COMPAT_SYSCALL_DEFINE4(pread64, unsigned int, fd, compat_uptr_t __user *, ubuf,
> > > +		       compat_size_t, count, off_t, offset)
> > > +{
> > > +	return sys_pread64(fd, (char *) ubuf, count, offset);
> > > +}
> > > +
> > > +COMPAT_SYSCALL_DEFINE4(pwrite64, unsigned int, fd, compat_uptr_t __user *, ubuf,
> > > +		       compat_size_t, count, off_t, offset)
> > > +{
> > > +	return sys_pwrite64(fd, (char *) ubuf, count, offset);
> > 
> > Nitpick: no space between cast type and variable name: (char *)ubuf, ...
> 
> I think it's really a matter of taste. I prefer to have a space, and
> there's no solid rule in coding style.
> 
> And there are 13032 insertions of my version vs 35030 of yours:
> ~/work/linux$ git grep ' \*)[a-zA-Z]'|wc -l
> 35030
> ~/work/linux$ git grep ' \*) [a-zA-Z]'|wc -l
> 13032
> 
> Of course, I will change it if you insist.

Not really, I thought it's covered by CodingStyle but it doesn't seem
to.

> > We can also make these functions static as they are not used outside
> > this file.
> 
> If it's OK, we can use just compat_sys_xxx() instead of
> COMPAT_SYSCALL_DEFINEx(xxx),

I got lost in macros, what difference would COMPAT_SYSCALL_DEFINE vs
compat_sys_*() make? Is this the delouse stuff?

> and for mmap2 we'll not need to change _SYSCALL to _SC_WRAP in
> unistd.h that way.

Looking at the generic unistd.h, adding _SC_WRAP for sys_mmap2 is
indeed not easy:

#define __NR3264_mmap 222
__SC_3264(__NR3264_mmap, sys_mmap2, sys_mmap)

So defining a compat_sys_mmap2() would work but I think you'd also need:

#define sys_mmap2	compat_sys_mmap2()

-- 
Catalin