Intel-Wired-Lan Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Jacob Keller <jacob.e.keller@intel.com>
To: <intel-wired-lan@osuosl.org>
Subject: Re: [Intel-wired-lan] [net, v1] ice: Fix l2-fwd-offload toggle crash
Date: Mon, 17 Oct 2022 14:13:05 -0700	[thread overview]
Message-ID: <b58bdbc1-9da4-9093-6ebe-7f205cc6b1b3@intel.com> (raw)
In-Reply-To: <CADEbmW0wNym5TJiXeJ+zABb3a5eRvSdPZqWYnDwB6KnXUYK8nQ@mail.gmail.com>



On 10/13/2022 8:25 AM, Michal Schmidt wrote:
> On Wed, Oct 12, 2022 at 6:03 PM Benjamin Mikailenko
> <benjamin.mikailenko@intel.com <mailto:benjamin.mikailenko@intel.com>>
> wrote:
> 
>     Running netperf traffic and toggling l2-fwd-offload in quick succession
>     caused the driver to crash.
> 
>     BUG: kernel NULL pointer dereference, address: 0000000000000020
>     [  861.517803] #PF: supervisor read access in kernel mode
>     [  861.517805] #PF: error_code(0x0000) - not-present page
>     [  861.517808] PGD 0 P4D 0
>     [  861.517811] Oops: 0000 [#1] PREEMPT SMP PTI
>     [  861.517815] CPU: 60 PID: 16471 Comm: netperf Kdump: loaded
>     Tainted: G S
>     [  861.517818] Hardware name: Intel Corporation S2600WTT/S2600WTT,
>     BIOS SE
>     [  861.517820] RIP: 0010:ice_start_xmit+0xb0/0x1420 [ice]
> 
>     This crash would happen because during l2-fwd-offload configuration,
>     ice_init_macvlan or ice_deinit_macvlan would temporarily work on Tx
>     rings.
> 
> 
> What are ice_init_macvlan and ice_deinit_macvlan? Are they function
> names? I do not see such functions in the code.
>  
> 
>     At the same time, ice_start_xmit would attempt to select the correct
>     send
>     buffer from Tx rings but reach a NULL pointer.
> 
>     Fix this by checking if ring exists before proceeding xmit. If ring does
>     not exist, return NETDEV_TX_BUSY.
> 
> 
> Isn't it still racy though?
> Shouldn't rather whatever is fiddling with the rings make sure the Tx
> queues are stopped first with netif_tx_stop_queue or similar?
> Michal
>  

I was going to mention this. No sign of a lock or any other mechanism.
Ideally separately managed structures like this would benefit from
reference counting or such..

Still, sounds like this patch needs to be rejected per other comments.

Thanks,
Jake
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

      parent reply	other threads:[~2022-10-17 21:13 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-12 15:55 [Intel-wired-lan] [net,v1] ice: Fix l2-fwd-offload toggle crash Benjamin Mikailenko
2022-10-13 15:25 ` [Intel-wired-lan] [net, v1] " Michal Schmidt
2022-10-13 16:51   ` Benjamin Mikailenko
2022-10-17 21:14     ` Jacob Keller
2022-10-17 21:13   ` Jacob Keller [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b58bdbc1-9da4-9093-6ebe-7f205cc6b1b3@intel.com \
    --to=jacob.e.keller@intel.com \
    --cc=intel-wired-lan@osuosl.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox