possible double call of kfree_skb in net/llc/llc_sap.c

possible double call of kfree_skb in net/llc/llc_sap.c

[Dmitry Petukhov]

In the file  net/llc/llc_sap.c, funcion llc_sap_state_process,
the call to kfree_skb in the line 227 can proceed even if skb was already freed
on line 218, or  224, or queued to the user within sock_queue_rcv_skb function.
Obviously return statement is missing after line 225.

This problem was found by Alex Shevkov.

the code in question:

204 static void llc_sap_state_process(struct llc_sap *sap, struct sk_buff *skb)
205 {
206         struct llc_sap_state_ev *ev = llc_sap_ev(skb);
207
....
213         skb_get(skb);
214         ev->ind_cfm_flag = 0;
215         llc_sap_next_state(sap, skb);
216         if (ev->ind_cfm_flag == LLC_IND) {
217                 if (skb->sk->sk_state == TCP_LISTEN)
218                         kfree_skb(skb);
219                 else {
220                         llc_save_primitive(skb->sk, skb, ev->prim);
221
222                         /* queue skb to the user. */
223                         if (sock_queue_rcv_skb(skb->sk, skb))
224                                 kfree_skb(skb);
225                 }
226         }
227         kfree_skb(skb);
228 }

Re: possible double call of kfree_skb in net/llc/llc_sap.c

[Patrick McHardy]

Dmitry Petukhov wrote:
> In the file  net/llc/llc_sap.c, funcion llc_sap_state_process,
> the call to kfree_skb in the line 227 can proceed even if skb was already freed
> on line 218, or  224, or queued to the user within sock_queue_rcv_skb function.
> Obviously return statement is missing after line 225.
> 
> This problem was found by Alex Shevkov.

Could you send a patch please?

Re: possible double call of kfree_skb in net/llc/llc_sap.c

[Pekka Enberg]

Dmitry Petukhov wrote:
>> In the file  net/llc/llc_sap.c, funcion llc_sap_state_process,
>> the call to kfree_skb in the line 227 can proceed even if skb was already
>> freed
>> on line 218, or  224, or queued to the user within sock_queue_rcv_skb
>> function.
>> Obviously return statement is missing after line 225.
>>
>> This problem was found by Alex Shevkov.

On Tue, May 27, 2008 at 9:16 AM, Patrick McHardy <kaber@trash.net> wrote:
> Could you send a patch please?

I wonder how the code got in that shape but it's probably best to just
remove the extra kfree_skb() calls and do it unconditionally at the
end of the function.