netdev - Re: [PATCH net] net: flow_offload: Fix UBSAN invalid-load warning in tcf_block

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: <20210420081059.GA18290@vdi.nvidia.com>
Date:   Tue, 20 Apr 2021 08:11:00 +0000
From:   Jianbo Liu <jianbol@...dia.com>
To:     Jakub Kicinski <kuba@...nel.org>
CC:     <netdev@...r.kernel.org>, <pablo@...filter.org>,
        Roi Dayan <roid@...dia.com>
Subject: Re: [PATCH net] net: flow_offload: Fix UBSAN invalid-load warning in
 tcf_block_unbind

The 04/09/2021 09:01, Jakub Kicinski wrote:
> On Fri, 9 Apr 2021 06:25:56 +0000 Jianbo Liu wrote:
> > The 04/08/2021 14:16, Jakub Kicinski wrote:
> > > On Thu, 8 Apr 2021 07:47:18 +0000 Jianbo Liu wrote:  
> > > > When device is removed, indirect block is unregisterd. As
> > > > bo->unlocked_driver_cb is not initialized, the following UBSAN is
> > > > triggered.
> > > > 
> > > > UBSAN: invalid-load in net/sched/cls_api.c:1496:10
> > > > load of value 6 is not a valid value for type '_Bool'
> > > > 
> > > > This patch fixes the warning by calling device's indr block bind
> > > > callback, and unlocked_driver_cb is assigned with correct value.
> > > > 
> > > > Fixes: 0fdcf78d5973 ("net: use flow_indr_dev_setup_offload()")
> > > > Signed-off-by: Jianbo Liu <jianbol@...dia.com>
> > > > Reviewed-by: Roi Dayan <roid@...dia.com>  
> > > 
> > > It's been a while since I looked at this code but I don't understand
> > > what you're doing here.  
> > 
> > To fix the UBSAN warning in tcf_block_unbind. It's easily triggered when
> > netdev is removed before tunnel netdev.
> > 
> > > The init in tc_block_indr_cleanup() makes sense. What's the change to
> > > setup_cb achieving? Thanks.  
> > 
> > But unlocked_driver_cb of flow_block_offload is not initialized in init.
> > Calling setup_cb is to get the correct value from driver.
> 
> I'm trying to understand what became of this code :/ Was there no call
> with FLOW_BLOCK_UNBIND to the driver when driver was unregistering
> before your change?
> 

Gentle ping.
Should I need any change for this patch?

Thanks!


--