Commit fc8d5db1 authored by Eric Biggers's avatar Eric Biggers Committed by Jakub Kicinski
Browse files

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: 1da177e4 ("Linux-2.6.12-rc2")
Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
Signed-off-by: default avatarJakub Kicinski <jakub.kicinski@netronome.com>
parent b74555de
Loading
Loading
Loading
Loading
+20 −14
Original line number Diff line number Diff line
@@ -113,23 +113,27 @@ 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 (unlikely(!rc))
		rc = llc_build_and_send_pkt(sk, skb);
		if (rc) {
			kfree_skb(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:
	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;
}
+2 −0
Original line number Diff line number Diff line
@@ -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)
{
+8 −4
Original line number Diff line number Diff line
@@ -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;
}