[PATCH] RDMA/uverbs: Consider capability of the process that opens the file

[Eric W. Biederman]

Jason Gunthorpe <jgg at nvidia.com> writes:

> On Wed, Apr 23, 2025 at 12:41:26PM +0000, Parav Pandit wrote:
>> 
>> > From: Serge E. Hallyn <serge at hallyn.com>
>> > Sent: Tuesday, April 22, 2025 10:00 PM
>> > 
>> > On Tue, Apr 22, 2025 at 01:11:27PM -0300, Jason Gunthorpe wrote:
>> > > On Tue, Apr 22, 2025 at 08:14:33AM -0500, Serge E. Hallyn wrote:
>> > > > Hi Jason,
>> > > >
>> > > > On Tue, Apr 22, 2025 at 09:46:40AM -0300, Jason Gunthorpe wrote:
>> > > > > On Mon, Apr 21, 2025 at 12:22:36PM -0500, Serge E. Hallyn wrote:
>> > > > > > > > 1. the create should check
>> > > > > > > > ns_capable(current->nsproxy->net->user_ns,
>> > > > > > > > CAP_NET_RAW)
>> > > > > > > I believe this is sufficient as this create call happens through the
>> > ioctl().
>> > > > > > > But more question on #3.
>> > > > >
>> > > > > I think this is the right one to use everywhere.
>> > > >
>> > > > It's the right one to use when creating resources, but when later
>> > > > using them, since below you say that the resource should in fact be
>> > > > tied to the creator's network namespace, that means that checking
>> > > > current->nsproxy->net->user_ns would have nothing to do with the
>> > > > resource being used, right?
>> > >
>> > > Yes, in that case you'd check something stored in the uobject.
>> > 
>> > Perfect, that's exactly the kind of thing I was looking for.  Thanks.
>> >
>> It means uboject create path will refcount and store user_ns, 
>> 
>> uobject->user_ns = get_user_ns(current->nsproxy->net->user_ns);
>> 
>> And uobject destroy will do,
>> 	put_user_ns(uobject->user_ns).
>> 
>> This will ensure that in below flow we won't have use_after_free.
>> 1. process_A created object in user_ns_A
>> 2. process_A shared fd with process_B in user_ns_B
>> 3. process_A is killed and
>> 4. user_ns_A is free is attempted (free is skipped, until uobject is destroyed by process_B).
>
> We only need to do that if something is legimitately doing capable
> from a uobject outside of creation? Did you find that?

I believe the proposed change that started this discussion,
was to make rdma usable inside of a user namespace.

Which led to the question: Are the current capable calls safe and
correct, as they aren't preserving the context that can with opening a
file descriptor?  If I have skimmed this thread correctly the answer
not preserving the opener's context is a seriously atypical but
deliberate choice.

> And I wonder if using the uobjects affiliated netdev's namespace is
> OK?

That is actually preferable.  It is what I updated the rest of the
network stack to do.  I don't know if you would use dev_net or
something else.

Going back to the original proposal I don't know how ready the code is
to handle callers that are not root.  This is both a question of
semantics (is it safe in theory) and a question of implementation (are
there unfixed bugs that no one cares about because only root has been
using the code).

Eric