Cc: Pekka Enberg <penberg@...>, Ingo Molnar <mingo@...>, David Miller <davem@...>, <herbert@...>, <w@...>, <davidn@...>, <akpm@...>, <netdev@...>, <linux-kernel@...>, <stefanr@...>, <rjw@...>, <ilpo.jarvinen@...>, Dave Jones <davej@...>
Hmm. Don't you need to fix some of the ordering of the initialization too?
If there are possible readers that happen in parallel with changing this
thing, don't you need to protect the update of "ext->len" against the
actual changes? And the readers should probably have a read barrier
between checking "len" and actually looking at the values? Finally, why do
the "ct->ext" dereference thing, when we know it has to be equal to "new"?
ie something like this on the writing side (in _addition_ to both the
patches already seen), but I didn't do the reading side (ie there are no
"smp_rmb()"'s on the reading side)
And no, I don't know the code, so I don't know who/what can read those
things with RCU, so maybe there is some reason why the actual data doesn't
need protecting. But I somehow doubt it.
Linus
---
net/netfilter/nf_conntrack_extend.c | 9 +++++----
1 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/net/netfilter/nf_conntrack_extend.c b/net/netfilter/nf_conntrack_extend.c
index 3469bc7..135e095 100644
--- a/net/netfilter/nf_conntrack_extend.c
+++ b/net/netfilter/nf_conntrack_extend.c
@@ -115,10 +115,11 @@ void *__nf_ct_ext_add(struct nf_conn *ct, enum nf_ct_ext_id id, gfp_t gfp)
ct->ext = new;
}
- ct->ext->offset[id] = newoff;
- ct->ext->len = newlen;
- memset((void *)ct->ext + newoff, 0, newlen - newoff);
- return (void *)ct->ext + newoff;
+ new->offset[id] = newoff;
+ memset((void *)new + newoff, 0, newlen - newoff);
+ smp_wmb();
+ new->len = newlen;
+ return (void *)new + newoff;
}
EXPORT_SYMBOL(__nf_ct_ext_add);
--
