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
| ||
|
Date: Wed, 29 Apr 2020 23:01:51 -0700 From: Nathan Chancellor <natechancellor@...il.com> To: Michael Kelley <mikelley@...rosoft.com> Cc: KY Srinivasan <kys@...rosoft.com>, Haiyang Zhang <haiyangz@...rosoft.com>, Stephen Hemminger <sthemmin@...rosoft.com>, Wei Liu <wei.liu@...nel.org>, "linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>, "netdev@...r.kernel.org" <netdev@...r.kernel.org>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, "clang-built-linux@...glegroups.com" <clang-built-linux@...glegroups.com>, Sami Tolvanen <samitolvanen@...gle.com> Subject: Re: [PATCH v2] hv_netvsc: Fix netvsc_start_xmit's return type Hi Michael, On Thu, Apr 30, 2020 at 12:06:09AM +0000, Michael Kelley wrote: > From: Nathan Chancellor <natechancellor@...il.com> Sent: Tuesday, April 28, 2020 10:55 AM > > > > Do note that netvsc_xmit still returns int because netvsc_xmit has a > > potential return from netvsc_vf_xmit, which does not return netdev_tx_t > > because of the call to dev_queue_xmit. > > > > I am not sure if that is an oversight that was introduced by > > commit 0c195567a8f6e ("netvsc: transparent VF management") or if > > everything works properly as it is now. > > > > My patch is purely concerned with making the definition match the > > prototype so it should be NFC aside from avoiding the CFI panic. > > > > While it probably works correctly now, I'm not too keen on just > changing the return type for netvsc_start_xmit() and assuming the > 'int' that is returned from netvsc_xmit() will be correctly mapped to > the netdev_tx_t enum type. While that mapping probably happens > correctly at the moment, this really should have a more holistic fix. While it might work correctly, I am not sure that the mapping is correct, hence that comment. netdev_tx_t is an enum with two acceptable types, 0x00 and 0x10. Up until commit 0c195567a8f6e ("netvsc: transparent VF management"), netvsc_xmit was guaranteed to return something of type netdev_tx_t. However, after said commit, we could return anything from netvsc_vf_xmit, which in turn calls dev_queue_xmit then __dev_queue_xmit which will return either an error code (-ENOMEM or -ENETDOWN) or something from __dev_xmit_skb, which appears to be NET_XMIT_SUCCESS, NET_XMIT_DROP, or NET_XMIT_CN. It does not look like netvsc_xmit or netvsc_vf_xmit try to convert those returns to netdev_tx_t in some way; netvsc_vf_xmit just passes the return value up to netvsc_xmit, which is the part that I am unsure about... > Nathan -- are you willing to look at doing the more holistic fix? Or > should we see about asking Haiyang Zhang to do it? I would be fine trying to look at a more holistic fix but I know basically nothing about this subsystem. I am unsure if something like this would be acceptable or if something else needs to happen. Otherwise, I'd be fine with you guys taking a look and just giving me reported-by credit. Cheers, Nathan diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c index d8e86bdbfba1e..a39480cfb8fa7 100644 --- a/drivers/net/hyperv/netvsc_drv.c +++ b/drivers/net/hyperv/netvsc_drv.c @@ -520,7 +520,8 @@ static int netvsc_vf_xmit(struct net_device *net, struct net_device *vf_netdev, return rc; } -static int netvsc_xmit(struct sk_buff *skb, struct net_device *net, bool xdp_tx) +static netdev_tx_t netvsc_xmit(struct sk_buff *skb, struct net_device *net, + bool xdp_tx) { struct net_device_context *net_device_ctx = netdev_priv(net); struct hv_netvsc_packet *packet = NULL; @@ -537,8 +538,11 @@ static int netvsc_xmit(struct sk_buff *skb, struct net_device *net, bool xdp_tx) */ vf_netdev = rcu_dereference_bh(net_device_ctx->vf_netdev); if (vf_netdev && netif_running(vf_netdev) && - !netpoll_tx_running(net)) - return netvsc_vf_xmit(net, vf_netdev, skb); + !netpoll_tx_running(net)) { + if (!netvsc_vf_xmit(net, vf_netdev, skb)) + return NETDEV_TX_OK; + goto drop; + } /* We will atmost need two pages to describe the rndis * header. We can only transmit MAX_PAGE_BUFFER_COUNT number @@ -707,7 +711,8 @@ static int netvsc_xmit(struct sk_buff *skb, struct net_device *net, bool xdp_tx) goto drop; } -static int netvsc_start_xmit(struct sk_buff *skb, struct net_device *ndev) +static netdev_tx_t netvsc_start_xmit(struct sk_buff *skb, + struct net_device *ndev) { return netvsc_xmit(skb, ndev, false); }
Powered by blists - more mailing lists