Hi,

On Fri, 7 Jun 2002, Andrew Morton wrote:

> 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);
>

The two codes actually identical, because the condition is always true.
There is no connection where the ORIGINAL and REPLY tuples would be equal.

Regards,
Jozsef
-
E-mail  : [EMAIL PROTECTED], [EMAIL PROTECTED]
WWW-Home: http://www.kfki.hu/~kadlec
Address : KFKI Research Institute for Particle and Nuclear Physics
          H-1525 Budapest 114, POB. 49, Hungary


Reply via email to