netdev - Re: [PATCH bpf] nfp: bpf: don't stop offload if replace failed

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <b69ee659-8dfe-acd3-f32c-f16c72215ad1@iogearbox.net>
Date:   Tue, 26 Jun 2018 12:02:20 +0200
From:   Daniel Borkmann <daniel@...earbox.net>
To:     Jakub Kicinski <jakub.kicinski@...ronome.com>,
        alexei.starovoitov@...il.com
Cc:     netdev@...r.kernel.org, oss-drivers@...ronome.com
Subject: Re: [PATCH bpf] nfp: bpf: don't stop offload if replace failed

On 06/22/2018 08:56 PM, Jakub Kicinski wrote:
> Stopping offload completely if replace of program failed dates
> back to days of transparent offload.  Back then we wanted to
> silently fall back to the in-driver processing.  Today we mark
> programs for offload when they are loaded into the kernel, so
> the transparent offload is no longer a reality.
> 
> Flags check in the driver will only allow replace of a driver
> program with another driver program or an offload program with
> another offload program.
> 
> When driver program is replaced stopping offload is a no-op,
> because driver program isn't offloaded.  When replacing
> offloaded program if the offload fails the entire operation
> will fail all the way back to user space and we should continue
> using the old program.  IOW when replacing a driver program
> stopping offload is unnecessary and when replacing offloaded
> program - it's a bug, old program should continue to run.
> 
> In practice this bug would mean that if offload operation was to
> fail (either due to FW communication error, kernel OOM or new
> program being offloaded but for a different netdev) driver
> would continue reporting that previous XDP program is offloaded
> but in fact no program will be loaded in hardware.  The failure
> is fairly unlikely (found by inspection, when working on the code)
> but it's unpleasant.
> 
> Backport note: even though the bug was introduced in commit
> cafa92ac2553 ("nfp: bpf: add support for XDP_FLAGS_HW_MODE"),
> this fix depends on commit 441a33031fe5 ("net: xdp: don't allow
> device-bound programs in driver mode"), so this fix is sufficient
> only in v4.15 or newer.  Kernels v4.13.x and v4.14.x do need to
> stop offload if it was transparent/opportunistic, i.e. if
> XDP_FLAGS_HW_MODE was not set on running program.
> 
> Fixes: cafa92ac2553 ("nfp: bpf: add support for XDP_FLAGS_HW_MODE")
> Signed-off-by: Jakub Kicinski <jakub.kicinski@...ronome.com>
> Reviewed-by: Quentin Monnet <quentin.monnet@...ronome.com>

Applied to bpf, thanks Jakub!