Hi Ying and Xin, Any viewpoints on this before I send it in? It should be noted that skb->tail in the worst case will be the same as skb->end, which points to the first byte of the skb_shared_info area. Lucklily, and not only due to luck, I think, this byte happens to be named " __unused" in that structure. So the change should be safe, and my tests have not revealed any problems. Whether this really solves the problem reported by syzbot I don't know, since I am unable to reproduce it, but this is my take on it.
Regards ///jon -----Original Message----- From: Jon Maloy Sent: 18-Dec-18 15:29 To: Jon Maloy <[email protected]>; Jon Maloy <[email protected]> Cc: Mohan Krishna Ghanta Krishnamurthy <[email protected]>; [email protected]; Tung Quang Nguyen <[email protected]>; Hoang Huu Le <[email protected]>; Canh Duc Luu <[email protected]>; Tuong Tong Lien <[email protected]>; Gordan Mihaljevic <[email protected]>; [email protected]; [email protected] Subject: [net 1/1] tipc: sanity check on received netlink buffer When tipc receives a sk buffer in tipc_net_link_compat_rcv() it performs no controls that the buffer has the required minimum size. Furthermore, the buffer may contain a string, which we have no guarantee is zero- terminated. We now introduce a check that the buffer at least is large enough to contain a generic and a TIPC specific netlink header, since those must be present in all valid messages. We also set the buffer tail to point to a zero character. This ensures that subsequent string operations on buffer data never can fail, even if the given string is invalid. Reported-by: [email protected] Reported-by: [email protected] Signed-off-by: Jon Maloy <[email protected]> --- net/tipc/netlink_compat.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/net/tipc/netlink_compat.c b/net/tipc/netlink_compat.c index 6376467..b37ed6e 100644 --- a/net/tipc/netlink_compat.c +++ b/net/tipc/netlink_compat.c @@ -1188,6 +1188,13 @@ static int tipc_nl_compat_recv(struct sk_buff *skb, struct genl_info *info) memset(&msg, 0, sizeof(msg)); + if (skb_headlen(skb) < GENL_HDRLEN + TIPC_GENL_HDRLEN) { + err = -EINVAL; + msg.rep = tipc_get_err_tlv(TIPC_CFG_TLV_ERROR); + goto send; + } + *skb_tail_pointer(skb) = 0; + req_nlh = (struct nlmsghdr *)skb->data; msg.req = nlmsg_data(req_nlh) + GENL_HDRLEN + TIPC_GENL_HDRLEN; msg.cmd = req_userhdr->cmd; -- 2.1.4 _______________________________________________ tipc-discussion mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/tipc-discussion
