lkml.org 
[lkml]   [2020]   [Jan]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.14 318/343] llc: fix another potential sk_buff leak in llc_ui_sendmsg()
    Date
    From: Eric Biggers <ebiggers@google.com>

    [ Upstream commit fc8d5db10cbe1338a52ebc74e7feab9276721774 ]

    All callers of llc_conn_state_process() except llc_build_and_send_pkt()
    (via llc_ui_sendmsg() -> llc_ui_send_data()) assume that it always
    consumes a reference to the skb. Fix this caller to do the same.

    Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
    Signed-off-by: Eric Biggers <ebiggers@google.com>
    Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
    Signed-off-by: Sasha Levin <sashal@kernel.org>
    ---
    net/llc/af_llc.c | 34 ++++++++++++++++++++--------------
    net/llc/llc_conn.c | 2 ++
    net/llc/llc_if.c | 12 ++++++++----
    3 files changed, 30 insertions(+), 18 deletions(-)

    diff --git a/net/llc/af_llc.c b/net/llc/af_llc.c
    index 2e472d5c3ea41..d552e88197133 100644
    --- a/net/llc/af_llc.c
    +++ b/net/llc/af_llc.c
    @@ -113,22 +113,26 @@ static inline u8 llc_ui_header_len(struct sock *sk, struct sockaddr_llc *addr)
    *
    * Send data via reliable llc2 connection.
    * Returns 0 upon success, non-zero if action did not succeed.
    + *
    + * This function always consumes a reference to the skb.
    */
    static int llc_ui_send_data(struct sock* sk, struct sk_buff *skb, int noblock)
    {
    struct llc_sock* llc = llc_sk(sk);
    - int rc = 0;

    if (unlikely(llc_data_accept_state(llc->state) ||
    llc->remote_busy_flag ||
    llc->p_flag)) {
    long timeout = sock_sndtimeo(sk, noblock);
    + int rc;

    rc = llc_ui_wait_for_busy_core(sk, timeout);
    + if (rc) {
    + kfree_skb(skb);
    + return rc;
    + }
    }
    - if (unlikely(!rc))
    - rc = llc_build_and_send_pkt(sk, skb);
    - return rc;
    + return llc_build_and_send_pkt(sk, skb);
    }

    static void llc_ui_sk_init(struct socket *sock, struct sock *sk)
    @@ -900,7 +904,7 @@ static int llc_ui_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
    DECLARE_SOCKADDR(struct sockaddr_llc *, addr, msg->msg_name);
    int flags = msg->msg_flags;
    int noblock = flags & MSG_DONTWAIT;
    - struct sk_buff *skb;
    + struct sk_buff *skb = NULL;
    size_t size = 0;
    int rc = -EINVAL, copied = 0, hdrlen;

    @@ -909,10 +913,10 @@ static int llc_ui_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
    lock_sock(sk);
    if (addr) {
    if (msg->msg_namelen < sizeof(*addr))
    - goto release;
    + goto out;
    } else {
    if (llc_ui_addr_null(&llc->addr))
    - goto release;
    + goto out;
    addr = &llc->addr;
    }
    /* must bind connection to sap if user hasn't done it. */
    @@ -920,7 +924,7 @@ static int llc_ui_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
    /* bind to sap with null dev, exclusive. */
    rc = llc_ui_autobind(sock, addr);
    if (rc)
    - goto release;
    + goto out;
    }
    hdrlen = llc->dev->hard_header_len + llc_ui_header_len(sk, addr);
    size = hdrlen + len;
    @@ -929,12 +933,12 @@ static int llc_ui_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
    copied = size - hdrlen;
    rc = -EINVAL;
    if (copied < 0)
    - goto release;
    + goto out;
    release_sock(sk);
    skb = sock_alloc_send_skb(sk, size, noblock, &rc);
    lock_sock(sk);
    if (!skb)
    - goto release;
    + goto out;
    skb->dev = llc->dev;
    skb->protocol = llc_proto_type(addr->sllc_arphrd);
    skb_reserve(skb, hdrlen);
    @@ -944,29 +948,31 @@ static int llc_ui_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
    if (sk->sk_type == SOCK_DGRAM || addr->sllc_ua) {
    llc_build_and_send_ui_pkt(llc->sap, skb, addr->sllc_mac,
    addr->sllc_sap);
    + skb = NULL;
    goto out;
    }
    if (addr->sllc_test) {
    llc_build_and_send_test_pkt(llc->sap, skb, addr->sllc_mac,
    addr->sllc_sap);
    + skb = NULL;
    goto out;
    }
    if (addr->sllc_xid) {
    llc_build_and_send_xid_pkt(llc->sap, skb, addr->sllc_mac,
    addr->sllc_sap);
    + skb = NULL;
    goto out;
    }
    rc = -ENOPROTOOPT;
    if (!(sk->sk_type == SOCK_STREAM && !addr->sllc_ua))
    goto out;
    rc = llc_ui_send_data(sk, skb, noblock);
    + skb = NULL;
    out:
    - if (rc) {
    - kfree_skb(skb);
    -release:
    + kfree_skb(skb);
    + if (rc)
    dprintk("%s: failed sending from %02X to %02X: %d\n",
    __func__, llc->laddr.lsap, llc->daddr.lsap, rc);
    - }
    release_sock(sk);
    return rc ? : copied;
    }
    diff --git a/net/llc/llc_conn.c b/net/llc/llc_conn.c
    index 444c13e752a08..7340f23e16de3 100644
    --- a/net/llc/llc_conn.c
    +++ b/net/llc/llc_conn.c
    @@ -55,6 +55,8 @@ int sysctl_llc2_busy_timeout = LLC2_BUSY_TIME * HZ;
    * (executing it's actions and changing state), upper layer will be
    * indicated or confirmed, if needed. Returns 0 for success, 1 for
    * failure. The socket lock has to be held before calling this function.
    + *
    + * This function always consumes a reference to the skb.
    */
    int llc_conn_state_process(struct sock *sk, struct sk_buff *skb)
    {
    diff --git a/net/llc/llc_if.c b/net/llc/llc_if.c
    index 6daf391b3e847..fc4d2bd8816f5 100644
    --- a/net/llc/llc_if.c
    +++ b/net/llc/llc_if.c
    @@ -38,6 +38,8 @@
    * closed and -EBUSY when sending data is not permitted in this state or
    * LLC has send an I pdu with p bit set to 1 and is waiting for it's
    * response.
    + *
    + * This function always consumes a reference to the skb.
    */
    int llc_build_and_send_pkt(struct sock *sk, struct sk_buff *skb)
    {
    @@ -46,20 +48,22 @@ int llc_build_and_send_pkt(struct sock *sk, struct sk_buff *skb)
    struct llc_sock *llc = llc_sk(sk);

    if (unlikely(llc->state == LLC_CONN_STATE_ADM))
    - goto out;
    + goto out_free;
    rc = -EBUSY;
    if (unlikely(llc_data_accept_state(llc->state) || /* data_conn_refuse */
    llc->p_flag)) {
    llc->failed_data_req = 1;
    - goto out;
    + goto out_free;
    }
    ev = llc_conn_ev(skb);
    ev->type = LLC_CONN_EV_TYPE_PRIM;
    ev->prim = LLC_DATA_PRIM;
    ev->prim_type = LLC_PRIM_TYPE_REQ;
    skb->dev = llc->dev;
    - rc = llc_conn_state_process(sk, skb);
    -out:
    + return llc_conn_state_process(sk, skb);
    +
    +out_free:
    + kfree_skb(skb);
    return rc;
    }

    --
    2.20.1


    \
     
     \ /
      Last update: 2020-01-24 11:07    [W:2.625 / U:0.976 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site