Re: [PATCH 2/3] ipvs: add missing lock in ip_vs_ftp_init_conn()

On Tue, Jul 17, 2012 at 5:07 AM, Pablo Neira Ayuso [off-list ref] wrote:

Hi Simon,

On Wed, Jul 11, 2012 at 09:19:22AM +0900, Simon Horman wrote:

quoted

From: Xiaotian Feng <redacted>

We met a kernel panic in 2.6.32.43 kernel:

[...]

quoted

 net/netfilter/ipvs/ip_vs_ftp.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/netfilter/ipvs/ip_vs_ftp.c b/net/netfilter/ipvs/ip_vs_ftp.c
index b20b29c..c2bc264 100644
--- a/net/netfilter/ipvs/ip_vs_ftp.c
+++ b/net/netfilter/ipvs/ip_vs_ftp.c

@@ -65,8 +65,10 @@ static int ip_vs_ftp_pasv;
 static int
 ip_vs_ftp_init_conn(struct ip_vs_app *app, struct ip_vs_conn *cp)
 {
+     spin_lock(&cp->lock);
      /* We use connection tracking for the command connection */
      cp->flags |= IP_VS_CONN_F_NFCT;
+     spin_unlock(&cp->lock);
      return 0;

The conntrack support for FTP IPVS helper seems to be there since
2.6.37.

However, the patch description mentions 2.6.32.43.

Something doesn't match here, could you clarify this?

Sorry for the misleading description in the patch. We found the panic
in 2.6.32.43 is caused by changing cp->flags without protection. In
2.6.32.43, ip_vs_process_message changes cp->flags without protection
while update active/inactive flags for the connection.

After code inspiration, we found in 3.x kernel, it is accidentally
fixed by commit  f73181c. But with ip_vs_app changes,
ip_vs_ftp_init_conn() will have chance to change cp->flags without
protection. So it is a potential bug in 3.x kernel.

Thanks.