On 2003.08.12 22:56:55 +0200 Stephen Hemminger wrote:
It looks like af_x25 is advertising itself as a new style protocol, yet
if
I walk the receive path:
ax25_kiss_rcv -> ax25_rcv -> ax25_addr_parse
there is no place that checks that the address portion of the buffer
isn't fragmented
into a non_linear skb.
Since at the moment the only zero-copy path I can imagine is from packets
over a bpq ethernet device wouldn't it be the easiest to just check at
ax25_rcv() for non-linear packets and call skb_linearize() for them? That
way we would be safe untill we can make sure that the whole of the ax25
stack is non-linear safe?
Should I make a new patch with this included? (and possibly the ifconfig
problem you mentioned?)
Jeroen
The problem is that you are freeing the ax25 control block too soon for the case
of sockets that were never bound. If the socket is not bound, it never makes it
into the node list and the refcount is 1. So when you decrement in ax25_cb_del
by calling ax25_cb_put it gets freed.
This fixes the problem, it assumes your earlier patch has been applied.
diff -Nru a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c
--- a/net/ax25/af_ax25.c Tue Aug 12 15:35:33 2003
+++ b/net/ax25/af_ax25.c Tue Aug 12 15:35:33 2003
@@ -66,10 +66,12 @@
*/
static void ax25_cb_del(ax25_cb *ax25)
{
- spin_lock_bh(&ax25_list_lock);
- hlist_del_init(&ax25->ax25_node);
- spin_unlock_bh(&ax25_list_lock);
- ax25_cb_put(ax25);
+ if (!hlist_unhashed(&ax25->ax25_node)) {
+ spin_lock_bh(&ax25_list_lock);
+ hlist_del(&ax25->ax25_node);
+ spin_unlock_bh(&ax25_list_lock);
+ ax25_cb_put(ax25);
+ }
}
/*On Tue, 12 Aug 2003 23:09:51 +0200
Jeroen Vreeken [off-list ref] wrote:
Since at the moment the only zero-copy path I can imagine is from packets
over a bpq ethernet device wouldn't it be the easiest to just check at
ax25_rcv() for non-linear packets and call skb_linearize() for them?
skb_linearize() is a deprecated interface, no new pieces of
code should use it. If you want to mark the protocol as a new
one you must do the non-linear handling properly just like every
other protocol.
Please, if you're going to be working in this area, fix this up properly
and don't paper around the problems Stephen is showing to you.