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?
--
Doug Ledford <dledford@xxxxxxxxxx>
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57
2FDD
Attachment:
signature.asc
Description: This is a digitally signed message part
