[RFC/PATCH] check tcp_v4_md5_do_add retval in tcp-2.6 tree

[Sven Schuster <schuster.sven@xxxxxx>]

Hi everybody, hi David,

after the recent posting asking for the tcp md5 implementation I
checked out the tcp-2.6 repo and had a look at it. I found that in
a few places the return value of tcp_v4_md5_do_add() isn't checked
and then kmalloc()ed memory is not kfree()ed. This patch does this,
or at least tries to...but I'm not sure if the kfree() on newkey is
enough in tcp_v4_syn_recv_sock(), shouldn't we also call
tcp_v4_destroy_sock() or something like that and sk_free() on newsk,
too??
Also there is another case in tcp_minisocks.c::tcp_check_req() where
tcp_v4_md5_do_add() is called indirectly over af_specific->md5_add().
What should be done there? RST the connection??


Regards

Sven Schuster



# This is a BitKeeper generated diff -Nru style patch.
#
# ChangeSet
#   2004/10/22 12:12:11+02:00 sven@xxxxxxxxxxxxxxxxxx 
#   tcp_ipv4.c:
#     [TCP-MD5]: check return values of tcp_v4_md5_do_add
#
#     Signed-off-by: Sven Schuster <schuster.sven@xxxxxx>
# 
# net/ipv4/tcp_ipv4.c
#   2004/10/22 12:11:03+02:00 sven@xxxxxxxxxxxxxxxxxx +10 -2
#   [TCP-MD5]: check return values of tcp_v4_md5_do_add
#
#   Signed-off-by: Sven Schuster <schuster.sven@xxxxxx>
# 
diff -Nru a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
--- a/net/ipv4/tcp_ipv4.c       2004-10-22 12:13:42 +02:00
+++ b/net/ipv4/tcp_ipv4.c       2004-10-22 12:13:42 +02:00
@@ -1731,11 +1731,16 @@
                         * memory, then we end up not copying the key
                         * across. Shucks.
                         */
+                       int ret;
                        char *newkey = kmalloc(key->keylen, GFP_ATOMIC);
                        if (newkey) {
                                memcpy(newkey, key->key, key->keylen);
-                               tcp_v4_md5_do_add(newsk, inet_sk(sk)->daddr,
+                               ret = tcp_v4_md5_do_add(newsk, 
inet_sk(sk)->daddr,
                                                  newkey, key->keylen);
+                               if (ret != 0) {
+                                       kfree(newkey);
+                                       goto exit;
+                               }
                        }
                }
        }
@@ -2334,6 +2339,7 @@
 
 static int tcp_v4_md5_add(struct sock *sk, struct tcp_rfc2385_cmd *cmd)
 {
+       int ret;
        unsigned char *newkey;
 
        /* Was a key already defined for this address?
@@ -2347,7 +2353,11 @@
                return -EFAULT;
        }
 
-       return tcp_v4_md5_do_add(sk, cmd->address, newkey, cmd->keylen);
+       ret =  tcp_v4_md5_do_add(sk, cmd->address, newkey, cmd->keylen);
+       if (ret != 0)
+               kfree(newkey);
+
+       return ret;
 }
 
 /* This can be called on a newly created socket, from other files */

-- 
Linux zion 2.6.9-rc1-mm4 #1 Tue Sep 7 12:57:19 CEST 2004 i686 athlon i386 
GNU/Linux
 16:34:02 up 26 days, 15:48,  1 user,  load average: 0.00, 0.01, 0.00

pgpJ9QzAFJS5s.pgp
Description: PGP signature