linuxkpi: Queue skbuffs at tail in skb_queue_tail

[tom_coldrick.cc]

Summary

This patch corrects skb_queue_tail to queue the buffer at the tail of the skbuff. The skbuff is a circular doubly-linked list, and we call with a pointer to the head of the list. Thus queueing before the head gives us a queueing at the tail.

As a motivating factor, the current behaviour (queueing at the head) was causing frequent kernel panics from my RTL8822BE wireless card, which uses the rtw88 driver. Interrupts can cause buffers to be added to the rtwdev c2h_queue while the queue is being drained in rtw_c2h_work. Queueing at the head would leave the nascent entry in the linked list pointing to the old, now freed, memory for the buffer being processed. When rtw_c2h_work is next called, we try reading this and so panic.

Now, I'm under no impression that this patch actually fixes the above race, but it does prevent the panics, making my laptop usable without frustration. I think the skbuff's lock would need to be taken when queueing or unlinking buffers to really avoid this, but I'm not familiar enough with kernel programming to know how this will interact with interrupts or performance.

Test Plan

I've been running using this patch for over a month and have had no issues due to it. It's also a minor change and easy enough to reason about.

[emaste]

FYI, please generate diffs for reviews with full context e.g. git diff -U999999 or git show -U999999 so that Phabricator will display the additional context on demand. See https://wiki.freebsd.org/Phabricator for more info.

[emaste]

Now in my staging tree as:

commit d333f718de798843043bc053cf5fe9a29d7ae0a0 (wipbsd)
Author: Tom Coldrick <[e-mail]>
Date:   Sat Mar 2 14:22:55 2024 -0500

    linuxkpi: Queue skbuffs at tail in skb_queue_tail
    
    Correct skb_queue_tail to queue the buffer at the tail of the skbuff.
    The skbuff is a circular doubly-linked list, and we call with a pointer
    to the head of the list.  Thus queueing before the head gives us a
    queueing at the tail.
    
    As a motivating factor, the current behaviour (queueing at the head) was
    causing frequent kernel panics from my RTL8822BE wireless card, which
    uses the rtw88 driver.  Interrupts can cause buffers to be added to the
    rtwdev c2h_queue while the queue is being drained in rtw_c2h_work.
    Queueing at the head would leave the nascent entry in the linked list
    pointing to the old, now freed, memory for the buffer being processed.
    When rtw_c2h_work is next called, we try reading this and so panic.
    
    Reviewed by:    emaste
    MFC after:      1 week
    Differential Revision: https://reviews.freebsd.org/D44192

[bz]

Great catch! I wonder what else that will help for the other (unconnected) drivers. Thanks!

@emaste usually I try to spell LinuxKPI: queue...

Also the real fix is in __skb_queue_tail but I think that's fine.

Was there a PR for the rtw88 panic somewhere?
From the ones blocking https://bugs.freebsd.org/bugzilla/showdependencytree.cgi?id=273621&hide_resolved=1 I couldn't see.

[emaste]

@bz thanks, commit subject line and Reviewed-by's updated.