netdev - Re: [PATCH] net/ncsi: Use real net-device for response handler

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]

Message-ID: <20201222182458.4651c564@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Tue, 22 Dec 2020 18:24:58 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     Samuel Mendoza-Jonas <sam@...dozajonas.com>
Cc:     Joel Stanley <joel@....id.au>,
        John Wang <wangzhiqiang.bj@...edance.com>,
        xuxiaohan@...edance.com,
        郁雷 <yulei.sh@...edance.com>,
        "David S. Miller" <davem@...emloft.net>,
        Gavin Shan <gwshan@...ux.vnet.ibm.com>,
        "open list:NETWORKING [GENERAL]" <netdev@...r.kernel.org>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] net/ncsi: Use real net-device for response handler

On Tue, 22 Dec 2020 10:38:21 -0800 Samuel Mendoza-Jonas wrote:
> On Tue, 2020-12-22 at 06:13 +0000, Joel Stanley wrote:
> > On Sun, 20 Dec 2020 at 12:40, John Wang wrote:
> > > When aggregating ncsi interfaces and dedicated interfaces to bond
> > > interfaces, the ncsi response handler will use the wrong net device
> > > to
> > > find ncsi_dev, so that the ncsi interface will not work properly.
> > > Here, we use the net device registered to packet_type to fix it.
> > > 
> > > Fixes: 138635cc27c9 ("net/ncsi: NCSI response packet handler")
> > > Signed-off-by: John Wang <wangzhiqiang.bj@...edance.com>  

This sounds like exactly the case for which orig_dev was introduced.
I think you should use the orig_dev argument, rather than pt->dev.

Can you test if that works?

> > Can you show me how to reproduce this?
> > 
> > I don't know the ncsi or net code well enough to know if this is the
> > correct fix. If you are confident it is correct then I have no
> > objections.  
> 
> This looks like it is probably right; pt->dev will be the original
> device from ncsi_register_dev(), if a response comes in to
> ncsi_rcv_rsp() associated with a different device then the driver will
> fail to find the correct ncsi_dev_priv. An example of the broken case
> would be good to see though.

From the description sounds like the case is whenever the ncsi
interface is in a bond, the netdev from the second argument is 
the bond not the interface from which the frame came. It should 
be possible to repro even with only one interface on the system,
create a bond or a team and add the ncsi interface to it.

Does that make sense? I'm likely missing the subtleties here.