llc: fix another potential sk_buff leak in llc_ui_sendmsg()

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>

diff --git a/net/llc/af_llc.c b/net/llc/af_llc.c
index 2017b7d780f5af73c1ac7461113842776d1b00fc..c74f44dfaa22a5020880ea218c6607ace8fd5e22 100644 (file)
--- 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)
@@ -899,7 +903,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;
 
@@ -908,10 +912,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. */
@@ -919,7 +923,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;
@@ -928,12 +932,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);
@@ -943,29 +947,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 ed2aca12460ca870d8b7b38b8a9aff0e923cf7c5..0b0c6f12153b0e2a3ccc6d4f80e609d8f9e1d80e 100644 (file)
--- 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 8db03c2d5440b12536b3b7016bc5c01b56666d8a..ad6547736c219dafe42c2014257c0c87f372934f 100644 (file)
--- 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;
 }