Re: [PATCH] mptcp: allow priviledged operations from user namespaces

[Florian Westphal <fw@strlen.de>]

Florian Westphal <fw@strlen.de> wrote:
> Mat Martineau <mathew.j.martineau@linux.intel.com> wrote:
> > On Fri, 5 Aug 2022, Thomas Haller wrote:
> > 
> > > GENL_ADMIN_PERM checks that the user has CAP_NET_ADMIN in the initial
> > > namespace by calling netlink_capable(). Instead, use GENL_UNS_ADMIN_PERM
> > > which uses netlink_ns_capable(). This checks that the caller has
> > > CAP_NET_ADMIN in the current user namespace.
> > > 
> > > See also commit 4a92602aa1cd ('openvswitch: allow management from inside
> > > user namespaces') which introduced this mechanism. See also commit
> > > 5617c6cd6f84 ('nl80211: Allow privileged operations from user
> > > namespaces'), which introduced this for nl80211.
> > > 
> > > Signed-off-by: Thomas Haller <thaller@redhat.com>
> > 
> > Hi Thomas -
> > 
> > Thanks for the patch! This does seem like a good idea to me (and it seems to
> > work ok with our MPTCP tests), but I'd like to get some more community input
> > before merging (Paolo/Florian?).
> 
> Looks good to me, I don't see any places where we'd accept e.g. a token
> from a different netns.
> 
> mptcp_nl_cmd_add_addr should probably be changed to use
> GFP_KERNEL_ACCOUNT so that the added entries can be limited via memcg.

> mptcp_userspace_pm_append_new_local_addr() helper needs a bit of rework
> for that, but I don't think this has do be done right away.

Actually its fine; the allocation is accounted vs the socket.

mptcp_nl_cmd_add_addr has the only "permanent" kmalloc that I can see.