[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?
> --- 2.5.19/include/linux/list.h~list-debug Sat Jun 1 01:18:05 2002
> +++ 2.5.19-akpm/include/linux/list.h Sat Jun 1 01:18:05 2002
> @@ -94,6 +94,11 @@ static __inline__ void __list_del(struct
> static __inline__ void list_del(struct list_head *entry)
> {
> __list_del(entry->prev, entry->next);
> + /*
> + * This is debug. Remove it when the kernel has no bugs ;)
> + */
> + entry->next = 0;
> + entry->prev = 0;
> }
>
> /**
Bernd Jendrissek
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);
-
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