Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 527feae5 authored by Pavel Skripkin's avatar Pavel Skripkin Committed by Greg Kroah-Hartman
Browse files

net: llc: fix skb_over_panic 



[ Upstream commit c7c9d2102c9c098916ab9e0ab248006107d00d6c ]

Syzbot reported skb_over_panic() in llc_pdu_init_as_xid_cmd(). The
problem was in wrong LCC header manipulations.

Syzbot's reproducer tries to send XID packet. llc_ui_sendmsg() is
doing following steps:

	1. skb allocation with size = len + header size
		len is passed from userpace and header size
		is 3 since addr->sllc_xid is set.

	2. skb_reserve() for header_len = 3
	3. filling all other space with memcpy_from_msg()

Ok, at this moment we have fully loaded skb, only headers needs to be
filled.

Then code comes to llc_sap_action_send_xid_c(). This function pushes 3
bytes for LLC PDU header and initializes it. Then comes
llc_pdu_init_as_xid_cmd(). It initalizes next 3 bytes *AFTER* LLC PDU
header and call skb_push(skb, 3). This looks wrong for 2 reasons:

	1. Bytes rigth after LLC header are user data, so this function
	   was overwriting payload.

	2. skb_push(skb, 3) call can cause skb_over_panic() since
	   all free space was filled in llc_ui_sendmsg(). (This can
	   happen is user passed 686 len: 686 + 14 (eth header) + 3 (LLC
	   header) = 703. SKB_DATA_ALIGN(703) = 704)

So, in this patch I added 2 new private constansts: LLC_PDU_TYPE_U_XID
and LLC_PDU_LEN_U_XID. LLC_PDU_LEN_U_XID is used to correctly reserve
header size to handle LLC + XID case. LLC_PDU_TYPE_U_XID is used by
llc_pdu_header_init() function to push 6 bytes instead of 3. And finally
I removed skb_push() call from llc_pdu_init_as_xid_cmd().

This changes should not affect other parts of LLC, since after
all steps we just transmit buffer.

Fixes: 1da177e4 ("Linux-2.6.12-rc2")
Reported-and-tested-by: default avatar <syzbot+5e5a981ad7cc54c4b2b4@syzkaller.appspotmail.com>
Signed-off-by: default avatarPavel Skripkin <paskripkin@gmail.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
Signed-off-by: default avatarSasha Levin <sashal@kernel.org>
parent ede4c938

include/net/llc_pdu.h

+23 −8
Original line number Diff line number Diff line
@@ -18,6 +18,8 @@ 
#define LLC_PDU_LEN_I		4       /* header and 2 control bytes */

#define LLC_PDU_LEN_S		4

#define LLC_PDU_LEN_U		3       /* header and 1 control byte */

/* header and 1 control byte and XID info */

#define LLC_PDU_LEN_U_XID	(LLC_PDU_LEN_U + sizeof(struct llc_xid_info))

/* Known SAP addresses */

#define LLC_GLOBAL_SAP	0xFF

#define LLC_NULL_SAP	0x00	/* not network-layer visible */
@@ -53,6 +55,7 @@ 
#define LLC_PDU_TYPE_I		0	/* first bit */

#define LLC_PDU_TYPE_S		1	/* first two bits */

#define LLC_PDU_TYPE_U		3	/* first two bits */

#define LLC_PDU_TYPE_U_XID	4	/* private type for detecting XID commands */



#define LLC_PDU_TYPE_IS_I(pdu) \

	((!(pdu->ctrl_1 & LLC_PDU_TYPE_I_MASK)) ? 1 : 0)
@@ -230,9 +233,18 @@ static inline struct llc_pdu_un *llc_pdu_un_hdr(struct sk_buff *skb) 
static inline void llc_pdu_header_init(struct sk_buff *skb, u8 type,

				       u8 ssap, u8 dsap, u8 cr)

{

	const int hlen = type == LLC_PDU_TYPE_U ? 3 : 4;

	int hlen = 4; /* default value for I and S types */

	struct llc_pdu_un *pdu;



	switch (type) {

	case LLC_PDU_TYPE_U:

		hlen = 3;

		break;

	case LLC_PDU_TYPE_U_XID:

		hlen = 6;

		break;

	}



	skb_push(skb, hlen);

	skb_reset_network_header(skb);

	pdu = llc_pdu_un_hdr(skb);
@@ -374,7 +386,10 @@ static inline void llc_pdu_init_as_xid_cmd(struct sk_buff *skb, 
	xid_info->fmt_id = LLC_XID_FMT_ID;	/* 0x81 */

	xid_info->type	 = svcs_supported;

	xid_info->rw	 = rx_window << 1;	/* size of receive window */

	skb_put(skb, sizeof(struct llc_xid_info));



	/* no need to push/put since llc_pdu_header_init() has already

	 * pushed 3 + 3 bytes

	 */

}



/**

net/llc/af_llc.c

+9 −1
Original line number Diff line number Diff line
@@ -98,8 +98,16 @@ static inline u8 llc_ui_header_len(struct sock *sk, struct sockaddr_llc *addr) 
{

	u8 rc = LLC_PDU_LEN_U;



	if (addr->sllc_test || addr->sllc_xid)

	if (addr->sllc_test)

		rc = LLC_PDU_LEN_U;

	else if (addr->sllc_xid)

		/* We need to expand header to sizeof(struct llc_xid_info)

		 * since llc_pdu_init_as_xid_cmd() sets 4,5,6 bytes of LLC header

		 * as XID PDU. In llc_ui_sendmsg() we reserved header size and then

		 * filled all other space with user data. If we won't reserve this

		 * bytes, llc_pdu_init_as_xid_cmd() will overwrite user data

		 */

		rc = LLC_PDU_LEN_U_XID;

	else if (sk->sk_type == SOCK_STREAM)

		rc = LLC_PDU_LEN_I;

	return rc;

net/llc/llc_s_ac.c

+1 −1
Original line number Diff line number Diff line
@@ -79,7 +79,7 @@ int llc_sap_action_send_xid_c(struct llc_sap *sap, struct sk_buff *skb) 
	struct llc_sap_state_ev *ev = llc_sap_ev(skb);

	int rc;



	llc_pdu_header_init(skb, LLC_PDU_TYPE_U, ev->saddr.lsap,

	llc_pdu_header_init(skb, LLC_PDU_TYPE_U_XID, ev->saddr.lsap,

			    ev->daddr.lsap, LLC_PDU_CMD);

	llc_pdu_init_as_xid_cmd(skb, LLC_XID_NULL_CLASS_2, 0);

	rc = llc_mac_hdr_init(skb, ev->saddr.mac, ev->daddr.mac);