On Fri, Jun 14, 2019 at 03:00:32PM -0400, Doug Ledford wrote:
> On Thu, 2019-06-13 at 21:38 -0300, Jason Gunthorpe wrote:
> > + if (ibdev)
> > + ret = __ib_get_client_nl_info(ibdev, client_name,
> > res);
> > + else
> > + ret = __ib_get_global_client_nl_info(client_name,
> > res);
> > +#ifdef CONFIG_MODULES
> > + if (ret == -ENOENT) {
> > + request_module("rdma-client-%s", client_name);
> > + if (ibdev)
> > + ret = __ib_get_client_nl_info(ibdev,
> > client_name, res);
> > + else
> > + ret =
> > __ib_get_global_client_nl_info(client_name, res);
> > + }
> > +#endif
>
> I was trying to put my finger on something yesterday while reading the
> code, and this change makes it more clear for me. Do we really want to
> limit the info type based on ibdev? It seems to me that all global
> info retrieval should work whether you open a specific ibdev or not.
> It's only the things that need the ibdev to return the correct response
> that should require it. Right now we only have one global info
> provider, but would it be better to do:
>
> if (!strcmp("rdma_cm", client_name))
> ret = __ib_get_global_client_nl_info(client_name, res);
> else
> ret = __ib_get_client_nl_info(ibdev, client_name, res);
>
> The other thing I was wondering about was the module loading. Every
> attempt to load a module is a fork/exec cycle and a context switch over
> to modprobe and back, and we make no attempt here to keep each
> invocation of the netlink query from requesting a module. I'm
> concerned this is actually a potential DoS attack vector. I was
> thinking we should track each client name that's valid, and only try
> each name once. I saw four module names: rdma_cm, umad, issm, and
> uverbs. I'm wondering if we should have a static table in the netlink
> file with an entry for each of the client names and a variable to
> indicate we've attempted to load that module, and on -ENOENT, we check
> the table for a match to our passed in client_name, and only if we have
> a match, and it's load count is 0, do we call request_module() and
> increment the load count. Thoughts?
Isn't request_module privileged operation, so only root or his friends
can do this DDoS?
Thanks
>
> --
> Doug Ledford <dledford@xxxxxxxxxx>
> GPG KeyID: B826A3330E572FDD
> Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57
> 2FDD
