Eric Lee / smarc-ti-linux-kernel

Download as
Browse Code ยป

Commit e7820e39b7d19b9fe1928fc19de9361b44150ca6

Authored by Eric Dumazet
Committed by David S. Miller
1 parent 892d6eb124
Exists in ti-lsk-linux-4.1.y and in 10 other branches dlt-processor-sdk-linux-03.00.00.04, processor-sdk-linux-02.00.01, smarc-ti-lsk-linux-4.1.y, smarct3x-processor-sdk-04.01.00.06, smarct3x-processor-sdk-linux-02.00.01, smarct3x-processor-sdk-linux-03.00.00.04, smarct4x-800-processor-sdk-linux-02.00.01, smarct4x-processor-sdk-04.01.00.06, smarct4x-processor-sdk-linux-02.00.01, smarct4x-processor-sdk-linux-03.00.00.04

net: Revert "net: avoid one atomic operation in skb_clone()" 

Not sure what I was thinking, but doing anything after
releasing a refcount is suicidal or/and embarrassing.

By the time we set skb->fclone to SKB_FCLONE_FREE, another cpu
could have released last reference and freed whole skb.

We potentially corrupt memory or trap if CONFIG_DEBUG_PAGEALLOC is set.

Reported-by: Chris Mason <clm@fb.com>
Fixes: ce1a4ea3f1258 ("net: avoid one atomic operation in skb_clone()")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Sabrina Dubroca <sd@queasysnail.net>
Signed-off-by: David S. Miller <davem@davemloft.net>

Showing 1 changed file with 6 additions and 17 deletions Side-by-side Diff

... ... @@ -552,20 +552,13 @@
552 552 case SKB_FCLONE_CLONE:
553 553 fclones = container_of(skb, struct sk_buff_fclones, skb2);
554 554  
555   - /* Warning : We must perform the atomic_dec_and_test() before
556   - * setting skb->fclone back to SKB_FCLONE_FREE, otherwise
557   - * skb_clone() could set clone_ref to 2 before our decrement.
558   - * Anyway, if we are going to free the structure, no need to
559   - * rewrite skb->fclone.
  555 + /* The clone portion is available for
  556 + * fast-cloning again.
560 557 */
561   - if (atomic_dec_and_test(&fclones->fclone_ref)) {
  558 + skb->fclone = SKB_FCLONE_FREE;
  559 +
  560 + if (atomic_dec_and_test(&fclones->fclone_ref))
562 561 kmem_cache_free(skbuff_fclone_cache, fclones);
563   - } else {
564   - /* The clone portion is available for
565   - * fast-cloning again.
566   - */
567   - skb->fclone = SKB_FCLONE_FREE;
568   - }
569 562 break;
570 563 }
571 564 }
... ... @@ -887,11 +880,7 @@
887 880 if (skb->fclone == SKB_FCLONE_ORIG &&
888 881 n->fclone == SKB_FCLONE_FREE) {
889 882 n->fclone = SKB_FCLONE_CLONE;
890   - /* As our fastclone was free, clone_ref must be 1 at this point.
891   - * We could use atomic_inc() here, but it is faster
892   - * to set the final value.
893   - */
894   - atomic_set(&fclones->fclone_ref, 2);
  883 + atomic_inc(&fclones->fclone_ref);
895 884 } else {
896 885 if (skb_pfmemalloc(skb))
897 886 gfp_mask |= __GFP_MEMALLOC;