31 Aug, 2012

1 commit

  • Existing code assumes that del_timer returns true for alive conntrack
    entries. However, this is not true if reliable events are enabled.
    In that case, del_timer may return true for entries that were
    just inserted in the dying list. Note that packets / ctnetlink may
    hold references to conntrack entries that were just inserted to such
    list.

    This patch fixes the issue by adding an independent timer for
    event delivery. This increases the size of the ecache extension.
    Still we can revisit this later and use variable size extensions
    to allocate this area on demand.

    Tested-by: Oliver Smith
    Signed-off-by: Pablo Neira Ayuso

    Pablo Neira Ayuso
     

30 Aug, 2012

3 commits

  • Initialize return variable before exiting on an error path.

    A simplified version of the semantic match that finds this problem is as
    follows: (http://coccinelle.lip6.fr/)

    //
    (
    if@p1 (\(ret < 0\|ret != 0\))
    { ... return ret; }
    |
    ret@p1 = 0
    )
    ... when != ret = e1
    when != &ret
    *if(...)
    {
    ... when != ret = e2
    when forall
    return ret;
    }

    //

    Signed-off-by: Julia Lawall
    Signed-off-by: Pablo Neira Ayuso

    Julia Lawall
     
  • Initialize return variable before exiting on an error path.

    A simplified version of the semantic match that finds this problem is as
    follows: (http://coccinelle.lip6.fr/)

    //
    (
    if@p1 (\(ret < 0\|ret != 0\))
    { ... return ret; }
    |
    ret@p1 = 0
    )
    ... when != ret = e1
    when != &ret
    *if(...)
    {
    ... when != ret = e2
    when forall
    return ret;
    }

    //

    Signed-off-by: Julia Lawall
    Signed-off-by: Pablo Neira Ayuso

    Julia Lawall
     
  • Initialize return variable before exiting on an error path.

    A simplified version of the semantic match that finds this problem is as
    follows: (http://coccinelle.lip6.fr/)

    //
    (
    if@p1 (\(ret < 0\|ret != 0\))
    { ... return ret; }
    |
    ret@p1 = 0
    )
    ... when != ret = e1
    when != &ret
    *if(...)
    {
    ... when != ret = e2
    when forall
    return ret;
    }

    //

    Signed-off-by: Julia Lawall
    Acked-by: Simon Horman
    Signed-off-by: Pablo Neira Ayuso

    Julia Lawall
     

20 Aug, 2012

2 commits

  • Commit 1db20a52 (nfnetlink_log: Stop using NLA_PUT*().) incorrectly
    converted a NLA_PUT_BE16 macro to nla_put_be32() in nfnetlink_log:

    - NLA_PUT_BE16(inst->skb, NFULA_HWTYPE, htons(skb->dev->type));
    + if (nla_put_be32(inst->skb, NFULA_HWTYPE, htons(skb->dev->type))

    Signed-off-by: Patrick McHardy
    Signed-off-by: Pablo Neira Ayuso

    Patrick McHardy
     
  • Pable Neira Ayuso says:

    ====================
    The following five patches contain fixes for 3.6-rc, they are:

    * Two fixes for message parsing in the SIP conntrack helper, from
    Patrick McHardy.

    * One fix for the SIP helper introduced in the user-space cthelper
    infrastructure, from Patrick McHardy.

    * fix missing appropriate locking while modifying one conntrack entry
    from the nfqueue integration code, from myself.

    * fix possible access to uninitiliazed timer in the nf_conntrack
    expectation infrastructure, from myself.
    ====================

    Signed-off-by: David S. Miller

    David S. Miller
     

16 Aug, 2012

2 commits

  • In __nf_ct_expect_check, the function refresh_timer returns 1
    if a matching expectation is found and its timer is successfully
    refreshed. This results in nf_ct_expect_related returning 0.
    Note that at this point:

    - the passed expectation is not inserted in the expectation table
    and its timer was not initialized, since we have refreshed one
    matching/existing expectation.

    - nf_ct_expect_alloc uses kmem_cache_alloc, so the expectation
    timer is in some undefined state just after the allocation,
    until it is appropriately initialized.

    This can be a problem for the SIP helper during the expectation
    addition:

    ...
    if (nf_ct_expect_related(rtp_exp) == 0) {
    if (nf_ct_expect_related(rtcp_exp) != 0)
    nf_ct_unexpect_related(rtp_exp);
    ...

    Note that nf_ct_expect_related(rtp_exp) may return 0 for the timer refresh
    case that is detailed above. Then, if nf_ct_unexpect_related(rtcp_exp)
    returns != 0, nf_ct_unexpect_related(rtp_exp) is called, which does:

    spin_lock_bh(&nf_conntrack_lock);
    if (del_timer(&exp->timeout)) {
    nf_ct_unlink_expect(exp);
    nf_ct_expect_put(exp);
    }
    spin_unlock_bh(&nf_conntrack_lock);

    Note that del_timer always returns false if the timer has been
    initialized. However, the timer was not initialized since setup_timer
    was not called, therefore, the expectation timer remains in some
    undefined state. If I'm not missing anything, this may lead to the
    removal an unexistent expectation.

    To fix this, the optimization that allows refreshing an expectation
    is removed. Now nf_conntrack_expect_related looks more consistent
    to me since it always add the expectation in case that it returns
    success.

    Thanks to Patrick McHardy for participating in the discussion of
    this patch.

    I think this may be the source of the problem described by:
    http://marc.info/?l=netfilter-devel&m=134073514719421&w=2

    Reported-by: Rafal Fitt
    Acked-by: Patrick McHardy
    Signed-off-by: Pablo Neira Ayuso

    Pablo Neira Ayuso
     
  • If at least one of CONFIG_IP_VS_PROTO_TCP or CONFIG_IP_VS_PROTO_UDP is
    not set, __ip_vs_get_timeouts() does not fully initialize the structure
    that gets copied to userland and that for leaks up to 12 bytes of kernel
    stack. Add an explicit memset(0) before passing the structure to
    __ip_vs_get_timeouts() to avoid the info leak.

    Signed-off-by: Mathias Krause
    Cc: Wensong Zhang
    Cc: Simon Horman
    Cc: Julian Anastasov
    Signed-off-by: David S. Miller

    Mathias Krause
     

14 Aug, 2012

1 commit


10 Aug, 2012

2 commits

  • Within SIP messages IPv6 addresses are enclosed in square brackets in most
    cases, with the exception of the "received=" header parameter. Currently
    the helper fails to parse enclosed addresses.

    This patch:

    - changes the SIP address parsing function to enforce square brackets
    when required, and accept them when not required but present, as
    recommended by RFC 5118.

    - adds a new SDP address parsing function that never accepts square
    brackets since SDP doesn't use them.

    With these changes, the SIP helper correctly parses all test messages
    from RFC 5118 (Session Initiation Protocol (SIP) Torture Test Messages
    for Internet Protocol Version 6 (IPv6)).

    Signed-off-by: Patrick McHardy
    Signed-off-by: Pablo Neira Ayuso

    Patrick McHardy
     
  • Commit 3a8fc53a (netfilter: nf_ct_helper: allocate 16 bytes for the helper
    and policy names) introduced a bug in the SIP helper, the helper name is
    sprinted to the sip_names array instead of instead of into the helper
    structure. This breaks the helper match and the /proc/net/nf_conntrack_expect
    output.

    Signed-off-by: Patrick McHardy
    Signed-off-by: Pablo Neira Ayuso

    Patrick McHardy
     

20 Jul, 2012

1 commit


17 Jul, 2012

2 commits

  • This will be used so that we can compose a full flow key.

    Even though we have a route in this context, we need more. In the
    future the routes will be without destination address, source address,
    etc. keying. One ipv4 route will cover entire subnets, etc.

    In this environment we have to have a way to possess persistent storage
    for redirects and PMTU information. This persistent storage will exist
    in the FIB tables, and that's why we'll need to be able to rebuild a
    full lookup flow key here. Using that flow key will do a fib_lookup()
    and create/update the persistent entry.

    Signed-off-by: David S. Miller

    David S. Miller
     
  • After commit 39f618b4fd95ae243d940ec64c961009c74e3333 (3.4)
    "ipvs: reset ipvs pointer in netns" we can oops in
    ip_vs_dst_event on rmmod ip_vs because ip_vs_control_cleanup
    is called after the ipvs_core_ops subsys is unregistered and
    net->ipvs is NULL. Fix it by exiting early from ip_vs_dst_event
    if ipvs is NULL. It is safe because all services and dests
    for the net are already freed.

    Signed-off-by: Julian Anastasov
    Signed-off-by: Simon Horman
    Signed-off-by: Pablo Neira Ayuso

    Julian Anastasov
     

11 Jul, 2012

2 commits


09 Jul, 2012

1 commit


08 Jul, 2012

1 commit


05 Jul, 2012

5 commits


30 Jun, 2012

2 commits

  • This patch adds a hook in the binding path of netlink.

    This is used by ctnetlink to allow module autoloading for the case
    in which one user executes:

    conntrack -E

    So far, this resulted in nfnetlink loaded, but not
    nf_conntrack_netlink.

    I have received in the past many complains on this behaviour.

    Signed-off-by: Pablo Neira Ayuso
    Signed-off-by: David S. Miller

    Pablo Neira Ayuso
     
  • This patch adds the following structure:

    struct netlink_kernel_cfg {
    unsigned int groups;
    void (*input)(struct sk_buff *skb);
    struct mutex *cb_mutex;
    };

    That can be passed to netlink_kernel_create to set optional configurations
    for netlink kernel sockets.

    I've populated this structure by looking for NULL and zero parameters at the
    existing code. The remaining parameters that always need to be set are still
    left in the original interface.

    That includes optional parameters for the netlink socket creation. This allows
    easy extensibility of this interface in the future.

    This patch also adapts all callers to use this new interface.

    Signed-off-by: Pablo Neira Ayuso
    Signed-off-by: David S. Miller

    Pablo Neira Ayuso
     

29 Jun, 2012

2 commits


28 Jun, 2012

11 commits


27 Jun, 2012

2 commits