Re: [PATCH mptcp-next v2 1/2] mptcp: allow priviledged operations from user namespaces

[Matthieu Baerts <matthieu.baerts@tessares.net>]

Hi Thomas, Mat, Florian,

On 11/08/2022 01:08, Mat Martineau wrote:
> On Wed, 10 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>
> 
> Thanks Thomas, looks good:
> 
> Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com>

Thank you for the patches and the reviews!

> Florian had mentioned preferring net-next in the discussion of v1 (which
> Thomas has agreed with by labeling this for mptcp-next), and I agree. I
> don't think it quite meets the bar for -net or stable backporting and it
> would be easier to explain that "6.1 and later support user namespaces
> for MPTCP generic netlink commands". (Matthieu, if someone makes a
> convincing case for -net, it's up to you :) )

I initially thought it was more a bug-fix but I also agree with Florian.
Also I see that the two mentioned commits above don't have a Fixes tag.

The only exception I found and related to this flag was for l2tp: the
commit 2abe05234f2e ("l2tp: Allow management of tunnels and session in
user namespace") has been selected by davem and backported to v5.4.

https://lore.kernel.org/stable/20200417.105100.821338189941807731.davem@davemloft.net/
(see patch 03/19)


Anyway, your 2 patches are now in our tree (feat. for net-next) with
Mat's RvB tag and without a typo (s/priviledged/privileged/) + a small
fix for checkpatch related to how the commit are mentioned, nothing
important:


New patches for t/upstream:
- 3d542a6c45ea: mptcp: allow privileged operations from user namespaces
- 11bdb1959854: mptcp: account memory allocation in
mptcp_nl_cmd_add_addr() to user
- Results: c4a0ae952875..004104cc8a77 (export)


Tests are now in progress:





https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20220811T101116


@Thomas: BTW, thank you for maintaining libnl!
Funny that you sent these patches to MPTCP while earlier this week I
sent the latest libnl version to Debian :)

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1016867
https://tracker.debian.org/pkg/libnl3


Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net