On Wed, Mar 23, 2022 at 02:22:07PM +0100, Florian Westphal wrote:
> I'd like to be able to switch lifetime management of ctnl_timeout
> to free-on-zero-refcount.
>
> This isn't possible at the moment because removal of the structures
> from the pernet list requires the nfnl mutex and release may happen from
> softirq.
>
> Current solution is to prevent this by disallowing policy object removal
> if the refcount is > 1 (i.e., policy is still referenced from the ruleset).
>
> Switch traversal to rcu-read-lock as a first step to reduce reliance on
> nfnl mutex protection: removal from softirq would require a extra list
> spinlock.
Needs .type = NFNL_CB_RCU?
> Signed-off-by: Florian Westphal <fw@xxxxxxxxx>
> ---
> net/netfilter/nfnetlink_cttimeout.c | 27 ++++++++++++++-------------
> 1 file changed, 14 insertions(+), 13 deletions(-)
>
> diff --git a/net/netfilter/nfnetlink_cttimeout.c b/net/netfilter/nfnetlink_cttimeout.c
> index eea486f32971..aef2547bb579 100644
> --- a/net/netfilter/nfnetlink_cttimeout.c
> +++ b/net/netfilter/nfnetlink_cttimeout.c
> @@ -253,6 +253,7 @@ static int cttimeout_get_timeout(struct sk_buff *skb,
> const struct nlattr * const cda[])
> {
> struct nfct_timeout_pernet *pernet = nfct_timeout_pernet(info->net);
> + struct sk_buff *skb2;
> int ret = -ENOENT;
> char *name;
> struct ctnl_timeout *cur;
> @@ -268,31 +269,31 @@ static int cttimeout_get_timeout(struct sk_buff *skb,
> return -EINVAL;
> name = nla_data(cda[CTA_TIMEOUT_NAME]);
>
> - list_for_each_entry(cur, &pernet->nfct_timeout_list, head) {
> - struct sk_buff *skb2;
> + skb2 = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
> + if (!skb2)
> + return -ENOMEM;
> +
> + rcu_read_lock();
>
> + list_for_each_entry_rcu(cur, &pernet->nfct_timeout_list, head) {
> if (strncmp(cur->name, name, CTNL_TIMEOUT_NAME_MAX) != 0)
> continue;
>
> - skb2 = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
> - if (skb2 == NULL) {
> - ret = -ENOMEM;
> - break;
> - }
> -
> ret = ctnl_timeout_fill_info(skb2, NETLINK_CB(skb).portid,
> info->nlh->nlmsg_seq,
> NFNL_MSG_TYPE(info->nlh->nlmsg_type),
> IPCTNL_MSG_TIMEOUT_NEW, cur);
> - if (ret <= 0) {
> - kfree_skb(skb2);
> + if (ret <= 0)
> break;
> - }
>
> - ret = nfnetlink_unicast(skb2, info->net, NETLINK_CB(skb).portid);
> - break;
> + rcu_read_unlock();
> +
> + return nfnetlink_unicast(skb2, info->net, NETLINK_CB(skb).portid);
> }
>
> + rcu_read_unlock();
> + kfree_skb(skb2);
> +
> return ret;
> }
>
> --
> 2.34.1
>
