netdev - Re: [RFC PATCH net-next] sfc: replace in

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] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 28 Sep 2020 22:24:57 +0200
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Edward Cree <ecree@...arflare.com>,
        linux-net-drivers@...arflare.com
Cc:     linux-kernel@...r.kernel.org, netdev@...r.kernel.org
Subject: Re: [RFC PATCH net-next] sfc: replace in_interrupt() usage

On Mon, Sep 28 2020 at 21:05, Edward Cree wrote:
> efx_ef10_try_update_nic_stats_vf() used in_interrupt() to figure out
>  whether it is safe to sleep (for MCDI) or not.
> The only caller from which it was not is efx_net_stats(), which can be
>  invoked under dev_base_lock from net-sysfs::netstat_show().
> So add a new update_stats_atomic() method to struct efx_nic_type, and
>  call it from efx_net_stats(), removing the need for
>  efx_ef10_try_update_nic_stats_vf() to behave differently for this case
>  (which it wasn't doing correctly anyway).
> For all nic_types other than EF10 VF, this method is NULL and so we
>  call the regular update_stats() methods, which are happy with being
>  called from atomic contexts.
>
> Fixes: f00bf2305cab ("sfc: don't update stats on VF when called in atomic context")
> Reported-by: Sebastian Andrzej Siewior <bigeasy@...utronix.de>
> Signed-off-by: Edward Cree <ecree@...arflare.com>

That's much nicer.

> ---
> Only compile-tested so far, because I'm waiting for my kernel to
>  finish rebuilding with CONFIG_DEBUG_ATOMIC_SLEEP which I'm hoping
>  is the right thing to detect the bug in the existing code.
> I also wasn't quite sure how to give credit to the thorough analysis
>  in the commit message of Sebastian's patch.  I don't think we have
>  a Whatever-by: tag to cover that, do we?

Sebastian did the analysis and I did some word polishing, but the credit
surely goes to him.

Thanks,

        tglx