Re: [PATCH net-next 2/2] ipvs: add ipv6 support to ftp

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Julian,

I love your patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Julian-Anastasov/Add-IPv6-support-to-IPVS-FTP-NAT/20180525-153345
reproduce:
        # apt-get install sparse
        make ARCH=x86_64 allmodconfig
        make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

>> net/netfilter/ipvs/ip_vs_ftp.c:399:24: sparse: Using plain integer as NULL pointer
   net/netfilter/ipvs/ip_vs_ftp.c:533:24: sparse: Using plain integer as NULL pointer

vim +399 net/netfilter/ipvs/ip_vs_ftp.c

   239	
   240	/* Look at outgoing ftp packets to catch the response to a PASV/EPSV command
   241	 * from the server (inside-to-outside).
   242	 * When we see one, we build a connection entry with the client address,
   243	 * client port 0 (unknown at the moment), the server address and the
   244	 * server port.  Mark the current connection entry as a control channel
   245	 * of the new entry. All this work is just to make the data connection
   246	 * can be scheduled to the right server later.
   247	 *
   248	 * The outgoing packet should be something like
   249	 *   "227 Entering Passive Mode (xxx,xxx,xxx,xxx,ppp,ppp)".
   250	 * xxx,xxx,xxx,xxx is the server address, ppp,ppp is the server port number.
   251	 * The extended format for EPSV response provides usually only port:
   252	 *   "229 Entering Extended Passive Mode (|||ppp|)"
   253	 */
   254	static int ip_vs_ftp_out(struct ip_vs_app *app, struct ip_vs_conn *cp,
   255				 struct sk_buff *skb, int *diff,
   256				 struct ip_vs_iphdr *ipvsh)
   257	{
   258		char *data, *data_limit;
   259		char *start, *end;
   260		union nf_inet_addr from;
   261		__be16 port;
   262		struct ip_vs_conn *n_cp;
   263		char buf[24];		/* xxx.xxx.xxx.xxx,ppp,ppp\000 */
   264		unsigned int buf_len;
   265		int ret = 0;
   266		enum ip_conntrack_info ctinfo;
   267		struct nf_conn *ct;
   268	
   269		*diff = 0;
   270	
   271		/* Only useful for established sessions */
   272		if (cp->state != IP_VS_TCP_S_ESTABLISHED)
   273			return 1;
   274	
   275		/* Linear packets are much easier to deal with. */
   276		if (!skb_make_writable(skb, skb->len))
   277			return 0;
   278	
   279		if (cp->app_data == (void *) IP_VS_FTP_PASV) {
   280			data = ip_vs_ftp_data_ptr(skb, ipvsh);
   281			data_limit = skb_tail_pointer(skb);
   282	
   283			if (!data || data >= data_limit)
   284				return 1;
   285	
   286			if (ip_vs_ftp_get_addrport(data, data_limit,
   287						   SERVER_STRING_PASV,
   288						   sizeof(SERVER_STRING_PASV)-1,
   289						   '(', false, IP_VS_FTP_PASV,
   290						   &from, &port, cp->af,
   291						   &start, &end) != 1)
   292				return 1;
   293	
   294			IP_VS_DBG(7, "PASV response (%pI4:%u) -> %pI4:%u detected\n",
   295				  &from.ip, ntohs(port), &cp->caddr.ip, 0);
   296		} else if (cp->app_data == (void *) IP_VS_FTP_EPSV) {
   297			data = ip_vs_ftp_data_ptr(skb, ipvsh);
   298			data_limit = skb_tail_pointer(skb);
   299	
   300			if (!data || data >= data_limit)
   301				return 1;
   302	
   303			/* Usually, data address is not specified but
   304			 * we support different address, so pre-set it.
   305			 */
   306			from = cp->daddr;
   307			if (ip_vs_ftp_get_addrport(data, data_limit,
   308						   SERVER_STRING_EPSV,
   309						   sizeof(SERVER_STRING_EPSV)-1,
   310						   '(', true, IP_VS_FTP_EPSV,
   311						   &from, &port, cp->af,
   312						   &start, &end) != 1)
   313				return 1;
   314	
   315			IP_VS_DBG_BUF(7, "EPSV response (%s:%u) -> %s:%u detected\n",
   316				      IP_VS_DBG_ADDR(cp->af, &from), ntohs(port),
   317				      IP_VS_DBG_ADDR(cp->af, &cp->caddr), 0);
   318		} else {
   319			return 1;
   320		}
   321	
   322		/* Now update or create a connection entry for it */
   323		{
   324			struct ip_vs_conn_param p;
   325	
   326			ip_vs_conn_fill_param(cp->ipvs, cp->af,
   327					      ipvsh->protocol, &from, port,
   328					      &cp->caddr, 0, &p);
   329			n_cp = ip_vs_conn_out_get(&p);
   330		}
   331		if (!n_cp) {
   332			struct ip_vs_conn_param p;
   333	
   334			ip_vs_conn_fill_param(cp->ipvs,
   335					      cp->af, ipvsh->protocol, &cp->caddr,
   336					      0, &cp->vaddr, port, &p);
   337			n_cp = ip_vs_conn_new(&p, cp->af, &from, port,
   338					      IP_VS_CONN_F_NO_CPORT |
   339					      IP_VS_CONN_F_NFCT,
   340					      cp->dest, skb->mark);
   341			if (!n_cp)
   342				return 0;
   343	
   344			/* add its controller */
   345			ip_vs_control_add(n_cp, cp);
   346		}
   347	
   348		/* Replace the old passive address with the new one */
   349		if (cp->app_data == (void *) IP_VS_FTP_PASV) {
   350			from.ip = n_cp->vaddr.ip;
   351			port = n_cp->vport;
   352			snprintf(buf, sizeof(buf), "%u,%u,%u,%u,%u,%u",
   353				 ((unsigned char *)&from.ip)[0],
   354				 ((unsigned char *)&from.ip)[1],
   355				 ((unsigned char *)&from.ip)[2],
   356				 ((unsigned char *)&from.ip)[3],
   357				 ntohs(port) >> 8,
   358				 ntohs(port) & 0xFF);
   359		} else if (cp->app_data == (void *) IP_VS_FTP_EPSV) {
   360			from = n_cp->vaddr;
   361			port = n_cp->vport;
   362			/* Only port, client will use VIP for the data connection */
   363			snprintf(buf, sizeof(buf), "|||%u|",
   364				 ntohs(port));
   365		} else {
   366			*buf = 0;
   367		}
   368		buf_len = strlen(buf);
   369	
   370		ct = nf_ct_get(skb, &ctinfo);
   371		if (ct) {
   372			bool mangled;
   373	
   374			/* If mangling fails this function will return 0
   375			 * which will cause the packet to be dropped.
   376			 * Mangling can only fail under memory pressure,
   377			 * hopefully it will succeed on the retransmitted
   378			 * packet.
   379			 */
   380			mangled = nf_nat_mangle_tcp_packet(skb, ct, ctinfo,
   381							   ipvsh->len,
   382							   start - data,
   383							   end - start,
   384							   buf, buf_len);
   385			if (mangled) {
   386				ip_vs_nfct_expect_related(skb, ct, n_cp,
   387							  ipvsh->protocol, 0, 0);
   388				if (skb->ip_summed == CHECKSUM_COMPLETE)
   389					skb->ip_summed = CHECKSUM_UNNECESSARY;
   390				/* csum is updated */
   391				ret = 1;
   392			}
   393		}
   394	
   395		/* Not setting 'diff' is intentional, otherwise the sequence
   396		 * would be adjusted twice.
   397		 */
   398	
 > 399		cp->app_data = IP_VS_FTP_ACTIVE;
   400		ip_vs_tcp_conn_listen(n_cp);
   401		ip_vs_conn_put(n_cp);
   402		return ret;
   403	}
   404	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Netfitler Users]     [LARTC]     [Bugtraq]     [Yosemite Forum]

  Powered by Linux