netdev - Re: [RFC v2] net: Introduce recvmmsg socket syscall

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: <20090612141924.GA2568@ghostprotocols.net>
Date:	Fri, 12 Jun 2009 11:19:24 -0300
From:	Arnaldo Carvalho de Melo <acme@...hat.com>
To:	Rémi Denis-Courmont 
	<remi.denis-courmont@...ia.com>
Cc:	David Miller <davem@...emloft.net>, netdev@...r.kernel.org,
	Chris Van Hoof <vanhoof@...hat.com>,
	Clark Williams <williams@...hat.com>,
	Caitlin Bestler <caitlin.bestler@...il.com>,
	Paul Moore <paul.moore@...com>,
	Steven Whitehouse <steve@...gwyn.com>,
	Neil Horman <nhorman@...driver.com>,
	Nivedita Singhvi <niv@...ibm.com>
Subject: Re: [RFC v2] net: Introduce recvmmsg socket syscall

Em Fri, Jun 12, 2009 at 10:26:25AM +0300, Rémi Denis-Courmont escreveu:
> On Thursday 11 June 2009 06:40:22 ext Arnaldo Carvalho de Melo wrote:
> > diff --git a/arch/x86/include/asm/unistd_64.h
> > b/arch/x86/include/asm/unistd_64.h index 900e161..713a32a 100644
> > --- a/arch/x86/include/asm/unistd_64.h
> > +++ b/arch/x86/include/asm/unistd_64.h
> > @@ -661,6 +661,8 @@ __SYSCALL(__NR_pwritev, sys_pwritev)
> >  __SYSCALL(__NR_rt_tgsigqueueinfo, sys_rt_tgsigqueueinfo)
> >  #define __NR_perf_counter_open                 298
> >  __SYSCALL(__NR_perf_counter_open, sys_perf_counter_open)
> > +#define __NR_recvmmsg                          299
> > +__SYSCALL(__NR_recvmmsg, sys_recvmmsg)
> 
> I guess socketcall is deprecated in favor of full syscalls, then?
> (sorry if this is a stupid question)

In some architectures, like x86_64, yes, but some still need it, the
ones that set __ARCH_WANT_SYS_SOCKETCALL:

[acme@...pio linux-2.6-tip]$ echo $(find . -type f | xargs egrep
"define.+__ARCH_WANT_SYS_SOCKETCALL" | sed -r 's#./arch/(\w+)/.*#\1#g' |
sort)
arm cris frv h8300 m32r m68k microblaze mips mn10300 parisc powerpc s390
sh sh sparc x86 x86

So the alpha and ia64, for instance, doesn't want sys_socketcall:

[acme@...pio linux-2.6-tip]$ egrep 'sys_((recv|send)msg|[gs]etsockopt)'
arch/alpha/kernel/systbls.S
	.quad sys_setsockopt			/* 105 */
	.quad sys_recvmsg
	.quad sys_sendmsg
	.quad sys_getsockopt
[acme@...pio linux-2.6-tip]$ egrep sys_socketcall arch/alpha/kernel/systbls.S
[acme@...pio linux-2.6-tip]$

[acme@...pio linux-2.6-tip]$ egrep 'sys_((recv|send)msg|[gs]etsockopt)' arch/ia64/kernel/entry.S
	data8 sys_setsockopt
	data8 sys_getsockopt
	data8 sys_sendmsg			// 1205
	data8 sys_recvmsg
[acme@...pio linux-2.6-tip]$ egrep sys_socketcall
arch/ia64/kernel/entry.S
[acme@...pio linux-2.6-tip]$ 

But some define __ARCH_WANT_SYS_SOCKETCALL conditionally, like x86_64
and um:

[acme@...pio linux-2.6-tip]$ find arch -type f | xargs egrep 'define.+__NO_STUBS'
arch/x86/kernel/syscall_64.c:#define __NO_STUBS
arch/x86/kernel/asm-offsets_64.c:#define __NO_STUBS 1
arch/um/sys-x86_64/syscall_table.c:#define __NO_STUBS
[acme@...pio linux-2.6-tip]$ 

And others want socketcall if preserving old ABIs, like ARM:

arch/arm/include/asm/unistd.h

#if !defined(CONFIG_AEABI) || defined(CONFIG_OABI_COMPAT)
#define __ARCH_WANT_SYS_TIME
#define __ARCH_WANT_SYS_OLDUMOUNT
#define __ARCH_WANT_SYS_ALARM
#define __ARCH_WANT_SYS_UTIME
#define __ARCH_WANT_SYS_OLD_GETRLIMIT
#define __ARCH_WANT_OLD_READDIR
#define __ARCH_WANT_SYS_SOCKETCALL
#endif

So for the final revision indeed I need to provide a sys_socketcall
interface to sys_recvmmsg.
 
> > +       if (timeout) {
> > +               /* Doesn't make much sense */
> > +               if (flags & MSG_DONTWAIT)
> > +                       return -EINVAL;
> 
> An application could possibly hit this degenerated case at the end of a loop 
> or whatever, and EINVAL makes it look like this is a bug. Why not EAGAIN?

EAGAIN for me looks like something that when repeated would possibly
produce a valid result, but in this case it will never be that way.

But lemme look around, perhaps this assumption of mine is invalidated by
some strange standard...  EAGAIN = EWOULDBLOCK, and in the current
implementation it wouldn't block at all, because recvmsg would return
right away...

The only semantic we could associate to this would be to make recvmmsg
call, in a non blocking way recvmsg (as specified in the flags) and if
it returns EAGAIN go to sleep waiting for more packets, i.e. a mixed
blocking recvmmsg with a noblocking recvmsg, sounds of creeping
featuritis...

Or perhaps just remove this check and let recvmmsg return imediately,
the app could then use this degenerated case to measure how long it took
for the packets to be received, looks like the most liberal and removes
3 lines of source code and a branch :-\

Keep it simple?

- Arnaldo
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html