• Kumar Kartikeya Dwivedi's avatar
    net: sched: bump refcount for new action in ACT replace mode · 6855e821
    Kumar Kartikeya Dwivedi authored
    Currently, action creation using ACT API in replace mode is buggy.
    When invoking for non-existent action index 42,
    
    	tc action replace action bpf obj foo.o sec <xyz> index 42
    
    kernel creates the action, fills up the netlink response, and then just
    deletes the action after notifying userspace.
    
    	tc action show action bpf
    
    doesn't list the action.
    
    This happens due to the following sequence when ovr = 1 (replace mode)
    is enabled:
    
    tcf_idr_check_alloc is used to atomically check and either obtain
    reference for existing action at index, or reserve the index slot using
    a dummy entry (ERR_PTR(-EBUSY)).
    
    This is necessary as pointers to these actions will be held after
    dropping the idrinfo lock, so bumping the reference count is necessary
    as we need to insert the actions, and notify userspace by dumping their
    attributes. Finally, we drop the reference we took using the
    tcf_action_put_many call in tcf_action_add. However, for the case where
    a new action is created due to free index, its refcount remains one.
    This when paired with the put_many call leads to the kernel setting up
    the action, notifying userspace of its creation, and then tearing it
    down. For existing actions, the refcount is still held so they remain
    unaffected.
    
    Fortunately due to rtnl_lock serialization requirement, such an action
    with refcount == 1 will not be concurrently deleted by anything else, at
    best CLS API can move its refcount up and down by binding to it after it
    has been published from tcf_idr_insert_many. Since refcount is atleast
    one until put_many call, CLS API cannot delete it. Also __tcf_action_put
    release path already ensures deterministic outcome (either new action
    will be created or existing action will be reused in case CLS API tries
    to bind to action concurrently) due to idr lock serialization.
    
    We fix this by making refcount of newly created actions as 2 in ACT API
    replace mode. A relaxed store will suffice as visibility is ensured only
    after the tcf_idr_insert_many call.
    
    Note that in case of creation or overwriting using CLS API only (i.e.
    bind = 1), overwriting existing action object is not allowed, and any
    such request is silently ignored (without error).
    
    The refcount bump that occurs in tcf_idr_check_alloc call there for
    existing action will pair with tcf_exts_destroy call made from the
    owner module for the same action. In case of action creation, there
    is no existing action, so no tcf_exts_destroy callback happens.
    
    This means no code changes for CLS API.
    
    Fixes: cae422f3 ("net: sched: use reference counting action init")
    Signed-off-by: default avatarKumar Kartikeya Dwivedi <memxor@gmail.com>
    Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
    6855e821
act_api.c 40.4 KB