Re: Question about skb clones and frag_list
Vlad Yasevich wrote: On Wed, 2006-04-05 at 10:38 -0600, Mark Butler wrote: The problem in this case is skb1 does not own the skb_shared_info structure (and hence the frag_list) once it has been cloned. skb_unshare is the function that should be used to get a modifiable copy. So, what you are saying is that what sctp_make_reassembled_event() is doing is really illegal? Yes - to the extent it doesn't make sure that the header skb has its own skb_shared_info first, it is doing something that does not work in general. The problem you are seeing should only arise if one receives more than one data chunk for the same message in the same packet, however, and that should be relatively unusual, though certainly not illegal. - Mark B. - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Question about skb clones and frag_list
On Wed, 2006-04-05 at 10:38 -0600, Mark Butler wrote: > Vlad Yasevich wrote: > > >Hi All > > > >I am trying to understand if it is a good idea to have cloned skbs > >reside on a frag_list? > > > >I've ran into a situation with SCTP, where it is possible to create > >infinite recursion loops by having cloned skbs reside on a frag_list. > >We end up in a situation where have two clones skb1 and skb2 that are > >clones of the same original skb. Then we try adding skb2 to the > >frag_list of skb1. Since both skb1 and skb2 share the skb_shared_info > >structure, skb2's frag_list now points to skb2 creating a loop. > > > >This situation also appears to cause a memory leak where skb2 and the > >actual data are not freed, because calling kfree_skb(skb1) should really > >free the frag_list, but we can't do that because the skb on the frag > >list holds the dataref on the same data. > > > >Can anyone say that putting clones on the frag_list is a BAD THING (tm) > >and shouldn't be done? Or is there a way around it? > > > > > > > The problem in this case is skb1 does not own the skb_shared_info > structure (and hence the frag_list) once it has been cloned. > skb_unshare is the function that should be used to get a modifiable copy. So, what you are saying is that what sctp_make_reassembled_event() is doing is really illegal? That makes sense... Thanks -vlad - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Question about skb clones and frag_list
Vlad Yasevich wrote: Hi All I am trying to understand if it is a good idea to have cloned skbs reside on a frag_list? I've ran into a situation with SCTP, where it is possible to create infinite recursion loops by having cloned skbs reside on a frag_list. We end up in a situation where have two clones skb1 and skb2 that are clones of the same original skb. Then we try adding skb2 to the frag_list of skb1. Since both skb1 and skb2 share the skb_shared_info structure, skb2's frag_list now points to skb2 creating a loop. This situation also appears to cause a memory leak where skb2 and the actual data are not freed, because calling kfree_skb(skb1) should really free the frag_list, but we can't do that because the skb on the frag list holds the dataref on the same data. Can anyone say that putting clones on the frag_list is a BAD THING (tm) and shouldn't be done? Or is there a way around it? The problem in this case is skb1 does not own the skb_shared_info structure (and hence the frag_list) once it has been cloned. skb_unshare is the function that should be used to get a modifiable copy. By the way, the comments and the function names in linux/skbuff.h overload the term "buffer" and "share" in a very ambiguous manner. skb_shared() and skb_shared_check() check to see if the sk_buff itself is has multiple references, where skb_cloned() and skb_unshare() check to see if the skb_shared_info has multiple references. The descriptions of the functions themselves do not make the distinction, referring to both as a "buffer". - Mark B. - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
