Re: [PATCH nf-next,v2] gtp: add initial driver for datapath of GPRS Tunneling Protocol (GTP-U)

On Mon, 2016-05-09 at 00:55 +0200, Pablo Neira Ayuso wrote:

+static int gtp1u_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb,

• 		bool xnet)
  

+{

• unsigned int hdrlen = sizeof(struct udphdr) +

• 	      sizeof(struct gtp1_header);
  

• struct gtp1_header *gtp1;
• struct pdp_ctx *pctx;
• int ret = 0;
• if (!pskb_may_pull(skb, hdrlen))

• return -1;
  

• gtp1 = (struct gtp1_header *)(skb->data + sizeof(struct udphdr));
• if ((gtp1->flags >> 5) != GTP_V1)

• return 1;
  

• if (gtp1->type != GTP_TPDU)

• return 1;
  

• /* From 29.060: "This field shall be present if and only if any one or

• * more of the S, PN and E flags are set.".
  

• *
  

• * If any of the bit is set, then the remaining ones also have to be
  

• * set.
  

• */
  

• if (gtp1->flags & GTP1_F_MASK)

• hdrlen += 4;
  

• /* Make sure the header is larger enough, including extensions. */
• if (!pskb_may_pull(skb, hdrlen))

• return -1;
  

You need to reload gtp1 here, as the previous pskb_may_pull() might have reallocated skb->head

• rcu_read_lock();
• pctx = gtp1_pdp_find(gtp, ntohl(gtp1->tid));

Or risk a use after free here.

• if (!pctx) {

• netdev_dbg(gtp->dev, "No PDP ctx to decap skb=%p\n", skb);
  

• ret = -1;
  

• goto out_rcu;
  

• }
• if (!gtp_check_src_ms(skb, pctx, hdrlen)) {

• netdev_dbg(gtp->dev, "No PDP ctx for this MS\n");
  

• ret = -1;
  

• goto out_rcu;
  

• }
• rcu_read_unlock();
• /* Get rid of the GTP + UDP headers. */
• return iptunnel_pull_header(skb, hdrlen, skb->protocol, xnet);

+out_rcu:

• rcu_read_unlock();
• return ret;

+}