On Tue, Sep 8, 2020 at 8:20 AM Christoph Hellwig <hch at lst.de> wrote:
> > @@ -264,68 +266,24 @@ asmlinkage long sys_oabi_epoll_ctl(int epfd, int op, int fd,
> > return do_epoll_ctl(epfd, op, fd, &kernel, false);
> > }
> >
> > -static long do_oabi_epoll_wait(int epfd, struct oabi_epoll_event __user *events,
> > - int maxevents, int timeout)
> > +struct epoll_event __user *
> > +epoll_put_uevent(__poll_t revents, __u64 data, struct epoll_event __user *uevent)
> > {
> > + if (in_oabi_syscall()) {
> > + struct oabi_epoll_event *oevent = (void __user *)uevent;
> >
> > + if (__put_user(revents, &oevent->events) ||
> > + __put_user(data, &oevent->data))
> > + return NULL;
> >
> > + return (void __user *)uevent+1;
FWIW, this line needs to be
return (void __user *)(oevent+1);
It turns out that while I thought I had tested this already, my earlier
tests were on the EABI Debian 5 instead of the OABI version of the
same distro. I reproduced it both ways now and LTP successfully
found that bug ;-)
> I wonder if we'd be better off doing the in_oabi_syscall() branch in
> the common code. E.g. rename in_oabi_syscall to in_legacy_syscall and
> stub it out for all other architectures. Then just do
>
> if (in_oabi_syscall()
> legacy_syscall_foo_bit();
> else
> normal_syscall_foo_bit();
>
> in common code, where so far only arm provides
> legacy_syscall_foo_bit().
I tried out different ways, the first one I had was with an #ifdef in the
C code that I did not like much.
Moving the different code path into common code would avoid that
#ifdef but also put the rather obscure oabi-compat code into a
much more prominent location. I'd prefer to keep it out of there
as much as possible and hope we don't need to do this anywhere
else. x86-32 has some similar issues with struct layout, but that
already goes through the normal compat layer on 64-bit kernels.
> Tons of long lines again in this patch..
Fixed now.
Arnd