On Thu, Jun 16, 2022 at 9:41 AM Maciej Żenczykowski <maze@xxxxxxxxxx> wrote:
>
> On Thu, Jun 16, 2022 at 8:57 AM Stanislav Fomichev <sdf@xxxxxxxxxx> wrote:
> > On Wed, Jun 15, 2022 at 6:36 PM Maciej Żenczykowski <maze@xxxxxxxxxx> wrote:
> > > I'm guessing this means the regression only affects 64-bit archs,
> > > where long = void* is 8 bytes > u32 of 4 bytes, but not 32-bit ones,
> > > where long = u32 = 4 bytes
> > >
> > > Unfortunately my dev machine's 32-bit build capability has somehow
> > > regressed again and I can't check this.
> >
> > Seems so, yes. But I'm actually not sure whether we should at all
> > treat it as a regression. There is a question of whether that EPERM is
> > UAPI or not. That's why we most likely haven't caught it in the
> > selftests; most of the time we only check that syscall has returned -1
> > and don't pay attention to the particular errno.
>
> EFAULT seems like a terrible error to return no matter what, it has a very clear
> 'memory read/write access violation' semantic (ie. if you'd done from
> userspace you'd get a SIGSEGV)
I chose EFAULT because the original code of getsockopt hook returns
-EFAULT if the retval is set to a number that isn't zero or the
original value. i.e., in c4dcfdd406aa^, there was:
/* BPF programs only allowed to set retval to 0, not some
* arbitrary value.
*/
if (ctx.retval != 0 && ctx.retval != retval) {
ret = -EFAULT;
goto out;
}
I understood that as the convention that if a BPF program does
something illegal at runtime, return -EFAULT.
> I'm actually surprised to learn you return EFAULT on positive number...
> It should rather be some unique error code or EINVAL or something.
>
> I know someone will argue that (most/all) system calls can return EFAULT...
> But that's not actually true. From a userspace developer the expectation is
> they will not return EFAULT if you pass in memory you know is good.
>
> #include <sys/utsname.h>
> int main() {
> struct utsname uts;
> uname(&uts);
> }
>
> The above cannot EFAULT in spite of it being documented as the only
> error uname can report,
> because obviously the uts structure on the stack is valid memory.
>
> Maybe ENOSYS would at least make it obvious something is very weird.
