Re: [patch 2/16] list_head debugging

From: Andrew Morton (akpm@zip.com.au)
Date: Fri Jun 07 2002 - 13:30:50 EST


Bernd Jendrissek wrote:
> [sorry for the nonexistent In-Reply-To/whatever headers - cutting&pasting]
>
> Andrew Morton wrote:
>
>> A common and very subtle bug is to use list_heads which aren't on any
>> lists. It causes kernel memory corruption which is observed long after
>> the offending code has executed.
>>
>> The patch nulls out the dangling pointers so we get a nice oops at the
>> site of the buggy code.
>
>
> I'm not current with the kernel tree, but will one such oops occur in
> netfilter? See
>
> http://lists.samba.org/pipermail/netfilter-announce/2002/000010.html
>
> Hmm, no. A DoS maybe?
>

An oops, actually. This code:

         /* Remove from both hash lists: must not NULL out next ptrs,
            otherwise we'll look unconfirmed. Fortunately, LIST_DELETE
            doesn't do this. --RR */
         LIST_DELETE(&ip_conntrack_hash
                     [hash_conntrack(&ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple)],
                     &ct->tuplehash[IP_CT_DIR_ORIGINAL]);
         LIST_DELETE(&ip_conntrack_hash
                     [hash_conntrack(&ct->tuplehash[IP_CT_DIR_REPLY].tuple)],
                     &ct->tuplehash[IP_CT_DIR_REPLY]);

I think what is needed is:

--- 2.5.20/net/ipv4/netfilter/ip_conntrack_core.c~ipconntrack-lists Fri Jun 7 11:26:38 2002
+++ 2.5.20-akpm/net/ipv4/netfilter/ip_conntrack_core.c Fri Jun 7 11:26:42 2002
@@ -210,17 +210,22 @@ static void destroy_expectations(struct
  static void
  clean_from_lists(struct ip_conntrack *ct)
  {
+
struct list_head *l1;
+
struct list_head *l2;
+
          DEBUGP("clean_from_lists(%p)\n", ct);
          MUST_BE_WRITE_LOCKED(&ip_conntrack_lock);
-
/* Remove from both hash lists: must not NULL out next ptrs,
- otherwise we'll look unconfirmed. Fortunately, LIST_DELETE
- doesn't do this. --RR */
+
+
l1 = &ct->tuplehash[IP_CT_DIR_ORIGINAL];
+
l2 = &ct->tuplehash[IP_CT_DIR_REPLY];
+
          LIST_DELETE(&ip_conntrack_hash
                      [hash_conntrack(&ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple)],
-
            &ct->tuplehash[IP_CT_DIR_ORIGINAL]);
-
LIST_DELETE(&ip_conntrack_hash
-
            [hash_conntrack(&ct->tuplehash[IP_CT_DIR_REPLY].tuple)],
-
            &ct->tuplehash[IP_CT_DIR_REPLY]);
+
            l1);
+
if (l1 != l2)
+
        LIST_DELETE(&ip_conntrack_hash
+
                    [hash_conntrack(&ct->tuplehash[IP_CT_DIR_REPLY].tuple)],
+
                    l2);

          /* Destroy all un-established, pending expectations */
          destroy_expectations(ct);

-

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Fri Jun 07 2002 - 22:00:31 EST