EACCES errors using preload libs

EACCES errors using preload libs

[Steve Wise]

Hey Jason,  I'm looking into a problem that appears to have been introduced
by:

commit e6bd18f57aad1a2d1ef40e646d03ed0f2515c9e3
Author: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
Date:   Sun Apr 10 19:13:13 2016 -0600

    IB/security: Restrict use of the write() interface

I'm using a preload library (similar to rsockets) that enables a user-mode
UDP stack over chelsio hardware.  The preload lib uses user rdma verbs
calls.  In some cases, verbs calls are getting EACCES.  Can you explain what
"suspicious" write API calls this is disallowing:

static inline bool ib_safe_file_access(struct file *filp)
{
        return filp->f_cred == current_cred() && segment_eq(get_fs(),
USER_DS);
}

The single case I see failing so far is trying to preload my library with
netserver.  If I run netserver with '-D -f' which avoids fork() calls, it
works fine.  If I omit the -f, then the forked process gets an EACCES error
trying to do ibv_query_device().  Thoughts?

Thanks,

Steve.

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: EACCES errors using preload libs

[Jason Gunthorpe]

On Fri, Sep 30, 2016 at 12:43:02PM -0500, Steve Wise wrote:

> The single case I see failing so far is trying to preload my library with
> netserver.  If I run netserver with '-D -f' which avoids fork() calls, it
> works fine.  If I omit the -f, then the forked process gets an EACCES error
> trying to do ibv_query_device().  Thoughts?

Hum, so I expect the patch to break things. Specifically if you change
the process credentials after opening verbs. However just forking()
shouldn't cause a problem. Can you confirm there is nothing that
changes uid,gid, etc during the fork?

Unfortunately it was designed by Linus and Andy to work this way, so
we cannot weaken the check.

I don't know how your preload library works, but after fork basically
everything related to verbs is garbage - so it should be safe to
close/reopen then uverbs fd which will avoid the problem. It is only
because the FD was passed across a credential change that you got
EACCESS.

The way forward to restore the lost functionality is to rework the
entire uAPI to use ioctl, which is what Matan/Sean/etc have been
doing. But that work seems to be going very slowly, I hoped we'd have
something by now.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: EACCES errors using preload libs

[Steve Wise]

> On Fri, Sep 30, 2016 at 12:43:02PM -0500, Steve Wise wrote:
> 
> > The single case I see failing so far is trying to preload my library
with
> > netserver.  If I run netserver with '-D -f' which avoids fork() calls,
it
> > works fine.  If I omit the -f, then the forked process gets an EACCES
error
> > trying to do ibv_query_device().  Thoughts?
> 
> Hum, so I expect the patch to break things. Specifically if you change
> the process credentials after opening verbs. However just forking()
> shouldn't cause a problem. Can you confirm there is nothing that
> changes uid,gid, etc during the fork?
> 

I'll debug this further, but this is running as root, so that sort of stuff
shouldn't be changing.

> Unfortunately it was designed by Linus and Andy to work this way, so
> we cannot weaken the check.
>

Sure.
 
> I don't know how your preload library works, but after fork basically
> everything related to verbs is garbage - so it should be safe to
> close/reopen then uverbs fd which will avoid the problem. It is only
> because the FD was passed across a credential change that you got
> EACCESS.
>

Fork() is actually not supported by this preload library, but we were
getting by, I guess, with netserver because it fork()s before it allocates
the UDP socket that then gets accelerated by the preload lib.  However, I'm
thinking that librdacm is getting invoked and is opening contexts to all the
verbs devices before the fork() and thus after it, this new "safe" check it
biting me.   

> The way forward to restore the lost functionality is to rework the
> entire uAPI to use ioctl, which is what Matan/Sean/etc have been
> doing. 

Right, I'm ok with waiting for this (and helping as I can, but I've been
very quiet to date :) ).

Thanks Jason, 

Steve.

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: EACCES errors using preload libs

[Jason Gunthorpe]

On Fri, Sep 30, 2016 at 01:13:02PM -0500, Steve Wise wrote:
> > Hum, so I expect the patch to break things. Specifically if you change
> > the process credentials after opening verbs. However just forking()
> > shouldn't cause a problem. Can you confirm there is nothing that
> > changes uid,gid, etc during the fork?
> 
> I'll debug this further, but this is running as root, so that sort of stuff
> shouldn't be changing.

Based on some emails I've got I think there is a 'thing' with
credentials in Linux where something copies the creds struct but
(perhaps?) does not change it.

Linus designed the test to be a pointer compare, so if anything does a
copy-but-no-modify then it will break the test unnecessarily. If I
understand everything properly :|

If you can add any more information I'm sure people will appreciate
it.

> Fork() is actually not supported by this preload library, but we were
> getting by, I guess, with netserver because it fork()s before it allocates
> the UDP socket that then gets accelerated by the preload lib.  However, I'm
> thinking that librdacm is getting invoked and is opening contexts to all the
> verbs devices before the fork() and thus after it, this new "safe" check it
> biting me.

Hmm, sounds plausible. Same deal with rdmacm, it has to close/open the
verbs device. Defering doing anything until first needed would also be
a reasonable strategy.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: EACCES errors using preload libs

[Steve Wise]

> On Fri, Sep 30, 2016 at 01:13:02PM -0500, Steve Wise wrote:
> > > Hum, so I expect the patch to break things. Specifically if you change
> > > the process credentials after opening verbs. However just forking()
> > > shouldn't cause a problem. Can you confirm there is nothing that
> > > changes uid,gid, etc during the fork?
> >
> > I'll debug this further, but this is running as root, so that sort of
stuff
> > shouldn't be changing.
> 
> Based on some emails I've got I think there is a 'thing' with
> credentials in Linux where something copies the creds struct but
> (perhaps?) does not change it.
> 
> Linus designed the test to be a pointer compare, so if anything does a
> copy-but-no-modify then it will break the test unnecessarily. If I
> understand everything properly :|
> 
> If you can add any more information I'm sure people will appreciate
> it.
> 
> > Fork() is actually not supported by this preload library, but we were
> > getting by, I guess, with netserver because it fork()s before it
allocates
> > the UDP socket that then gets accelerated by the preload lib.  However,
I'm
> > thinking that librdacm is getting invoked and is opening contexts to all
the
> > verbs devices before the fork() and thus after it, this new "safe" check
it
> > biting me.
> 
> Hmm, sounds plausible. Same deal with rdmacm, it has to close/open the
> verbs device. Defering doing anything until first needed would also be
> a reasonable strategy.

I think it does.  But perhaps my offload lib is actually the culprit.  It
goes and walks all the interfaces and builds a database of available devices
that can support this user mode UDP stack.  

I'll post more as I identify the root cause.

Steve.

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html