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: <CAJkfWY7dGXV=YPaCKqZ0YguRGTN2JA+n9jyVWk8LPj3dgtC=tA@mail.gmail.com>
Date:   Tue, 13 Sep 2022 13:46:00 -0700
From:   Nathan Huckleberry <nhuck@...gle.com>
To:     Casper Andersson <casper.casan@...il.com>
Cc:     Dan Carpenter <error27@...il.com>, llvm@...ts.linux.dev,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Lars Povlsen <lars.povlsen@...rochip.com>,
        Steen Hegelund <Steen.Hegelund@...rochip.com>,
        UNGLinuxDriver@...rochip.com,
        Nathan Chancellor <nathan@...nel.org>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        Tom Rix <trix@...hat.com>,
        Horatiu Vultur <horatiu.vultur@...rochip.com>,
        netdev@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] net: sparx5: Fix return type of sparx5_port_xmit_impl

Hey Casper,

On Tue, Sep 13, 2022 at 1:15 AM Casper Andersson <casper.casan@...il.com> wrote:
>
> Hi,
>
> On 2022-09-12 14:44, Nathan Huckleberry wrote:
> > The ndo_start_xmit field in net_device_ops is expected to be of type
> > netdev_tx_t (*ndo_start_xmit)(struct sk_buff *skb, struct net_device *dev).
> >
> > The mismatched return type breaks forward edge kCFI since the underlying
> > function definition does not match the function hook definition.
> >
> > The return type of sparx5_port_xmit_impl should be changed from int to
> > netdev_tx_t.
>
> I noticed that the functions that assign the return value inside
> sparx5_port_xmit_impl also have return type int, which would ideally
> also be changed. But a bigger issue might be that
> sparx5_ptp_txtstamp_request and sparx5_inject (called inside
> sparx5_port_xmit_impl) returns -EBUSY (-16), when they should return
> NETDEV_TX_BUSY (16). If this is an issue then it also needs to be fixed.

It's not clear to me what happens when returning an error vs
NETDEV_TX_BUSY. The netdev_tx_t enum allows negative values, so
returning an error might be valid. If anyone more familiar with this
code has insight into why it's done like this, that'd be useful.

The important bit here for me is changing the return type on the
hooked function since the current code will cause panics.

>
> sparx5_fdma_xmit also has int return type, but always returns
> NETDEV_TX_OK right now.
>
> Best Regards,
> Casper
>

Thanks,
Huck

Powered by blists - more mailing lists