On Tue, Jan 09, 2018 at 01:09:17PM -0700, Jason Gunthorpe wrote: > On Tue, Jan 02, 2018 at 10:18:32AM +0200, Leon Romanovsky wrote: > > > + /* PID == 0 means that this QP was created by kernel */ > > + if (qp->res.pid && nla_put_u32(msg, > > + RDMA_NLDEV_ATTR_RES_PID, qp->res.pid)) > > This is returning a pid in the init name space, obtained here: > > + res->pid = task_pid_nr(current); > > And since the netlink user is not running in the init name space this > will return the wrong pid #, and worse potentially pids the current > name space should not see. > > This API also needs to filter the results and only return pids > visible, and translate the pids as well.. Correct, PID namespace wasn't taken into account, exactly as it wasn't taken in CMA. So, right now, CMA netlink statistics is returning wrong and unfiltered PIDs. How do you want to progress with that part of the code? I personally have no plans to fix CMA netlink code and for my opinion it should be removed, instead of beating that dead horse. > > I also suspsect this needs to be a netlink array of pids for future, > as we have hope someday to have RDMA uobjects shared between multiple > processes? Not really, there is no such feature yet and once it will be introduced the authors will need to update netlink interface and rdmatool to support multiple PID option. They will add new netlink attribute and fill it for their shared objects. For most users of RDMA stack, this shared object thing is not needed and better to avoid from over-engineering. > > > + if (nla_put_string(msg, > > + RDMA_NLDEV_ATTR_RES_PID_COMM, qp->res.task_comm)) > > + goto err; > > Feels odd to return the content of /proc/XX/comm in netlink? > It was just to save extra syscalls for every object. I'll remove it for the user created objects to simplify PID namespaces support. > Jason
Attachment:
signature.asc
Description: PGP signature
